Page MenuHomeFreeBSD

Add load offset value to struct link_map.
ClosedPublic

Authored by kib on May 19 2020, 2:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 8 2024, 5:33 AM
Unknown Object (File)
Dec 27 2023, 8:17 PM
Unknown Object (File)
Dec 20 2023, 7:30 AM
Unknown Object (File)
Dec 10 2023, 9:10 AM
Unknown Object (File)
Dec 7 2023, 2:40 AM
Unknown Object (File)
Nov 5 2023, 7:35 AM
Unknown Object (File)
Oct 4 2023, 7:32 AM
Unknown Object (File)
Sep 17 2023, 6:51 AM
Subscribers

Details

Summary

Do it in ABI-compatible way, we only return pointers to the structure or to the list of structures.
Except on MIPS, instread ignore ABI compat for the arch,

PR: 246561
Requested by: Damjan Jovanovic <damjan.jov@gmail.com>

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 31194

Event Timeline

kib requested review of this revision.May 19 2020, 2:42 PM
cem added a subscriber: cem.
cem added inline comments.
sys/sys/link_elf.h
61โ€“63

ABI-breaking on MIPS, although I am not especially concerned about that.

This revision is now accepted and ready to land.May 19 2020, 3:54 PM

I do wonder what the impact of changing l_addr would actually be

sys/sys/link_elf.h
61โ€“63

I agree that's fine for a tier-2 or lower arch.

I actually think we would be fine to fix l_addr itself. GDB doesn't use l_offs, and this will break GDB on MIPS since it knows about the layout of struct link_map already. As noted in the bug though, GDB expects l_addr to be relocbase. It only happens to work on FreeBSD currently because all of our shared libraries are linked at an address of 0 so mapbase == relocbase. If we ever wanted to do pre-linking we'd have to fix this for GDB (and presumably lldb) to find shared libraries. Right now it does mean l_addr is "wrong" for binaries, but GDB doesn't use the link_map for those. Not sure if this would affect PIE debugging on FreeBSD (i.e. is PIE debugging currently broken and would fixing l_addr fix PIE debugging? I haven't tried).

In D24918#548517, @jhb wrote:

I actually think we would be fine to fix l_addr itself. GDB doesn't use l_offs, and this will break GDB on MIPS since it knows about the layout of struct link_map already. As noted in the bug though, GDB expects l_addr to be relocbase. It only happens to work on FreeBSD currently because all of our shared libraries are linked at an address of 0 so mapbase == relocbase. If we ever wanted to do pre-linking we'd have to fix this for GDB (and presumably lldb) to find shared libraries. Right now it does mean l_addr is "wrong" for binaries, but GDB doesn't use the link_map for those. Not sure if this would affect PIE debugging on FreeBSD (i.e. is PIE debugging currently broken and would fixing l_addr fix PIE debugging? I haven't tried).

I do not agree with breaking ABI, on principle. Even if gdb does not use the field that way, it is still there from 4.x times. I perhaps agree to rename the new field to l_addr, and invent some other name for current l_addr, say l_base.

This revision now requires review to proceed.May 19 2020, 5:38 PM

