labath added inline comments.
================
Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:153
+ cursor = info_addr =
+ ResolveRendezvousAddress(m_process, m_executable_interpreter);
else
----------------
At this point, you can just make `ResolveRendezvousAddress` a (private) member
of the DYLDRendezvous class.
================
Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:318
+ if (!SOEntryIsMainExecutable(entry)) {
+ if (entry.file_spec.GetFilename().IsEmpty())
+ UpdateFileSpecIfNecessary(entry);
----------------
Since the function already has `IfNecessary` in the name, you don't need to
guard the call once more.
================
Comment at:
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:453-455
+ // When executable is run with argv[0] as ld.so, ld.so corresponds to
+ // m_exe_file_spec. In this case, the returned link map
+ // has empty entry for executable, which is not the main executable.
----------------
How about this:
```
// If were debugging ld.so, then all SOEntries should be treated as libraries,
including the "main" one (denoted by an empty string).
if (m_executable_interpreter)
return false;
return !entry.file_spec;
================
Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:552
+ // When argv[0] is ld.so, we need to be update executable path.
+ if (entry.file_spec.GetFilename().IsEmpty()) {
+ MemoryRegionInfo region;
----------------
I don't think this should make a difference, but `if(!entry.file_spec)` is
shorter and consistent with the logic in `SOEntryIsMainExecutable`.
================
Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h:186
+ /// It is set to true when executable is run with argv[0] as ld.so.
+ bool m_executable_interpreter;
----------------
I get what you're trying to say, but I'm having issues with these comments, as
they're wrong on multiple levels. For one, argv[0] essentially has no
relationship to the binary being executed (ld.so even has an argument to supply
an arbitrary string as argv[0] to the executable). But more importantly, when
the executable is "run" in this way (as an argument to ld.so), as far as the
operating system (and lldb) is concerned ld.so _is_ the main executable. You
can see this in the /proc/$PID/exe symlink, or in all the
target->GetExecutableModule calls in this patch. What this patch actually does
is to _disable_ the logic of treating the "main executable" (the one with an
empty name in the rendezvous structure) as special, so that it can be treated
as one of the libraries.
With that in mind, I think it'd be better to say that this flag indicates
whether the main program is the dynamic linker/loader/program interpreter.
================
Comment at:
lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py:18-24
+ candidates = [
+ "/lib64/ld-linux-x86-64.so.2",
+ "/usr/lib/ld-linux-x86-64.so.2"
+ ]
+ exe = next((c for c in candidates if os.path.exists(c)), None)
+ if not os.path.exists(exe):
+ return
----------------
It should be possible to retrieve the actual interpreter path by inspecting the
compiled executable. Something like
`SBModule(exe)->FindSection(".interp")->GetSectionData()` ought to do it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108061/new/
https://reviews.llvm.org/D108061
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
