Page MenuHomePhabricator

[lldb] [POSIX-DYLD] Add libraries from initial rendezvous brkpt hit
ClosedPublic

Authored by mgorny on Nov 26 2020, 8:56 AM.

Details

Summary

Explicitly consider the libraries reported on the initial rendezvous
breakpoint hit added. This is necessary on FreeBSD since the dynamic
loader issues only a single 'consistent' state rendezvous breakpoint hit
for all the libraries present in DT_NEEDED. It is also helpful on Linux
where it ensures that ld-linux is considered loaded as well
as the shared system libraries reported afterwards.

Reenable memory maps on FreeBSD since this fixed the issue triggered
by them.

Diff Detail

Event Timeline

mgorny created this revision.Nov 26 2020, 8:56 AM
mgorny requested review of this revision.Nov 26 2020, 8:56 AM

What's the state of TestBreakpointInGlobalConstructor.py after this change? Does it continue to fail?

If so, then this is probably a bug in FreeBSD that is worth reporting, as it means that it is not possible to debug the initialization code (global constructors, __attribute__((init)), etc.) of libraries loaded at startup (DT_NEEDED). It might be also worth checking what gdb does in this case. If gdb is able to break in this code, then it means that there is a different way to fetch the loaded libraries early enough, and we could possibly emulate that... (I would very much love that, as the current approach is pretty hacky already)

What's the state of TestBreakpointInGlobalConstructor.py after this change? Does it continue to fail?

Yes, it does.

If so, then this is probably a bug in FreeBSD that is worth reporting, as it means that it is not possible to debug the initialization code (global constructors, __attribute__((init)), etc.) of libraries loaded at startup (DT_NEEDED). It might be also worth checking what gdb does in this case. If gdb is able to break in this code, then it means that there is a different way to fetch the loaded libraries early enough, and we could possibly emulate that... (I would very much love that, as the current approach is pretty hacky already)

I will check that.

GDB indeed handles it somehow. I'm going to investigate.

emaste added a subscriber: bsdjhb.Nov 30 2020, 7:49 AM

I'm curious how gdb handles this, and asked @bsdjhb if he knows off hand.

Is there a brief description of how this works on Linux and/or NetBSD?

krytarowski added a comment.EditedNov 30 2020, 7:53 AM

I'm curious how gdb handles this, and asked @bsdjhb if he knows off hand.

Is there a brief description of how this works on Linux and/or NetBSD?

We are working on researching this, NetBSD, Linux and FreeBSD should behave almost in the same way. At least GNU gdbserver reused the Linux code on NetBSD practically as-is. LLDB does not work with r_debug on NetBSD either.

One thing that FreeBSD should do, is to upgrade to the protocol version 1 (stored in r_version), like Linux, NetBSD and OpenBSD.

This comment was removed by emaste.

One thing that FreeBSD should do, is to upgrade to the protocol version 1 (stored in r_version), like Linux, NetBSD and OpenBSD.

It looks like Linux has always used r_version=1 (since 1995).

AFAICT we can just add r_ldbase and set version to 1.
See https://reviews.freebsd.org/D27429

One thing that FreeBSD should do, is to upgrade to the protocol version 1 (stored in r_version), like Linux, NetBSD and OpenBSD.

It looks like Linux has always used r_version=1 (since 1995).

AFAICT we can just add r_ldbase and set version to 1.
See https://reviews.freebsd.org/D27429

I propose to add #define R_DEBUG_VERSION 1 too, to keep compat with NetBSD (and SunOS?).

mgorny added a comment.Dec 1 2020, 4:05 AM

I've added some more debug as requested, and it confirmed that Linux and FreeBSD dyld are behaving differently here.

Linux triggers the breakpoint twice: first time in add state, including only /lib64/ld-linux-x86-64.so.2 and linux-vdso.so.1 in module list, and the second time in consistent state, adding soentries for all shared libraries.

FreeBSD triggers it only once, in consistent state and the remote list includes all libraries immediately.

mgorny added a comment.Dec 1 2020, 4:06 AM

Full logs below.

Linux:

(lldb) log enable lldb dyld
(lldb) run
lldb             DynamicLoaderDarwin::UseDYLDSPI: Use old DynamicLoader plugin
lldb             DynamicLoaderDarwin::UseDYLDSPI: Use old DynamicLoader plugin
lldb             DYLDRendezvous::DYLDRendezvous exe module executable path set: '/home/mgorny/git/llvm-project/_build/a.out'
lldb             DynamicLoaderPOSIXDYLD::DidLaunch()
lldb             DynamicLoaderPOSIXDYLD::DidLaunch about to call ProbeEntry()
lldb             Rendezvous structure is not set up yet. Trying to locate rendezvous breakpoint in the interpreter by symbol name.
lldb             Successfully set rendezvous breakpoint at address 0x7ffff7fe1850 for pid 221689
intern-state     DynamicLoaderPOSIXDYLD::RendezvousBreakpointHit called for pid 221689
intern-state     DYLDRendezvous::Resolve address size: 8, padding 4
intern-state     ResolveRendezvousAddress info_location = 0xffffffffffffffff
intern-state     ResolveRendezvousAddress resolved via direct object file approach to 0x403ee8
intern-state     ResolveRendezvousAddress reading pointer (8 bytes) from 0x403ee8
intern-state     DYLDRendezvous::Resolve cursor = 0x7ffff7ffe0a0
intern-state     DYLDRendezvous:
intern-state        Address: 7ffff7ffe0a0
intern-state        Version: 1
intern-state        Link   : 7ffff7ffe1a0
intern-state        Break  : 7ffff7fe1850
intern-state        LDBase : 7ffff7fd1000
intern-state        State  : add
intern-state     DYLDRendezvous::UpdateSOEntriesFromRemote action = 1

intern-state     DYLDRendezvous::SaveSOEntriesFromRemote soentry from module list: link_addr=7ffff7ffda08, base_addr=7ffff7fd1000, dyn_addr=7ffff7ffce28, filename=/lib64/ld-linux-x86-64.so.2
intern-state     DYLDRendezvous::SaveSOEntriesFromRemote soentry from module list: link_addr=7ffff7ffe750, base_addr=7ffff7fcf000, dyn_addr=7ffff7fcf3a0, filename=linux-vdso.so.1
intern-state     DynamicLoaderPOSIXDYLD::RendezvousBreakpointHit pid 221689 stop_when_images_change=false
intern-state     DynamicLoaderPOSIXDYLD::RendezvousBreakpointHit called for pid 221689
intern-state     DYLDRendezvous::Resolve address size: 8, padding 4
intern-state     DYLDRendezvous::Resolve cursor = 0x7ffff7ffe0a0
intern-state     DYLDRendezvous:
intern-state        Address: 7ffff7ffe0a0
intern-state        Version: 1
intern-state        Link   : 7ffff7ffe1a0
intern-state        Break  : 7ffff7fe1850
intern-state        LDBase : 7ffff7fd1000
intern-state        State  : consistent
intern-state     DYLDRendezvous SOEntries:
intern-state     
   SOEntry [1] /lib64/ld-linux-x86-64.so.2
intern-state           Base : 7ffff7fd1000
intern-state           Path : 0
intern-state           Dyn  : 7ffff7ffce28
intern-state           Next : 0
intern-state           Prev : 0
intern-state     
   SOEntry [2] linux-vdso.so.1
intern-state           Base : 7ffff7fcf000
intern-state           Path : 0
intern-state           Dyn  : 7ffff7fcf3a0
intern-state           Next : 0
intern-state           Prev : 0
intern-state     DYLDRendezvous::UpdateSOEntriesFromRemote action = 2

intern-state     DYLDRendezvous::AddSOEntriesFromRemote add soentry: link_addr=7ffff7f8c000, base_addr=7ffff7f4f000, dyn_addr=7ffff7f86cb0, filename=/lib64/libedit.so.0
intern-state     DYLDRendezvous::AddSOEntriesFromRemote add soentry: link_addr=7ffff7f8c4f0, base_addr=7ffff7d88000, dyn_addr=7ffff7f48b60, filename=/lib64/libc.so.6
intern-state     DYLDRendezvous::AddSOEntriesFromRemote add soentry: link_addr=7ffff7f8c9e0, base_addr=7ffff7d4c000, dyn_addr=7ffff7d85cc8, filename=/lib64/libtinfo.so.6
intern-state     DynamicLoaderPOSIXDYLD::RendezvousBreakpointHit pid 221689 stop_when_images_change=false
Hello, world!
Process 221689 exited with status = 0 (0x00000000) 

Process 221689 launched: '/home/mgorny/git/llvm-project/_build/a.out' (x86_64)

FreeBSD:

(lldb) run
lldb             DynamicLoaderDarwin::UseDYLDSPI: Use old DynamicLoader plugin
lldb             DynamicLoaderDarwin::UseDYLDSPI: Use old DynamicLoader plugin
lldb             DYLDRendezvous::DYLDRendezvous exe module executable path set: '/home/mgorny/llvm-project/_build/a.out'
lldb             DynamicLoaderPOSIXDYLD::DidLaunch()
lldb             DynamicLoaderPOSIXDYLD::DidLaunch about to call ProbeEntry()
lldb             Rendezvous structure is not set up yet. Trying to locate rendezvous breakpoint in the interpreter by symbol name.
lldb             Successfully set rendezvous breakpoint at address 0x80020fe20 for pid 3770
intern-state     DynamicLoaderPOSIXDYLD::RendezvousBreakpointHit called for pid 3770
intern-state     DYLDRendezvous::Resolve address size: 8, padding 4
intern-state     ResolveRendezvousAddress info_location = 0xffffffffffffffff
intern-state     ResolveRendezvousAddress resolved via direct object file approach to 0x202a50
intern-state     ResolveRendezvousAddress reading pointer (8 bytes) from 0x202a50
intern-state     DYLDRendezvous::Resolve cursor = 0x800229900
intern-state     DYLDRendezvous:
intern-state        Address: 800229900
intern-state        Version: 0
intern-state        Link   : 80022e240
intern-state        Break  : 80020fe20
intern-state        LDBase : 3
intern-state        State  : consistent
intern-state     DYLDRendezvous::UpdateSOEntriesFromRemote action = 1