This seems not useful, and still breaks GDB for MIPS such that I have to figure out the version of rtld (somehow) for both core dumps and live processes in the MIPS GDB backend to determine which layout of link_map to use (which I'd really rather avoid). What actual, real, software depends on l_addr being mapbase instead of relocbase? It won't be an ABI change for FreeBSD shared libraries because we link them all at 0, and it will match what other OS's do making it more likely that we will work with other portable software.

Note that while documentation for l_addr is pretty scant (no mention in psABI), what does exist does seem to suggest it being an offset: https://sourceware.org/gdb/current/onlinedocs/gdb/Library-List-Format-for-SVR4-Targets.html

lldb is a bit of a mystery code-wise, but it's "POSIXDyld" class does assume that the "base address is an offset" rather than an absolute address based on a boolean it passes as "true" when calling into the generic "DynamicLoader" code. Certainly it uses the same code for FreeBSD, NetBSD, and Linux, so it seems most likely it is also assuming relocbase.

libunwind uses dl_iterate_phdr() instead so doesn't run into this.

To be clear, I don't view changing 'l_addr' as breaking the ABI. I view it as fixing a bug where we happen to be incompatible with the rest of the world, but just got lucky since for the majority of use cases (shared libraries and rtld) the two values are identical.

In D24918#548561, @jhb wrote:

To be clear, I don't view changing 'l_addr' as breaking the ABI. I view it as fixing a bug where we happen to be incompatible with the rest of the world, but just got lucky since for the majority of use cases (shared libraries and rtld) the two values are identical.

For me it does qualify as ABI breakage. And the fact that it was noticed supports this view, even if the note is that the current behavior is considered incompatible with other systems.

Well, if you prefer to have it 'fixed' instead of providing ABI-compatible extension of the struct link_map layout, let it be so. But what I do not want to keep is the l_offs for mips, both because it is not duplicates other field, and because mips is tier 2. If you still want to have gdb working with both old and new rtld, I can add an exported symbol to rtld, which presence would indicate the new layout.

Change semantic of l_addr instead.

Some kind of symbol would be needed for GDB. It just means more work for me for an architecture that will probably get removed in 14 anyway. I think it would be fine to leave the field with an 'XXX: Unused comment' but remove the code that sets it from rtld if that were ok with you. That would avoid needing to patch GDB, but would remove setting it twice in rtld (since GDB doesn't actually use l_offs despite the comment's claim). I wonder if Juniper actually added l_offs as a hack due to the issue of l_addr being mapbase and that hack just got imported from Juniper but their private GDB hack to make use of it it didn't? Linux doesn't have l_offs for MIPS, so it seems to be a local invention in FreeBSD. :(

In D24918#548726, @jhb wrote:

Some kind of symbol would be needed for GDB. It just means more work for me for an architecture that will probably get removed in 14 anyway. I think it would be fine to leave the field with an 'XXX: Unused comment' but remove the code that sets it from rtld if that were ok with you. That would avoid needing to patch GDB, but would remove setting it twice in rtld (since GDB doesn't actually use l_offs despite the comment's claim). I wonder if Juniper actually added l_offs as a hack due to the issue of l_addr being mapbase and that hack just got imported from Juniper but their private GDB hack to make use of it it didn't? Linux doesn't have l_offs for MIPS, so it seems to be a local invention in FreeBSD. :(

Why do you say that mips going to be removed ?

That said, why not use the tier-2 argument and change the layout. You would need to rebuild gdb to debug. IMO potential impact of changing semantic for l_addr is higher than need to rebuild gdb on mips.

I restored l_offs for now, as l_xxx. Lets make the discussion of removing it separate.

Restore l_offs (as l_xxx), since it is reported as unused.

This revision is now accepted and ready to land.May 20 2020, 2:36 PM
In D24918#548727, @kib wrote:
In D24918#548726, @jhb wrote:

Some kind of symbol would be needed for GDB. It just means more work for me for an architecture that will probably get removed in 14 anyway. I think it would be fine to leave the field with an 'XXX: Unused comment' but remove the code that sets it from rtld if that were ok with you. That would avoid needing to patch GDB, but would remove setting it twice in rtld (since GDB doesn't actually use l_offs despite the comment's claim). I wonder if Juniper actually added l_offs as a hack due to the issue of l_addr being mapbase and that hack just got imported from Juniper but their private GDB hack to make use of it it didn't? Linux doesn't have l_offs for MIPS, so it seems to be a local invention in FreeBSD. :(

Why do you say that mips going to be removed ?

Cambridge will stop using it in another year or so due to all of their work shifting over to RISC-V. Without Cambridge supporting MIPS I suspect it will have very little active support in the developer community.

That said, why not use the tier-2 argument and change the layout. You would need to rebuild gdb to debug. IMO potential impact of changing semantic for l_addr is higher than need to rebuild gdb on mips.

Hmm, I suppose I could just change gdb's assumption, but it is a bit annoying that you won't be able to get symbols for shared libraries if you have an old core dump. Note that most of the time I've used gdb for mips it's been cross debugging, and often using a gdb built on 12.x/amd64 to debug cores from current, etc. I could change gdb so it only works for head but would no longer be able to debug cores on 12, but you can't have it do both without some kind of dynamic decision in GDB in terms of which layout of 'struct link_map' to use.

This revision was automatically updated to reflect the committed changes.