release/21.x: [lldb][Instrumentation] Set selected frame to outside sanitizer libraries

lldb/include/lldb/Target/InstrumentationRuntimeStopInfo.h

@@ -24,6 +24,9 @@ class InstrumentationRuntimeStopInfo : public StopInfo {
     return lldb::eStopReasonInstrumentation;
   }
 
+  std::optional<uint32_t>
+  GetSuggestedStackFrameIndex(bool inlined_stack) override;
+
   const char *GetDescription() override;
 
   bool DoShouldNotify(Event *event_ptr) override { return true; }

lldb/include/lldb/Target/StackFrameList.h

@@ -46,6 +46,9 @@ class StackFrameList {
   /// Mark a stack frame as the currently selected frame and return its index.
   uint32_t SetSelectedFrame(lldb_private::StackFrame *frame);
 
+  /// Resets the selected frame index of this object.
+  void ClearSelectedFrameIndex();
+
   /// Get the currently selected frame index.
   /// We should only call SelectMostRelevantFrame if (a) the user hasn't already
   /// selected a frame, and (b) if this really is a user facing
@@ -172,6 +175,15 @@ class StackFrameList {
   /// The currently selected frame. An optional is used to record whether anyone
   /// has set the selected frame on this stack yet. We only let recognizers
   /// change the frame if this is the first time GetSelectedFrame is called.
+  ///
+  /// Thread-safety:
+  /// This member is not protected by a mutex.
+  /// LLDB really only should have an opinion about the selected frame index
+  /// when a process stops, before control gets handed back to the user.
+  /// After that, it's up to them to change it whenever they feel like it.
+  /// If two parts of lldb decided they wanted to be in control of the selected
+  /// frame index on stop the right way to fix it would need to be some explicit
+  /// negotiation for who gets to control this.
   std::optional<uint32_t> m_selected_frame_idx;
 
   /// The number of concrete frames fetched while filling the frame list. This

lldb/include/lldb/Target/Thread.h

@@ -479,6 +479,11 @@ class Thread : public std::enable_shared_from_this<Thread>,
   bool SetSelectedFrameByIndexNoisily(uint32_t frame_idx,
                                       Stream &output_stream);
 
+  /// Resets the selected frame index of this object.
+  void ClearSelectedFrameIndex() {
+    return GetStackFrameList()->ClearSelectedFrameIndex();
+  }
+
   void SetDefaultFileAndLineToSelectedFrame() {
     GetStackFrameList()->SetDefaultFileAndLineToSelectedFrame();
   }

lldb/source/Target/InstrumentationRuntimeStopInfo.cpp

@@ -8,13 +8,20 @@
 
 #include "lldb/Target/InstrumentationRuntimeStopInfo.h"
 
+#include "lldb/Core/Module.h"
 #include "lldb/Target/InstrumentationRuntime.h"
 #include "lldb/Target/Process.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
+static bool IsStoppedInDarwinSanitizer(Thread &thread, Module &module) {
+  return module.GetFileSpec().GetFilename().GetStringRef().starts_with(
+      "libclang_rt.");
+}
+
 InstrumentationRuntimeStopInfo::InstrumentationRuntimeStopInfo(
     Thread &thread, std::string description,
     StructuredData::ObjectSP additional_data)
@@ -34,3 +41,38 @@ InstrumentationRuntimeStopInfo::CreateStopReasonWithInstrumentationData(
   return StopInfoSP(
       new InstrumentationRuntimeStopInfo(thread, description, additionalData));
 }
+
+std::optional<uint32_t>
+InstrumentationRuntimeStopInfo::GetSuggestedStackFrameIndex(
+    bool inlined_stack) {
+  ThreadSP thread_sp = GetThread();
+  if (!thread_sp)
+    return std::nullopt;
+
+  // Defensive upper-bound of when we stop walking up the frames in
+  // case we somehow ended up looking at an infinite recursion.
+  constexpr size_t max_stack_depth = 128;
+
+  // Start at parent frame.
+  size_t stack_idx = 1;
+  StackFrameSP most_relevant_frame_sp =
+      thread_sp->GetStackFrameAtIndex(stack_idx);
+
+  while (most_relevant_frame_sp && stack_idx <= max_stack_depth) {
+    auto const &sc =
+        most_relevant_frame_sp->GetSymbolContext(lldb::eSymbolContextModule);
+
+    if (!sc.module_sp)
+      return std::nullopt;
+
+    // Found a frame outside of the sanitizer runtime libraries.
+    // That's the one we want to display.
+    if (!IsStoppedInDarwinSanitizer(*thread_sp, *sc.module_sp))
+      return stack_idx;
+
+    ++stack_idx;
+    most_relevant_frame_sp = thread_sp->GetStackFrameAtIndex(stack_idx);
+  }
+
+  return stack_idx;
+}

lldb/source/Target/Process.cpp

@@ -4257,6 +4257,14 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr,
         // appropriately. We also need to stop processing actions, since they
         // aren't expecting the target to be running.
 
+        // Clear the selected frame which may have been set as part of utility
+        // expressions that have been run as part of this stop. If we didn't
+        // clear this, then StopInfo::GetSuggestedStackFrameIndex would not
+        // take affect when we next called SelectMostRelevantFrame.
+        // PerformAction should not be the one setting a selected frame, instead
+        // this should be done via GetSuggestedStackFrameIndex.
+        thread_sp->ClearSelectedFrameIndex();
+
         // FIXME: we might have run.
         if (stop_info_sp->HasTargetRunSinceMe()) {
           SetRestarted(true);

lldb/source/Target/StackFrameList.cpp

@@ -936,3 +936,5 @@ size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame,
   strm.IndentLess();
   return num_frames_displayed;
 }
+
+void StackFrameList::ClearSelectedFrameIndex() { m_selected_frame_idx.reset(); }

lldb/test/API/functionalities/asan/TestMemoryHistory.py

@@ -2,14 +2,14 @@
 Test that ASan memory history provider returns correct stack traces
 """
 
-
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbplatform
 from lldbsuite.test import lldbutil
 from lldbsuite.test_event.build_exception import BuildError
 
+
 class MemoryHistoryTestCase(TestBase):
     @skipIfFreeBSD  # llvm.org/pr21136 runtimes not yet available by default
     @expectedFailureNetBSD
@@ -96,6 +96,11 @@ def libsanitizers_asan_tests(self):
         )
         self.check_traces(skip_line_numbers=True)
 
+        if self.platformIsDarwin():
+            # Make sure we're not stopped in the sanitizer library but instead at the
+            # point of failure in the user-code.
+            self.assertEqual(self.frame().GetFunctionName(), "main")
+
         # do the same using SB API
         process = self.dbg.GetSelectedTarget().process
         val = (
@@ -220,6 +225,11 @@ def compiler_rt_asan_tests(self):
 
         self.check_traces()
 
+        if self.platformIsDarwin():
+            # Make sure we're not stopped in the sanitizer library but instead at the
+            # point of failure in the user-code.
+            self.assertEqual(self.frame().GetFunctionName(), "main")
+
         # make sure the 'memory history' command still works even when we're
         # generating a report now
         self.expect(

lldb/test/API/functionalities/asan/TestReportData.py

@@ -2,14 +2,14 @@
 Test the AddressSanitizer runtime support for report breakpoint and data extraction.
 """
 
-
 import json
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 from lldbsuite.test_event.build_exception import BuildError
 
+
 class AsanTestReportDataCase(TestBase):
     @skipIfFreeBSD  # llvm.org/pr21136 runtimes not yet available by default
     @expectedFailureNetBSD
@@ -67,6 +67,11 @@ def asan_tests(self, libsanitizers=False):
             lldb.eStopReasonInstrumentation,
         )
 
+        if self.platformIsDarwin():
+            # Make sure we're not stopped in the sanitizer library but instead at the
+            # point of failure in the user-code.
+            self.assertEqual(self.frame().GetFunctionName(), "main")
+
         self.expect(
             "bt",
             "The backtrace should show the crashing line",

lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py

@@ -63,11 +63,14 @@ def tsan_tests(self):
             substrs=["1 match found"],
         )
 
-        # We should be stopped in __tsan_on_report
         process = self.dbg.GetSelectedTarget().process
         thread = process.GetSelectedThread()
         frame = thread.GetSelectedFrame()
-        self.assertIn("__tsan_on_report", frame.GetFunctionName())
+        if self.platformIsDarwin():
+            # We should not be stopped in the sanitizer library.
+            self.assertIn("f2", frame.GetFunctionName())
+        else:
+            self.assertIn("__tsan_on_report", frame.GetFunctionName())
 
         # The stopped thread backtrace should contain either line1 or line2
         # from main.c.

lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py

@@ -52,8 +52,11 @@ def ubsan_tests(self):
             substrs=["1 match found"],
         )
 
-        # We should be stopped in __ubsan_on_report
-        self.assertIn("__ubsan_on_report", frame.GetFunctionName())
+        if self.platformIsDarwin():
+            # We should not be stopped in the sanitizer library.
+            self.assertIn("main", frame.GetFunctionName())
+        else:
+            self.assertIn("__ubsan_on_report", frame.GetFunctionName())
 
         # The stopped thread backtrace should contain either 'align line'
         found = False