intern-state     DYLDRendezvous::SaveSOEntriesFromRemote soentry from module list: link_addr=80022e240, base_addr=200000, dyn_addr=202a28, filename=/home/mgorny/llvm-project/_build/a.out
intern-state     DYLDRendezvous::SaveSOEntriesFromRemote soentry from module list: link_addr=80022e640, base_addr=80024c000, dyn_addr=8002815c0, filename=/lib/libedit.so.7
intern-state     DYLDRendezvous::SaveSOEntriesFromRemote soentry from module list: link_addr=80022ea40, base_addr=800287000, dyn_addr=80044f058, filename=/lib/libc.so.7
intern-state     DYLDRendezvous::SaveSOEntriesFromRemote soentry from module list: link_addr=80022ee40, base_addr=80067d000, dyn_addr=8006db830, filename=/lib/libncursesw.so.8
intern-state     DYLDRendezvous::SaveSOEntriesFromRemote soentry from module list: link_addr=800229580, base_addr=800203000, dyn_addr=800227af0, filename=/libexec/ld-elf.so.1
intern-state     DynamicLoaderPOSIXDYLD::RendezvousBreakpointHit pid 3770 stop_when_images_change=false
Hello world
Process 3770 exited with status = 0 (0x00000000) 

Process 3770 launched: '/home/mgorny/llvm-project/_build/a.out' (x86_64)

(the test program links to libc and libedit, the latter links to libtinfo/libncursesw appropriately)

labath added a comment.Dec 1 2020, 5:17 AM

I've added some more debug as requested, and it confirmed that Linux and FreeBSD dyld are behaving differently here.

Linux triggers the breakpoint twice: first time in add state, including only /lib64/ld-linux-x86-64.so.2 and linux-vdso.so.1 in module list, and the second time in consistent state, adding soentries for all shared libraries.

FreeBSD triggers it only once, in consistent state and the remote list includes all libraries immediately

Thanks for investigating this. And I'm sorry for being so picky -- this is all very messy, and I am trying to understand things to avoid making an even bigger mess.

When you say "FreeBSD triggers it only once", which breakpoint are you referring to? The "rendezvous" breakpoint? Judging by the log's, I would say yes. But if that's true, then how does the "entry" breakpoint fit into this picture (IIUC, your change basically forces setting of the entry breakpoint on freebsd)? The question I'm looking to answer is whether we really need to set the entry breakpoint, or we just need some side-effect of what happens when we process that breakpoint hit (and we could achieve that differently).

I'm not saying that setting the entry breakpoint is a bad idea -- that mechanism seems more canonical than searching for some hardcoded symbol name -- but in order to decide that, we need to know what actually goes on in there...

Ideally we should iterate over the startup process and investigate the state of the r_debug structure. Once it gets initialized, set the software brakpoint to r_brk and keep track of the dynamic loading and unloading of libraries. The tricky part is to detect the right moment to plug into r_brk, early enough in the startup process before calling constructors, loading the libraries etc and late enough to become initialized.

Right now we place the breakpoints unconditionally, hardcoding the linker name, mostly ignoring the r_brk, but in practice it's not clear whether it's the simplest and sufficient approach.

mgorny added a comment.Dec 1 2020, 8:07 AM

I've added some more debug as requested, and it confirmed that Linux and FreeBSD dyld are behaving differently here.

Linux triggers the breakpoint twice: first time in add state, including only /lib64/ld-linux-x86-64.so.2 and linux-vdso.so.1 in module list, and the second time in consistent state, adding soentries for all shared libraries.

FreeBSD triggers it only once, in consistent state and the remote list includes all libraries immediately

Thanks for investigating this. And I'm sorry for being so picky -- this is all very messy, and I am trying to understand things to avoid making an even bigger mess.

When you say "FreeBSD triggers it only once", which breakpoint are you referring to? The "rendezvous" breakpoint? Judging by the log's, I would say yes.

Yes.

But if that's true, then how does the "entry" breakpoint fit into this picture (IIUC, your change basically forces setting of the entry breakpoint on freebsd)? The question I'm looking to answer is whether we really need to set the entry breakpoint, or we just need some side-effect of what happens when we process that breakpoint hit (and we could achieve that differently).

