clayborg updated this revision to Diff 553259.
clayborg added a comment.
Fixed more header documentation and cleaned up the test even more.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158583/new/
https://reviews.llvm.org/D158583
Files:
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
lldb/test/API/functionalities/dyld-multiple-rdebug/Makefile
lldb/test/API/functionalities/dyld-multiple-rdebug/TestDyldWithMultupleRDebug.py
lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.cpp
lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.h
lldb/test/API/functionalities/dyld-multiple-rdebug/main.cpp
Index: lldb/test/API/functionalities/dyld-multiple-rdebug/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/dyld-multiple-rdebug/main.cpp
@@ -0,0 +1,32 @@
+#include "library_file.h"
+#include <link.h>
+#include <stdio.h>
+// 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;
+}
Index: lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.h
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.h
@@ -0,0 +1 @@
+int library_function();
Index: lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/dyld-multiple-rdebug/library_file.cpp
@@ -0,0 +1,7 @@
+#include "library_file.h"
+#include <stdio.h>
+
+int library_function(void) {
+ puts(__FUNCTION__); // Library break here
+ return 0;
+}
Index: lldb/test/API/functionalities/dyld-multiple-rdebug/TestDyldWithMultupleRDebug.py
===================================================================
--- /dev/null
+++ 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)
+ )
Index: lldb/test/API/functionalities/dyld-multiple-rdebug/Makefile
===================================================================
--- /dev/null
+++ 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
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
===================================================================
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
+++ 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;
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ 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 "<invalid RendezvousState>";
+}
+
+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 "<invalid RendezvousAction>";
+}
+
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 <link.h>
+ // 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 <link.h> 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);
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
