[Lldb-commits] [PATCH] D41533: Advanced guessing of rendezvous breakpoint

Greg Clayton via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Jan 9 08:25:11 PST 2018

clayborg added a comment.

Looks fine. Set the breakpoint using the list of names and delete the breakpoint if you get no locations and this will be good to go.

Comment at: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:365-368
+    static const char *DebugStateCandidates[] = {
+        "_dl_debug_state", "rtld_db_dlactivity", "__dl_rtld_db_dlactivity",
+        "r_debug_state",   "_r_debug_state",     "_rtld_debug_state",
+    };
Will only one of these ever be present? 

Comment at: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:379
+    }
+    break_addr = symbol->GetLoadAddress(&m_process->GetTarget());
+    if (break_addr == LLDB_INVALID_ADDRESS) {
labath wrote:
> It should be possible to create the breakpoint via `Target::CreateBreakpoint` (the overload that lets you specify a list of function names) instead of manually resolving the symbols... Have you tried doing that?
Agreed, I would use the breakpoint setting function in target that allows you to specify one or more names as labath suggested. One question about this code though: how would we ever get a load address from a module if the dynamic loader doesn't load it itself? Are these symbols in the "[vdso]" module and is the [vdso]" module loaded differently?

Comment at: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:399
+  dyld_break->SetBreakpointKind("shared-library-event");
+  if (dyld_break->GetNumResolvedLocations() == 0)
+    return false;
You should delete the breakpoint if the number of locations is zero (for this one by address and also in case you end up setting the breakpoint using a list of names). Otherwise you will have a dangling breakpoint that no one wants.


More information about the lldb-commits mailing list