It's not really about setting the breakpoint (it actually doesn't get set again because it's set already) but about registering the loaded libraries. I've presumed there's no harm in reusing the existing function as-is, i.e. issuing the unnecessary breakpoint call.

However, as I've noted on iRC, assuming the libraries reported on first rendezvous breakpoint to be 'added' would be a better solution. Most importantly, it fixes handling breakpoints in global constructors. I mean something like this:

diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
index 866acbddbdc..ad696995f9a 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -296,8 +296,10 @@ bool DYLDRendezvous::SaveSOEntriesFromRemote(
       return false;
 
     // Only add shared libraries and not the executable.
-    if (!SOEntryIsMainExecutable(entry))
+    if (!SOEntryIsMainExecutable(entry)) {
+      m_added_soentries.push_back(entry);
       m_soentries.push_back(entry);
+    }
   }
 
   m_loaded_modules = module_list;

I would appreciate any advice how to turn that hack into a proper-ish solution.

mgorny updated this revision to Diff 309587.Dec 4 2020, 11:14 AM
mgorny retitled this revision from [lldb] [FreeBSD] Fix establishing DT_NEEDED libraries from dyld to [lldb] [POSIX-DYLD] Add libraries from initial eTakeSnapshot action.
mgorny edited the summary of this revision. (Show Details)

Updated as discussed on IRC, to instead add libs returned by the first 'take snapshot' action.

labath accepted this revision.Dec 7 2020, 12:05 AM

Awesome. Let's ship it.

This revision is now accepted and ready to land.Dec 7 2020, 12:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 7 2020, 12:57 AM
mgorny reopened this revision.Dec 7 2020, 7:18 AM

This causes ld-linux to be added twice on Linux. I need to change the logic a bit.

This revision is now accepted and ready to land.Dec 7 2020, 7:18 AM
mgorny updated this revision to Diff 309950.Dec 7 2020, 10:33 AM
mgorny retitled this revision from [lldb] [POSIX-DYLD] Add libraries from initial eTakeSnapshot action to [lldb] [POSIX-DYLD] Add libraries from initial rendezvous brkpt hit.
mgorny edited the summary of this revision. (Show Details)

Updated to perform the logic inside DynamicLoaderPOSIXDYLD, without touching DYLDRendezvous. This is much safer, and does not cause the double-load of ld-linux.

labath accepted this revision.Dec 10 2020, 12:51 AM

Actually, I can still reproduce the double symbol problem with Arch's ld-2.32.so. I can't figure out what's wrong.

One possible hackaround would be to do this only if 1st breakpoint hit is consistent (vs add on Linux), or just make the whole logic #if defined(__FreeBSD__).

mgorny updated this revision to Diff 310877.Dec 10 2020, 6:57 AM

Avoid adding duplicate entry for ld-linux.

mgorny updated this revision to Diff 311191.Dec 11 2020, 5:47 AM

Updated to unload duplicate ld.so as suggested by @labath.

mgorny updated this revision to Diff 311412.Dec 12 2020, 2:14 PM

Another fix: we should only unload duplicate ld.so if it's actually a duplicate, i.e. the path differs. Otherwise, we've ended up unloading the only copy.

Another fix: we should only unload duplicate ld.so if it's actually a duplicate, i.e. the path differs. Otherwise, we've ended up unloading the only copy.

Would comparing the module shared_pointers work? I think that's exactly what we want here. I think it's possible for LoadModuleAtAddress to return the same module, even with paths which don't match exactly. But if it does that, then we will still end up unloading the original interpreter module...

Another fix: we should only unload duplicate ld.so if it's actually a duplicate, i.e. the path differs. Otherwise, we've ended up unloading the only copy.

Would comparing the module shared_pointers work? I think that's exactly what we want here. I think it's possible for LoadModuleAtAddress to return the same module, even with paths which don't match exactly. But if it does that, then we will still end up unloading the original interpreter module...

How do you suggest we get the shared pointer for comparison? Store in class instance, or search for it somehow?

Another fix: we should only unload duplicate ld.so if it's actually a duplicate, i.e. the path differs. Otherwise, we've ended up unloading the only copy.

Would comparing the module shared_pointers work? I think that's exactly what we want here. I think it's possible for LoadModuleAtAddress to return the same module, even with paths which don't match exactly. But if it does that, then we will still end up unloading the original interpreter module...

How do you suggest we get the shared pointer for comparison? Store in class instance, or search for it somehow?

Store in the class (probably as a weak_ptr), just like you did with the path....

mgorny updated this revision to Diff 311552.Dec 14 2020, 4:46 AM

Compare using weak_ptr to the interpreter module.

labath accepted this revision.Dec 17 2020, 12:03 AM

Let's give this another shot.