https://github.com/llvmbot updated https://github.com/llvm/llvm-project/pull/157568
>From 54896838ca5e1f69f0f3040fe6847546db0463d4 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 8 Sep 2025 17:18:25 +0100 Subject: [PATCH 1/4] [lldb][Target] Clear selected frame index after a StopInfo::PerformAction (#133078) The motivation for this patch is that `StopInfo::GetSuggestedStackFrameIndex` would not take effect for `InstrumentationRuntimeStopInfo` (which we plan to implement in https://github.com/llvm/llvm-project/pull/133079). This was happening because the instrumentation runtime plugins would run utility expressions as part of the stop that would set the `m_selected_frame_idx`. This means `SelectMostRelevantFrame` was never called, and we would not be able to report the selected frame via the `StopInfo` object. This patch makes sure we clear the `m_selected_frame_idx` to allow `GetSuggestedStackFrameIndex` to take effect, regardless of what the frame recognizers choose to do. (cherry picked from commit 39572f5e9168b1b44c2f9078494616fed8752086) --- lldb/include/lldb/Target/StackFrameList.h | 12 ++++++++++++ lldb/include/lldb/Target/Thread.h | 5 +++++ lldb/source/Target/Process.cpp | 8 ++++++++ lldb/source/Target/StackFrameList.cpp | 2 ++ 4 files changed, 27 insertions(+) diff --git a/lldb/include/lldb/Target/StackFrameList.h b/lldb/include/lldb/Target/StackFrameList.h index 8a66296346f2d..8d455dc831df3 100644 --- a/lldb/include/lldb/Target/StackFrameList.h +++ b/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 diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index 6ede7fa301a82..688c056da2633 100644 --- a/lldb/include/lldb/Target/Thread.h +++ b/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(); } diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 2aa02fd58335e..a2fa88b569135 100644 --- a/lldb/source/Target/Process.cpp +++ b/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); diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp index 16cd2548c2784..931b73b1e3633 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/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(); } >From faedeb1a242079c7fdea59d8b8487ab9d4c31bb9 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 8 Sep 2025 18:03:31 +0100 Subject: [PATCH 2/4] [lldb][Instrumentation] Set selected frame to outside sanitizer libraries (#133079) When hitting a sanitizer breakpoint, LLDB currently displays the frame in the sanitizer dylib (which we usually don't have debug-info for), which isn't very helpful to the user. A more helpful frame to display would be the first frame not in the sanitizer library (we have a [similar heuristic when we trap inside libc++](https://github.com/llvm/llvm-project/pull/108825)). This patch does just that, by implementing the `GetSuggestedStackFrameIndex` API Depends on https://github.com/llvm/llvm-project/pull/133078 (cherry picked from commit 879f40ab041b31fa73b9b25e4ec9e06e810bc767) --- .../Target/InstrumentationRuntimeStopInfo.h | 3 ++ .../Target/InstrumentationRuntimeStopInfo.cpp | 42 +++++++++++++++++++ .../functionalities/asan/TestMemoryHistory.py | 8 ++++ .../functionalities/asan/TestReportData.py | 4 ++ .../ubsan/basic/TestUbsanBasic.py | 4 +- 5 files changed, 59 insertions(+), 2 deletions(-) diff --git a/lldb/include/lldb/Target/InstrumentationRuntimeStopInfo.h b/lldb/include/lldb/Target/InstrumentationRuntimeStopInfo.h index 5345160850914..dafa41c11327a 100644 --- a/lldb/include/lldb/Target/InstrumentationRuntimeStopInfo.h +++ b/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; } diff --git a/lldb/source/Target/InstrumentationRuntimeStopInfo.cpp b/lldb/source/Target/InstrumentationRuntimeStopInfo.cpp index 7f82581cc601e..aef895def7939 100644 --- a/lldb/source/Target/InstrumentationRuntimeStopInfo.cpp +++ b/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; +} diff --git a/lldb/test/API/functionalities/asan/TestMemoryHistory.py b/lldb/test/API/functionalities/asan/TestMemoryHistory.py index 1568140b355dc..0a2ae96a557ac 100644 --- a/lldb/test/API/functionalities/asan/TestMemoryHistory.py +++ b/lldb/test/API/functionalities/asan/TestMemoryHistory.py @@ -96,6 +96,10 @@ def libsanitizers_asan_tests(self): ) self.check_traces(skip_line_numbers=True) + # 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 +224,10 @@ def compiler_rt_asan_tests(self): self.check_traces() + # 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( diff --git a/lldb/test/API/functionalities/asan/TestReportData.py b/lldb/test/API/functionalities/asan/TestReportData.py index dd6834a01b80c..ccc1b846d1607 100644 --- a/lldb/test/API/functionalities/asan/TestReportData.py +++ b/lldb/test/API/functionalities/asan/TestReportData.py @@ -67,6 +67,10 @@ def asan_tests(self, libsanitizers=False): lldb.eStopReasonInstrumentation, ) + # 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", diff --git a/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py b/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py index 868a2864d2b5e..f46d167d910ea 100644 --- a/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py +++ b/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py @@ -52,8 +52,8 @@ def ubsan_tests(self): substrs=["1 match found"], ) - # We should be stopped in __ubsan_on_report - self.assertIn("__ubsan_on_report", frame.GetFunctionName()) + # We should not be stopped in the sanitizer library. + self.assertIn("main", frame.GetFunctionName()) # The stopped thread backtrace should contain either 'align line' found = False >From f5ba88341e7c4f7427e33c8237c07c3c5f1e189c Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 8 Sep 2025 19:25:20 +0100 Subject: [PATCH 3/4] [lldb][test] TestTsanBasic.py: fix function name assertion Since https://github.com/llvm/llvm-project/pull/133079 we no longer stop in the stanitizer library when hitting a sanitizer breakpoint. Adjust the test accordingly. (cherry picked from commit bdf645bb9b509b60bdb6a71d865b4f8999187977) --- lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py b/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py index ca8b74e35dff6..2747b2b442195 100644 --- a/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py +++ b/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py @@ -63,11 +63,11 @@ def tsan_tests(self): substrs=["1 match found"], ) - # We should be stopped in __tsan_on_report + # We should not be stopped in the sanitizer library. process = self.dbg.GetSelectedTarget().process thread = process.GetSelectedThread() frame = thread.GetSelectedFrame() - self.assertIn("__tsan_on_report", frame.GetFunctionName()) + self.assertIn("f2", frame.GetFunctionName()) # The stopped thread backtrace should contain either line1 or line2 # from main.c. >From 2c8cb316b5d2b0121d7d364d30b86b4310fb0809 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 8 Sep 2025 22:32:49 +0100 Subject: [PATCH 4/4] [lldb][test] Only assert function name is in user-code on Darwin platforms The frame recognizer for the instrumentation runtimes (added in https://github.com/llvm/llvm-project/pull/133079) only triggers on Darwin for `libclang_rt`. Adjust the tests accordingly. (cherry picked from commit 5326b3b176e82191b18ffc368118b36e0103af3d) --- .../functionalities/asan/TestMemoryHistory.py | 16 +++++++++------- .../API/functionalities/asan/TestReportData.py | 9 +++++---- .../functionalities/tsan/basic/TestTsanBasic.py | 7 +++++-- .../ubsan/basic/TestUbsanBasic.py | 7 +++++-- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/lldb/test/API/functionalities/asan/TestMemoryHistory.py b/lldb/test/API/functionalities/asan/TestMemoryHistory.py index 0a2ae96a557ac..302a45c2368de 100644 --- a/lldb/test/API/functionalities/asan/TestMemoryHistory.py +++ b/lldb/test/API/functionalities/asan/TestMemoryHistory.py @@ -2,7 +2,6 @@ Test that ASan memory history provider returns correct stack traces """ - import lldb from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * @@ -10,6 +9,7 @@ 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,9 +96,10 @@ def libsanitizers_asan_tests(self): ) self.check_traces(skip_line_numbers=True) - # 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") + 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 @@ -224,9 +225,10 @@ def compiler_rt_asan_tests(self): self.check_traces() - # 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") + 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 diff --git a/lldb/test/API/functionalities/asan/TestReportData.py b/lldb/test/API/functionalities/asan/TestReportData.py index ccc1b846d1607..c832436b0f447 100644 --- a/lldb/test/API/functionalities/asan/TestReportData.py +++ b/lldb/test/API/functionalities/asan/TestReportData.py @@ -2,7 +2,6 @@ Test the AddressSanitizer runtime support for report breakpoint and data extraction. """ - import json import lldb from lldbsuite.test.decorators import * @@ -10,6 +9,7 @@ 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,9 +67,10 @@ def asan_tests(self, libsanitizers=False): lldb.eStopReasonInstrumentation, ) - # 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") + 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", diff --git a/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py b/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py index 2747b2b442195..51a28c5013071 100644 --- a/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py +++ b/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py @@ -63,11 +63,14 @@ def tsan_tests(self): substrs=["1 match found"], ) - # We should not be stopped in the sanitizer library. process = self.dbg.GetSelectedTarget().process thread = process.GetSelectedThread() frame = thread.GetSelectedFrame() - self.assertIn("f2", 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. diff --git a/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py b/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py index f46d167d910ea..9e9ea2114196e 100644 --- a/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py +++ b/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py @@ -52,8 +52,11 @@ def ubsan_tests(self): substrs=["1 match found"], ) - # We should not be stopped in the sanitizer library. - self.assertIn("main", 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 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
