Merge pull request #9315 from jasonmolenda/cp/136247381-dont-invalid-register-context-unnecessarily

[lldb] Don't invalid register context after setting thread pc's (llvm#109499)

Author: jasonmolenda

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

@@ -1597,7 +1597,6 @@ bool ProcessGDBRemote::CalculateThreadStopInfo(ThreadGDBRemote *thread) {
     // If we have "jstopinfo" then we have stop descriptions for all threads
     // that have stop reasons, and if there is no entry for a thread, then it
     // has no stop reason.
-    thread->GetRegisterContext()->InvalidateIfNeeded(true);
     if (!GetThreadStopInfoFromJSON(thread, m_jstopinfo_sp)) {
       // If a thread is stopped at a breakpoint site, set that as the stop
       // reason even if it hasn't executed the breakpoint instruction yet.

lldb/test/API/macosx/expedited-thread-pcs/Makefile

@@ -0,0 +1,11 @@
+CXX_SOURCES := main.cpp
+
+.PHONY: build-libfoo
+all: build-libfoo a.out
+
+include Makefile.rules
+
+build-libfoo: foo.c
+	$(MAKE) -f $(MAKEFILE_RULES) \
+		DYLIB_C_SOURCES=foo.c DYLIB_NAME=foo DYLIB_ONLY=YES
+

lldb/test/API/macosx/expedited-thread-pcs/TestExpeditedThreadPCs.py

@@ -0,0 +1,91 @@
+"""Test that the expedited thread pc values are not re-fetched by lldb."""
+
+import subprocess
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+file_index = 0
+
+
+class TestExpeditedThreadPCs(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipUnlessDarwin
+    def test_expedited_thread_pcs(self):
+        TestBase.setUp(self)
+
+        global file_index
+        ++file_index
+        logfile = os.path.join(
+            self.getBuildDir(),
+            "packet-log-" + self.getArchitecture() + "-" + str(file_index) + ".txt",
+        )
+        self.runCmd("log enable -f %s gdb-remote packets" % (logfile))
+
+        def cleanup():
+            self.runCmd("log disable gdb-remote packets")
+            if os.path.exists(logfile):
+                os.unlink(logfile)
+
+        self.addTearDownHook(cleanup)
+
+        self.source = "main.cpp"
+        self.build()
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "break here", lldb.SBFileSpec(self.source, False)
+        )
+
+        # verify that libfoo.dylib hasn't loaded yet
+        for m in target.modules:
+            self.assertNotEqual(m.GetFileSpec().GetFilename(), "libfoo.dylib")
+
+        thread.StepInto()
+        thread.StepInto()
+
+        thread.StepInto()
+        thread.StepInto()
+        thread.StepInto()
+
+        # verify that libfoo.dylib has loaded
+        for m in target.modules:
+            if m.GetFileSpec().GetFilename() == "libfoo.dylib":
+                found_libfoo = True
+        self.assertTrue(found_libfoo)
+
+        thread.StepInto()
+        thread.StepInto()
+        thread.StepOver()
+        thread.StepOver()
+        thread.StepOver()
+        thread.StepOver()
+        thread.StepOver()
+        thread.StepOver()
+        thread.StepOver()
+        thread.StepOver()
+        thread.StepOver()
+        thread.StepOver()
+
+        process.Kill()
+
+        # Confirm that we never fetched the pc for any threads during
+        # this debug session.
+        if os.path.exists(logfile):
+            f = open(logfile)
+            lines = f.readlines()
+            num_errors = 0
+            for line in lines:
+                arch = self.getArchitecture()
+                if arch == "arm64" or arch == "arm64_32":
+                    #   <reg name="pc" regnum="32" offset="256" bitsize="64" group="general" group_id="1" ehframe_regnum="32" dwarf_regnum="32" generic="pc"/>
+                    # A fetch of $pc on arm64 looks like
+                    #  <  22> send packet: $p20;thread:91698e;#70
+                    self.assertNotIn("$p20;thread", line)
+                else:
+                    #   <reg name="rip" regnum="16" offset="128" bitsize="64" group="general" altname="pc" group_id="1" ehframe_regnum="16" dwarf_regnum="16" generic="pc"/>
+                    # A fetch of $pc on x86_64 looks like
+                    #  <  22> send packet: $p10;thread:91889c;#6f
+                    self.assertNotIn("$p10;thread", line)
+
+            f.close()

lldb/test/API/macosx/expedited-thread-pcs/foo.c

@@ -0,0 +1 @@
+int foo() { return 5; }

lldb/test/API/macosx/expedited-thread-pcs/main.cpp

@@ -0,0 +1,62 @@
+#include <dlfcn.h>
+#include <stdio.h>
+#include <thread>
+#include <unistd.h>
+
+void f1() {
+  while (1)
+    sleep(1);
+}
+void f2() {
+  while (1)
+    sleep(1);
+}
+void f3() {
+  while (1)
+    sleep(1);
+}
+
+int main() {
+  std::thread t1{f1};
+  std::thread t2{f2};
+  std::thread t3{f3};
+
+  puts("break here");
+
+  void *handle = dlopen("libfoo.dylib", RTLD_LAZY);
+  int (*foo_ptr)() = (int (*)())dlsym(handle, "foo");
+  int c = foo_ptr();
+
+  // clang-format off
+  // multiple function calls on a single source line so 'step'
+  // and 'next' need to do multiple steps of work.
+  puts("1"); puts("2"); puts("3"); puts("4"); puts("5");
+  puts("6"); puts("7"); puts("8"); puts("9"); puts("10");
+  puts("11"); puts("12"); puts("13"); puts("14"); puts("15");
+  puts("16"); puts("17"); puts("18"); puts("19"); puts("20");
+  puts("21"); puts("22"); puts("23"); puts("24"); puts("24");
+  // clang-format on
+  puts("one");
+  puts("two");
+  puts("three");
+  puts("four");
+  puts("five");
+  puts("six");
+  puts("seven");
+  puts("eight");
+  puts("nine");
+  puts("ten");
+  c++;
+  c++;
+  c++;
+  c++;
+  c++;
+  c++;
+  c++;
+  c++;
+  c++;
+  c++;
+  c++;
+  c++;
+  return c;
+}