diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h @@ -21,6 +21,7 @@ using lldb_private::LoadedModuleInfoList; namespace lldb_private { +class Log; class Process; } @@ -28,9 +29,81 @@ /// Interface to the runtime linker. /// /// A structure is present in a processes memory space which is updated by the -/// runtime liker each time a module is loaded or unloaded. This class +/// dynamic linker each time a module is loaded or unloaded. This class /// provides an interface to this structure and maintains a consistent /// snapshot of the currently loaded modules. +/// +/// In the dynamic loader sources, this structure has a type of "r_debug" and +/// the name of the structure us "_r_debug". The structure looks like: +/// +/// struct r_debug { +/// // Version number for this protocol. +/// int r_version; +/// // Head of the chain of loaded objects. +/// struct link_map *r_map; +/// // The address the debugger should set a breakpoint at in order to get +/// // notified when shared libraries are added or removed +/// uintptr_t r_brk; +/// // This state value describes the mapping change taking place when the +/// // 'r_brk' address is called. +/// enum { +/// RT_CONSISTENT, // Mapping change is complete. +/// RT_ADD, // Beginning to add a new object. +/// RT_DELETE, // Beginning to remove an object mapping. +/// } r_state; +/// // Base address the linker is loaded at. +/// uintptr_t r_ldbase; +/// }; +/// +/// The dynamic linker then defines a global variable using this type named +/// "_r_debug": +/// +/// r_debug _r_debug; +/// +/// The DYLDRendezvous class defines a local version of this structure named +/// DYLDRendezvous::Rendezvous. See the definition inside the class definition +/// for DYLDRendezvous. +/// +/// This structure can be located by looking through the .dynamic section in +/// the main executable and finding the DT_DEBUG tag entry. This value starts +/// out with a value of zero when the program first is initially loaded, but +/// the address of the "_r_debug" structure from ld.so is filled in by the +/// dynamic loader during program initialization code in ld.so prior to loading +/// or unloading and shared libraries. +/// +/// The dynamic loader will update this structure as shared libraries are +/// loaded and will call a specific function that LLDB knows to set a +/// breakpoint on (from _r_debug.r_brk) so LLDB will find out when shared +/// libraries are loaded or unloaded. Each time this breakpoint is hit, LLDB +/// looks at the contents of this structure and the contents tell LLDB what +/// needs to be done. +/// +/// Currently we expect the "state" in this structure to change as things +/// happen. +/// +/// When any shared libraries are loaded the following happens: +/// - _r_debug.r_map is updated with the new shared libraries. This is a +/// doubly linked list of "link_map *" entries. +/// - _r_debug.r_state is set to RT_ADD and the debugger notification +/// function is called notifying the debugger that shared libraries are +/// about to be added, but are not yet ready for use. +/// - Once the the shared libraries are fully loaded, _r_debug.r_state is set +/// to RT_CONSISTENT and the debugger notification function is called again +/// notifying the debugger that shared libraries are ready for use. +/// DYLDRendezvous must remember that the previous state was RT_ADD when it +/// receives a RT_CONSISTENT in order to know to add libraries +/// +/// When any shared libraries are unloaded the following happens: +/// - _r_debug.r_map is updated and the unloaded libraries are removed. +/// - _r_debug.r_state is set to RT_DELETE and the debugger notification +/// function is called notifying the debugger that shared libraries are +/// about to be removed. +/// - Once the the shared libraries are removed _r_debug.r_state is set to +/// RT_CONSISTENT and the debugger notification function is called again +/// notifying the debugger that shared libraries have been removed. +/// DYLDRendezvous must remember that the previous state was RT_DELETE when +/// it receives a RT_CONSISTENT in order to know to remove libraries +/// class DYLDRendezvous { // This structure is used to hold the contents of the debug rendezvous @@ -45,6 +118,8 @@ lldb::addr_t ldbase = 0; Rendezvous() = default; + + void DumpToLog(lldb_private::Log *log, const char *label); }; /// Locates the address of the rendezvous structure. It updates @@ -126,8 +201,15 @@ /// Constants describing the state of the rendezvous. /// + /// These values are defined to match the r_debug.r_state enum from the + /// actual dynamic loader sources. + /// /// \see GetState(). - enum RendezvousState { eConsistent, eAdd, eDelete }; + enum RendezvousState { + eConsistent, // RT_CONSISTENT + eAdd, // RT_ADD + eDelete // RT_DELETE + }; /// Structure representing the shared objects currently loaded into the /// inferior process. @@ -276,6 +358,9 @@ eRemoveModules }; + static const char *StateToCStr(RendezvousState state); + static const char *ActionToCStr(RendezvousAction action); + /// Returns the current action to be taken given the current and previous /// state RendezvousAction GetAction() const; diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp @@ -25,6 +25,32 @@ using namespace lldb; using namespace lldb_private; +const char *DYLDRendezvous::StateToCStr(RendezvousState state) { + switch (state) { + case DYLDRendezvous::eConsistent: + return "eConsistent"; + case DYLDRendezvous::eAdd: + return "eAdd"; + case DYLDRendezvous::eDelete: + return "eDelete"; + } + return ""; +} + +const char *DYLDRendezvous::ActionToCStr(RendezvousAction action) { + switch (action) { + case DYLDRendezvous::RendezvousAction::eTakeSnapshot: + return "eTakeSnapshot"; + case DYLDRendezvous::RendezvousAction::eAddModules: + return "eAddModules"; + case DYLDRendezvous::RendezvousAction::eRemoveModules: + return "eRemoveModules"; + case DYLDRendezvous::RendezvousAction::eNoAction: + return "eNoAction"; + } + return ""; +} + DYLDRendezvous::DYLDRendezvous(Process *process) : m_process(process), m_rendezvous_addr(LLDB_INVALID_ADDRESS), m_executable_interpreter(false), m_current(), m_previous(), @@ -129,6 +155,13 @@ } } +void DYLDRendezvous::Rendezvous::DumpToLog(Log *log, const char *label) { + LLDB_LOGF(log, "%s Rendezvous: version = %" PRIu64 ", map_addr = 0x%16.16" + PRIx64 ", brk = 0x%16.16" PRIx64 ", state = %" PRIu64 + " (%s), ldbase = 0x%16.16" PRIx64, label ? label : "", version, + map_addr, brk, state, StateToCStr((RendezvousState)state), ldbase); +} + bool DYLDRendezvous::Resolve() { Log *log = GetLog(LLDBLog::DynamicLoader); @@ -176,6 +209,9 @@ m_previous = m_current; m_current = info; + m_previous.DumpToLog(log, "m_previous"); + m_current.DumpToLog(log, "m_current "); + if (m_current.map_addr == 0) return false; @@ -217,6 +253,75 @@ break; case eAdd: + // If the main executable or a shared library defines a publicly visible + // symbol named "_r_debug", then it will cause problems once the executable + // that contains the symbol is loaded into the process. The correct + // "_r_debug" structure is currently found by LLDB by looking through + // the .dynamic section in the main executable and finding the DT_DEBUG tag + // entry. + // + // An issue comes up if someone defines another publicly visible "_r_debug" + // struct in their program. Sample code looks like: + // + // #include + // r_debug _r_debug; + // + // If code like this is in an executable or shared library, this creates a + // new "_r_debug" structure and it causes problems once the executable is + // loaded due to the way symbol lookups happen in linux: the shared library + // list from _r_debug.r_map will be searched for a symbol named "_r_debug" + // and the first match will be the new version that is used. The dynamic + // loader is always last in this list. So at some point the dynamic loader + // will start updating the copy of "_r_debug" that gets found first. The + // issue is that LLDB will only look at the copy that is pointed to by the + // DT_DEBUG entry, or the initial version from the ld.so binary. + // + // Steps that show the problem are: + // + // - LLDB finds the "_r_debug" structure via the DT_DEBUG entry in the + // .dynamic section and this points to the "_r_debug" in ld.so + // - ld.so uodates its copy of "_r_debug" with "state = eAdd" before it + // loads the dependent shared libraries for the main executable and + // any dependencies of all shared libraries from the executable's list + // and ld.so code calls the debugger notification function + // that LLDB has set a breakpoint on. + // - LLDB hits the breakpoint and the breakpoint has a callback function + // where we read the _r_debug.state (eAdd) state and we do nothing as the + // "eAdd" state indicates that the shared libraries are about to be added. + // - ld.so finishes loading the main executable and any dependent shared + // libraries and it will update the "_r_debug.state" member with a + // "eConsistent", but it now updates the "_r_debug" in the a.out program + // and it calls the debugger notification function. + // - lldb hits the notification breakpoint and checks the ld.so copy of + // "_r_debug.state" which still has a state of "eAdd", but LLDB needs to see a + // "eConsistent" state to trigger the shared libraries to get loaded into + // the debug session, but LLDB the ld.so _r_debug.state which still + // contains "eAdd" and doesn't do anyhing and library load is missed. + // The "_r_debug" in a.out has the state set correctly to "eConsistent" + // but LLDB is still looking at the "_r_debug" from ld.so. + // + // So if we detect two "eAdd" states in a row, we assume this is the issue + // and we now load shared libraries correctly and will emit a log message + // in the "log enable lldb dyld" log channel which states there might be + // multiple "_r_debug" structs causing problems. + // + // The correct solution is that no one should be adding a duplicate + // publicly visible "_r_debug" symbols to their binaries, but we have + // programs that are doing this already and since it can be done, we should + // be able to work with this and keep debug sessions working as expected. + // + // If a user includes the file, they can just use the existing + // "_r_debug" structure as it is defined in this header file as "extern + // struct r_debug _r_debug;" and no local copies need to be made. + if (m_previous.state == eAdd) { + Log *log = GetLog(LLDBLog::DynamicLoader); + LLDB_LOG(log, "DYLDRendezvous::GetAction() found two eAdd states in a " + "row, check process for multiple \"_r_debug\" symbols. " + "Returning eAddModules to ensure shared libraries get loaded " + "correctly"); + return eAddModules; + } + return eNoAction; case eDelete: return eNoAction; } @@ -225,7 +330,9 @@ } bool DYLDRendezvous::UpdateSOEntriesFromRemote() { - auto action = GetAction(); + const auto action = GetAction(); + Log *log = GetLog(LLDBLog::DynamicLoader); + LLDB_LOG(log, "{0} action = {1}", __PRETTY_FUNCTION__, ActionToCStr(action)); if (action == eNoAction) return false; @@ -263,7 +370,10 @@ bool DYLDRendezvous::UpdateSOEntries() { m_added_soentries.clear(); m_removed_soentries.clear(); - switch (GetAction()) { + const auto action = GetAction(); + Log *log = GetLog(LLDBLog::DynamicLoader); + LLDB_LOG(log, "{0} action = {1}", __PRETTY_FUNCTION__, ActionToCStr(action)); + switch (action) { case eTakeSnapshot: m_soentries.clear(); return TakeSnapshot(m_soentries); diff --git a/lldb/test/API/functionalities/dyld-multiple-rdebug/Makefile b/lldb/test/API/functionalities/dyld-multiple-rdebug/Makefile new file mode 100644 --- /dev/null +++ b/lldb/test/API/functionalities/dyld-multiple-rdebug/Makefile @@ -0,0 +1,4 @@ +CXX_SOURCES := main.cpp +DYLIB_NAME := testlib +DYLIB_CXX_SOURCES := library_file.cpp +include Makefile.rules diff --git a/lldb/test/API/functionalities/dyld-multiple-rdebug/TestDyldWithMultupleRDebug.py b/lldb/test/API/functionalities/dyld-multiple-rdebug/TestDyldWithMultupleRDebug.py new file mode 100644 --- /dev/null +++ b/lldb/test/API/functionalities/dyld-multiple-rdebug/TestDyldWithMultupleRDebug.py @@ -0,0 +1,39 @@ +""" +Test that LLDB can launch a linux executable through the dynamic loader where +the main executable has an extra exported "_r_debug" symbol that used to mess +up shared library loading with DYLDRendezvous and the POSIX dynamic loader +plug-in. What used to happen is that any shared libraries other than the main +executable and the dynamic loader and VSDO would not get loaded. This test +checks to make sure that we still load libraries correctly when we have +multiple "_r_debug" symbols. See comments in the main.cpp source file for full +details on what the problem is. +""" + +import lldb +import os + +from lldbsuite.test import lldbutil +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * + + +class TestDyldWithMultipleRDebug(TestBase): + @skipIf(oslist=no_match(["linux"])) + @no_debug_info_test + def test(self): + self.build() + # Run to a breakpoint in main.cpp to ensure we can hit breakpoints + # in the main executable. Setting breakpoints by file and line ensures + # that the main executable was loaded correctly by the dynamic loader + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "// Break here", lldb.SBFileSpec("main.cpp"), + extra_images=["testlib"] + ) + # Set breakpoints both on shared library function to ensure that + # we hit a source breakpoint in the shared library which only will + # happen if we load the shared library correctly in the dynamic + # loader. + lldbutil.continue_to_source_breakpoint( + self, process, "// Library break here", + lldb.SBFileSpec("library_file.cpp", False) + ) diff --git a/lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.h b/lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.h new file mode 100644 --- /dev/null +++ b/lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.h @@ -0,0 +1 @@ +int library_function(); diff --git a/lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.cpp b/lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.cpp new file mode 100644 --- /dev/null +++ b/lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.cpp @@ -0,0 +1,7 @@ +#include "library_file.h" +#include + +int library_function(void) { + puts(__FUNCTION__); // Library break here + return 0; +} diff --git a/lldb/test/API/functionalities/dyld-multiple-rdebug/main.cpp b/lldb/test/API/functionalities/dyld-multiple-rdebug/main.cpp new file mode 100644 --- /dev/null +++ b/lldb/test/API/functionalities/dyld-multiple-rdebug/main.cpp @@ -0,0 +1,32 @@ +#include "library_file.h" +#include +#include +// Make a duplicate "_r_debug" symbol that is visible. This is the global +// variable name that the dynamic loader uses to communicate changes in shared +// libraries that get loaded and unloaded. LLDB finds the address of this +// variable by reading the DT_DEBUG entry from the .dynamic section of the main +// executable. +// What will happen is the dynamic loader will use the "_r_debug" symbol from +// itself until the a.out executable gets loaded. At this point the new +// "_r_debug" symbol will take precedence over the orignal "_r_debug" symbol +// from the dynamic loader and the copy below will get updated with shared +// library state changes while the version that LLDB checks in the dynamic +// loader stays the same for ever after this. +// +// When our DYLDRendezvous.cpp tries to check the state in the _r_debug +// structure, it will continue to get the last eAdd as the state before the +// switch in symbol resolution. +// +// Before a fix in LLDB, this would mean that we wouldn't ever load any shared +// libraries since DYLDRendezvous was waiting to see a eAdd state followed by a +// eConsistent state which would trigger the adding of shared libraries, but we +// would never see this change because the local copy below is actually what +// would get updated. Now if DYLDRendezvous detects two eAdd states in a row, +// it will load the shared libraries instead of doing nothing and a log message +// will be printed out if "log enable lldb dyld" is active. +r_debug _r_debug; + +int main() { + library_function(); // Break here + return 0; +}