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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

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).

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797