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>
Differential D24918
Add load offset value to struct link_map. kib on May 19 2020, 2:42 PM. Authored by Tags None Referenced Files
Details
Do it in ABI-compatible way, we only return pointers to the structure or to the list of structures. PR: 246561
Diff Detail
Event Timeline
Comment Actions I do wonder what the impact of changing l_addr would actually be
Comment Actions 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). Comment Actions 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. Comment Actions 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. Comment Actions 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. Comment Actions 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. Comment Actions 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. :( Comment Actions 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. Comment Actions 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.
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. |