Page MenuHomeFreeBSD

kldxref(8): Sort MDT_MODULE info first in linker.hints output
ClosedPublic

Authored by cem on May 25 2019, 4:43 AM.

Details

Summary

MDT_MODULE info is required to be ordered before any other MDT metadata for
a given kld because it serves as an implicit record boundary between
distinct klds for linker.hints consumers. kldxref(8) has previously relied
on the assumption that MDT_MODULE was ordered relative to other module
metadata in kld objects by source code ordering.

However, C does not require implementations to emit file scope objects in
any particular order, and it seems that GCC 6.4.0 and/or binutils 2.32 ld
may reorder emitted objects with respect to source code ordering.

So: just take two passes over a given .ko's module metadata, scanning for
the MDT_MODULE on the first pass and the other metadata on subsequent
passes. It's not super expensive and not exactly a performance-critical
piece of code. This ensures MDT_MODULE is always ordered before
MDT_PNP_INFO and other MDTs, regardless of compiler/linker movement. As a
fringe benefit, it removes the requirement that care be taken to always
order MODULE_PNP_INFO after DRIVER_MODULE in source code.

Test Plan

This change fixes devmatch on my sample kld where the MDT_MODULE ended up at
0x4b0 and the MDT_PNP_INFO at 0x460.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cem created this revision.May 25 2019, 4:43 AM
imp added a comment.May 25 2019, 3:19 PM

Other than word-smithing the comments, I like it.

usr.sbin/kldxref/kldxref.c
579 ↗(On Diff #57880)

MDT_MODULE is required to be first for all the other data that follows, not just devmatch. devmatch is the most obvious thing with ordering requirements.

581 ↗(On Diff #57880)

I hate vague things like this. Which compilers? Which linkers? Why? There's a number of other structures that we lay out in a similar manner to the modules stuff we need to audit if this is really the case.

586 ↗(On Diff #57880)

I'd phrase this a little differently

The standard does not impose address ordering constraints relative to source ordering. Ensure that MDT_MODULE is emitted first because all other MDT types rely on it implicitly to give them proper context.

cem added inline comments.May 25 2019, 4:48 PM
usr.sbin/kldxref/kldxref.c
579 ↗(On Diff #57880)

Ok, thanks. devmatch is just the one I’m aware of. Will reword.

581 ↗(On Diff #57880)

In particular either gcc6.4.0 or whatever version of ld is used (or whatever is used for .kos, which are not actually linked on amd64?). I can give specific numbers if it’s helpful.

586 ↗(On Diff #57880)

Will fix

cem marked 3 inline comments as done.May 25 2019, 9:04 PM
cem added inline comments.
usr.sbin/kldxref/kldxref.c
581 ↗(On Diff #57880)

I've changed the text and commit message to provide specific versions used. I'd like to help audit for other structures that require similar ordering, if you can give me some pointers on where to start. Thanks!

cem updated this revision to Diff 57902.May 25 2019, 9:05 PM

Rework comment wording describing the change.

cem edited the summary of this revision. (Show Details)May 25 2019, 9:05 PM
emaste accepted this revision.May 27 2019, 1:57 PM

LGTM

This revision is now accepted and ready to land.May 27 2019, 1:57 PM
imp accepted this revision.May 27 2019, 3:16 PM
This revision was automatically updated to reflect the committed changes.