[Lldb-commits] [PATCH] D96637: Make sure the interpreter module was loaded before making checks against it

Sat Feb 20 10:36:13 PST 2021

aadsm added inline comments.

================
Comment at: lldb/test/API/functionalities/module_load_attach/TestModuleLoadAttach.py:28
+    def test_x(self):
+        process = self.build_launch_and_attach()
+        thread = process.GetSelectedThread()
----------------
clayborg wrote:
> Is this racy? What happens on a really slow system? Can we fail to attach? If we do attach, are we guaranteed to be at a place where we can set "flip_to_1_to_continue = 1"? The nice thing is it is a global variable that we should be able to set no matter where we stop. 
> Is this racy?

I don't think so because we already have a pid at that point in time, so we should always be able to attach.

> If we do attach, are we guaranteed to be at a place where we can set "flip_to_1_to_continue = 1"?

yeah, that's exactly why I made it global. I could also wait until there's a `flip_to_1_to_continue` in the scope if you think it's worthwhile.

================
Comment at: lldb/test/API/functionalities/module_load_attach/TestModuleLoadAttach.py:32-33
+        # Continue so that dlopen is called.
+        breakpoint = self.target().BreakpointCreateBySourceRegex(
+            "// break after dlopen", lldb.SBFileSpec("main.c"))
+        self.assertNotEqual(breakpoint.GetNumResolvedLocations(), 0)
----------------
clayborg wrote:
> Don't we need to break before the dlopen and make sure we don't have a libfeature.so in our module list, then run over the dlopen and verify we do see it afterwards? Wasn't this bug that we will see shared libraries correctly one time when we attach, but just not get any updates after this??
that was a completely different bug and I have a different test for that situation as well.
Something that I could test though, is that before we got an update for an unresolved breakpoint to make sure we did indeed transitioned from unresolved -> resolved.

I'll add that.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96637