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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
