[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

Fri Sep 17 03:00:53 PDT 2021

labath added a comment.

In D109797#3004232 <https://reviews.llvm.org/D109797#3004232>, @emrekultursay wrote:

> In D109797#3003265 <https://reviews.llvm.org/D109797#3003265>, @labath wrote:
>
>> Since the meaning of `m_initial_modules_added` is basically "should I do a full scan through the linked list" and LoadAllCurrentModules (called from DidAttach) does a full scan, it seems to me the real bug is that this function does not set `m_initial_modules_added = true`. Would that fix your issue?
>
> Yes, it does fix my bug, thanks for the suggestion. Would you like me to remove the lines 445-447 that I added, or keep them as a secondary protection (since `module_sp != m_interpreter_module.lock()` condition doesn't seem to be strong enough).

I suppose keeping it is fine. It will make sure m_interpreter_module is up to date, if anyone needs to access it in the future.

> In D109797#3003265 <https://reviews.llvm.org/D109797#3003265>, @labath wrote:
>
>> Also, what makes Android 28 special? Is it the presence/absence of a dynamic linker symlink? I'm wondering if we could reproduce (test) this more generally by ensuring we launch a binary through a symlink (or not).
>
> On Android, `GetExecutableModule()` returns `GetModuleAtIndex(0)`. For Android 27/29, this returns `app_process64` (correct), but for Android 28 it returns `[vdso]`, which is incorrect. This causes `ResolveExecutableModule()` to fall through due to `module_spec` (app_process64) not matching `module_sp` ([vdso]). This causes `target.SetExecutableModule` to be called, which triggers `ClearModules`. Since all modules are cleared, we cannot get the load address of the `.dynamic` section when we check whether we should rebase (i.e., `addr.GetLoadAddress(&target)` returns invalid address), thus `rebase_exec=true`.  The rest is in CL description.
>
> To test this, we would need a binary that (1) doesn't have `eTypeExecutable` in any of its modules (2) has a module other than the executable module at location 0.

I'm not sure all of this is relevant. I've just tried to debug a very simple (linux) executable that does a dlopens a library. If I attach to the executable before the dlopen call, lldb does not notice the new library. This patch fixes the issue. So I guess the test could be just that. You could do something similar to TestLoadUnload -- either add a new test case which attaches to that binary, or use it as inspiration to write a new test.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109797/new/

https://reviews.llvm.org/D109797