Page MenuHomeFreeBSD

Implement special handling of soft-float binaries.
ClosedPublic

Authored by imp on Jun 3 2015, 1:47 AM.
Tags
None
Referenced Files
F108090073: D2718.id6110.diff
Tue, Jan 21, 7:15 AM
Unknown Object (File)
Sat, Jan 18, 4:37 AM
Unknown Object (File)
Mon, Jan 13, 7:07 AM
Unknown Object (File)
Wed, Jan 8, 10:16 AM
Unknown Object (File)
Dec 3 2024, 3:25 PM
Unknown Object (File)
Oct 9 2024, 5:13 AM
Unknown Object (File)
Sep 24 2024, 11:31 PM
Unknown Object (File)
Sep 24 2024, 4:35 PM
Subscribers

Details

Summary

Move all the paths into a new path.h to centralize them.

Rather than using the #define for path names, indirect through a char *
variable that could change for different executable types detected.

Use a macro to create the names for the library path names. This will
allow later substitution at run time instead of compile time of the
environment variable name prefix.

Create a generalized exec hook that different architectures can hook
into if they need to, but default to no action.

If md_exec_hook is defined, provide a way to create the strings
for the environment variables we look up at runtime. Otherwise,
there's no way they will change, optimize it at compile time.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

imp retitled this revision from to Implement special handling of soft-float binaries..
imp updated this object.
imp edited the test plan for this revision. (Show Details)

I am the wrong person to validate the code, however I am happy to do any testing (buildtime, runtime, etc) testing required.

Just let me know what you (all) need from me.

libexec/rtld-elf/arm/rtld_machdep.h
80

exec_hook is quite non-telling name. If reflecting the purpose, it should be e.g. abi_hook. If explaining the actions, it could be path_select_hook.

libexec/rtld-elf/libmap.c
19

You added #include :"paths.h" into rtld.h. Either this include, or the one in rtld.h, is excessive. I would prefer to keep this and remove the include statement from the rtld.h. rtld.h is used by external code, which has no deal with LD stuff.

libexec/rtld-elf/paths.h
68

extern const ?

libexec/rtld-elf/rtld.c
208

Please provide empty XXX_hook in the corresponding arch/rtld_machdep.h.

263

Again, const.

Do all of the variables need to be extern ? libmap_conf is used in libmap.c, can it be moved there ? My feeling is that all vars could become static if properly located.

343

return (buffer);

346

Wouldn't something like "LD_" #x or "LD_" __XSTRING(x) be better ?

Thanks for the review Konstantin. I'll update, retest and re-upload...

libexec/rtld-elf/arm/rtld_machdep.h
80

I like abi_hook better. path_select_hook seems overly specific. abi isn't quite right either, but I can't think of anything that isn't mosterously wrong. Maybe abi_variant_hook() would be a good length, and properly descriptive.

libexec/rtld-elf/libmap.c
19

Good catch. I'll remove it from rtld.h and mop up the fallout.

libexec/rtld-elf/paths.h
68

agreed.

libexec/rtld-elf/rtld.c
208

Doing that messes up seeing if this hook is defined and optimizing the LD_ macro / function better.
Although it looks like I messed this up by moving the definition of the LD_ macro down below where md_exec_hook is always defined.

263

Since we are setting it (or should be setting it) in the arch dependent code, locating it in one file becomes difficult without some kind of additional API to set it, which seems more effort than the gain of having it be global vs static.
Agreed on the const bit.

346

It would be if these were identifiers. However, since x is a literal string that's passed into this macro, the string pasting happens automatically. I'm sure I see how either of your suggestions could work.

libexec/rtld-elf/arm/reloc.c
25

kenrel?

libexec/rtld-elf/arm/rtld_machdep.h
80

I like abi_variant.

libexec/rtld-elf/rtld.c
263

Ok.

346

Single-hash is defined to convert token to "a single character string literal preprocessing token that contains the spelling of the preprocessing token sequence for the corresponding argument".

It is double-hash that concatenates symbols.

libexec/rtld-elf/arm/reloc.c
25

Sigh. I've made that typo now for over two decades, though not as much as in my younger career. I got bit several times installing kenrel.new but booting kernel.new and assuming my changes were good...

imp edited edge metadata.

Update all my changes...

  • imported patch exception
  • Move all the paths into a new path.h to centralize them.
  • Rather than using the #define for path names, indirect through a char *
  • Use a macro to create the names for the library path names. This will
  • Create a generalized exec hook that different architectures can hook
  • If md_exec_hook is defined, provide a way to create the strings

Changes to sys/arm/arm/exception.S cannot be related to the rtld.

Mostly the rtld chunks looks fine, modulo the small nits I pointed out.

libexec/rtld-elf/arm/reloc.c
26

It is still keNRel.

37

if ((ehdr & EF_ARM_VFP_FLOAT) != 0)

libexec/rtld-elf/paths.h
6

Marino certainly can be excluded from the copyright owners list. His contribution to rtld.c was the GNU hash algorithm.

I probably can be excluded as well.

libexec/rtld-elf/rtld.c
341

return (buffer);

Oh, I already commented on this.

imp marked 3 inline comments as done.Dec 16 2015, 5:01 AM

Given the last changes were minor, I'm going to commit this once I do another final round of testing to make sure things haven't broken in the last few months. I doubt they have.

libexec/rtld-elf/arm/reloc.c
26

Fixed.

37

fixed.

libexec/rtld-elf/paths.h
6

Done.

Warner:

Is this going to hit the tree soonish?

imp marked 3 inline comments as done.Jan 30 2016, 3:42 AM

sbruno: It's in.

Hrm, this review is still open. Do you want to close it with the svn revision that it was committed?

sbruno edited edge metadata.

this should have been closed with commit R293066 or 292810 ... but phabricator.

stlgtm

This revision is now accepted and ready to land.Apr 11 2016, 4:58 PM