When rendezvous structure is not initialized we need to set up rendezvous breakpoint anyway. In this case the code will locate dynamic loader (interpreter) and look for known function names.
Bug: 25806
Details
- Reviewers
labath tberghammer - Commits
- rGb7386d994335: Advanced guessing of rendezvous breakpoint (resubmit)
rG4c3ea8029eb8: Advanced guessing of rendezvous breakpoint
rLLDB322251: Advanced guessing of rendezvous breakpoint (resubmit)
rLLDB322209: Advanced guessing of rendezvous breakpoint
rL322251: Advanced guessing of rendezvous breakpoint (resubmit)
rL322209: Advanced guessing of rendezvous breakpoint
Diff Detail
- Repository
- rL LLVM
Event Timeline
packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py | ||
---|---|---|
371 ↗ | (On Diff #127981) | This test has been disabled for a couple of years. I wouldn't be surprised if the problems you are running into with it are due to bitrot. I didn't know this test exists, so I've added TestBreakpointInGlobalConstructor.py to test this behavior a week or two ago. That one is known to pass on mac, so it shouldn't suffer from bitrot. If that's the case then maybe we can just remove this test (or the other one -- In any case, I think we don't need both of them). |
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | ||
265 ↗ | (On Diff #127981) | So, the way I understand this, the main purpose of the entry breakpoint is to set the rendezvous breakpoint (the LoadAllCurrentModules is just a side-effect of us not being able to catch the library load early enough -- if we resolve the rendezvous breakpoint this way, it should not be needed. I expect we can just omit the entry breakpoint altogether if we've managed to set the rendezvous breakpoint this way (we can probably keep the code as a fallback, but use it only for cases where we don't locate any of the rendezvous symbols). |
345 ↗ | (On Diff #127981) | s/tring/trying |
384 ↗ | (On Diff #127981) | the LLDB_LOG will automatically insert file+function (if you do log enable --file-function) so you can just drop this __FUNCTION__ part |
599 ↗ | (On Diff #127981) | s/moduel/module |
packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py | ||
---|---|---|
371 ↗ | (On Diff #127981) | I fixed both of them, I think it's fine to keep both. |
I'm adding some people with non-linux non-x86/arm targets who use this plugin for more visibility. I have a couple of small comments, but in general, I am happy with this.
packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py | ||
---|---|---|
24 ↗ | (On Diff #129002) | The right way to handle this is to use the environment which is returned from self.registerSharedLibrariesWithTarget as the launch environment (you'll need to launch using the SB API). The reason that TestLoadUnload does not handle this is probably because it predates this function. |
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | ||
224 ↗ | (On Diff #129002) | s/enty/entry |
379 ↗ | (On Diff #129002) | 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? |
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.
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | ||
---|---|---|
365–368 ↗ | (On Diff #129002) | Will only one of these ever be present? |
379 ↗ | (On Diff #129002) | 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? |
399 ↗ | (On Diff #129002) | 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. |
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | ||
---|---|---|
365–368 ↗ | (On Diff #129002) | Yes. These are the names of rendezvous functions in different known linkers (Android, Linux, BSD) None of them should have more than one function from this list. |
379 ↗ | (On Diff #129002) | OS loads the dynamic loader, if it is requested by the executable, and passes its base address via AT_BASE value in the auxiliary vector (https://refspecs.linuxfoundation.org/LSB_1.3.0/IA64/spec/auxiliaryvector.html). That's how we know where to look for it. (see EvalSpecialModulesStatus) vdso is yet another special module, but it is not related to the dynamic loader. |
Addressing code review comments. Switching from manual symbol resolution to the appropriate overload of CreateBreakpoint.
Thanks for adding me, Pavel.
Hexagon running Linux uses this plugin. These changes lgtm.
Standalone Hexagon uses its own dyld plugin; I need to look at it and see if I want to pull any of these ideas into it.