From ee58ec0f67b5ef70d7c11756f5931ad08ed7c4dc Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 7 Oct 2025 13:44:45 -0700 Subject: [PATCH 1/7] Make SBBreakpoint/SBBreakpointLocation.SetCondition(nullptr) work again. The addition of the StopCondition in the lldb_private layer meant that clearing a breakpoint condition with: sb_break.SetCondition(nullptr); now crashes. Also, GetCondition for an empty condition used to return a nullptr, but now it returns "". This patch fixes that crash and makes the SB GetCondition always return nullptr for an empty condition. --- lldb/source/API/SBBreakpoint.cpp | 11 ++- lldb/source/API/SBBreakpointLocation.cpp | 11 ++- .../BreakpointClearConditionTest.cpp | 85 +++++++++++++++++++ lldb/unittests/Breakpoint/CMakeLists.txt | 7 +- 4 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 lldb/unittests/Breakpoint/BreakpointClearConditionTest.cpp diff --git a/lldb/source/API/SBBreakpoint.cpp b/lldb/source/API/SBBreakpoint.cpp index 07c0a2ea907ba..23dba462478c9 100644 --- a/lldb/source/API/SBBreakpoint.cpp +++ b/lldb/source/API/SBBreakpoint.cpp @@ -275,7 +275,11 @@ void SBBreakpoint::SetCondition(const char *condition) { if (bkpt_sp) { std::lock_guard guard( bkpt_sp->GetTarget().GetAPIMutex()); - bkpt_sp->SetCondition(StopCondition(condition)); + // Treat a null pointer as resetting the condition. + if (!condition) + bkpt_sp->SetCondition(StopCondition()); + else + bkpt_sp->SetCondition(StopCondition(condition)); } } @@ -288,7 +292,10 @@ const char *SBBreakpoint::GetCondition() { std::lock_guard guard( bkpt_sp->GetTarget().GetAPIMutex()); - return ConstString(bkpt_sp->GetCondition().GetText()).GetCString(); + StopCondition cond = bkpt_sp->GetCondition(); + if (!cond) + return nullptr; + return ConstString(cond.GetText()).GetCString(); } void SBBreakpoint::SetAutoContinue(bool auto_continue) { diff --git a/lldb/source/API/SBBreakpointLocation.cpp b/lldb/source/API/SBBreakpointLocation.cpp index e786435c4f8af..2feaa5c805a15 100644 --- a/lldb/source/API/SBBreakpointLocation.cpp +++ b/lldb/source/API/SBBreakpointLocation.cpp @@ -160,7 +160,11 @@ void SBBreakpointLocation::SetCondition(const char *condition) { if (loc_sp) { std::lock_guard guard( loc_sp->GetTarget().GetAPIMutex()); - loc_sp->SetCondition(StopCondition(condition)); + // Treat a nullptr as clearing the condition + if (!condition) + loc_sp->SetCondition(StopCondition()); + else + loc_sp->SetCondition(StopCondition(condition)); } } @@ -173,7 +177,10 @@ const char *SBBreakpointLocation::GetCondition() { std::lock_guard guard( loc_sp->GetTarget().GetAPIMutex()); - return ConstString(loc_sp->GetCondition().GetText()).GetCString(); + StopCondition cond = loc_sp->GetCondition(); + if (!cond) + return nullptr; + return ConstString(cond.GetText()).GetCString(); } void SBBreakpointLocation::SetAutoContinue(bool auto_continue) { diff --git a/lldb/unittests/Breakpoint/BreakpointClearConditionTest.cpp b/lldb/unittests/Breakpoint/BreakpointClearConditionTest.cpp new file mode 100644 index 0000000000000..688c85a4a711f --- /dev/null +++ b/lldb/unittests/Breakpoint/BreakpointClearConditionTest.cpp @@ -0,0 +1,85 @@ +//===-- BreakpointClearConditionTest.cpp +//--------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Plugins/Platform/MacOSX/PlatformMacOSX.h" +#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h" +#include "TestingSupport/SubsystemRAII.h" +#include "TestingSupport/TestUtilities.h" +#include "lldb/API/SBBreakpoint.h" +#include "lldb/API/SBBreakpointLocation.h" +#include "lldb/API/SBDebugger.h" +#include "lldb/API/SBTarget.h" +#include "lldb/Breakpoint/StoppointCallbackContext.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Core/Progress.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Host/HostInfo.h" +#include "lldb/Target/ExecutionContext.h" +#include "lldb/lldb-private-enumerations.h" +#include "lldb/lldb-types.h" +#include "gtest/gtest.h" +#include +#include +#include + +using namespace lldb_private; +using namespace lldb; + +class BreakpointClearConditionTest : public ::testing::Test { +public: + void SetUp() override { + std::call_once(TestUtilities::g_debugger_initialize_flag, + []() { SBDebugger::Initialize(); }); + }; + + SBDebugger m_sb_debugger; + SubsystemRAII subsystems; +}; + +template +void test_condition(T sb_object) { + const char *in_cond_str = "Here is a condition"; + sb_object.SetCondition(in_cond_str); + // Make sure we set the condition correctly: + const char *out_cond_str = sb_object.GetCondition(); + EXPECT_STREQ(in_cond_str, out_cond_str); + // Now unset it by passing in nullptr and make sure that works: + const char *empty_tokens[2] = {nullptr, ""}; + for (auto token : empty_tokens) { + sb_object.SetCondition(token); + out_cond_str = sb_object.GetCondition(); + // And make sure an unset condition returns nullptr: + EXPECT_EQ(nullptr, out_cond_str); + } +} + +TEST_F(BreakpointClearConditionTest, BreakpointClearConditionTest) { + // Set up the debugger, make sure that was done properly. + m_sb_debugger = SBDebugger::Create(false); + + // Create target + SBTarget sb_target; + SBError error; + sb_target = m_sb_debugger.CreateTarget("", "x86_64-apple-macosx-", "remote-macosx", + /*add_dependent=*/ false, error); + + EXPECT_EQ(sb_target.IsValid(), true); + + // Create breakpoint + SBBreakpoint sb_breakpoint = + sb_target.BreakpointCreateByAddress(0xDEADBEEF); + test_condition(sb_breakpoint); + + // Address breakpoints always have one location, so we can also use this + // to test the location: + SBBreakpointLocation sb_loc = sb_breakpoint.GetLocationAtIndex(0); + EXPECT_EQ(sb_loc.IsValid(), true); + test_condition(sb_loc); + +} diff --git a/lldb/unittests/Breakpoint/CMakeLists.txt b/lldb/unittests/Breakpoint/CMakeLists.txt index 3c234a4fea29a..005cb4787c365 100644 --- a/lldb/unittests/Breakpoint/CMakeLists.txt +++ b/lldb/unittests/Breakpoint/CMakeLists.txt @@ -1,10 +1,15 @@ -add_lldb_unittest(LLDBBreakpointTests +add_lldb_unittest(LLDBBreakpointTests BreakpointIDTest.cpp + BreakpointClearConditionTest.cpp WatchpointAlgorithmsTests.cpp LINK_COMPONENTS Support LINK_LIBS + liblldb lldbBreakpoint lldbCore + LLVMTestingSupport + lldbUtilityHelpers + lldbPluginPlatformMacOSX ) From 0af04fad1d92b8f9d47083069421efc78f55c079 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 7 Oct 2025 14:40:03 -0700 Subject: [PATCH 2/7] Move the unittest to the API directory and make it a pure SB test. --- lldb/unittests/API/CMakeLists.txt | 1 + .../SBBreakpointClearConditionTest.cpp} | 21 +++---------------- lldb/unittests/Breakpoint/CMakeLists.txt | 1 - 3 files changed, 4 insertions(+), 19 deletions(-) rename lldb/unittests/{Breakpoint/BreakpointClearConditionTest.cpp => API/SBBreakpointClearConditionTest.cpp} (73%) diff --git a/lldb/unittests/API/CMakeLists.txt b/lldb/unittests/API/CMakeLists.txt index 06ac49244176c..1ced3b119f87f 100644 --- a/lldb/unittests/API/CMakeLists.txt +++ b/lldb/unittests/API/CMakeLists.txt @@ -2,6 +2,7 @@ add_lldb_unittest(APITests SBCommandInterpreterTest.cpp SBLineEntryTest.cpp SBMutexTest.cpp + SBBreakpointClearConditionTest.cpp LINK_LIBS liblldb diff --git a/lldb/unittests/Breakpoint/BreakpointClearConditionTest.cpp b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp similarity index 73% rename from lldb/unittests/Breakpoint/BreakpointClearConditionTest.cpp rename to lldb/unittests/API/SBBreakpointClearConditionTest.cpp index 688c85a4a711f..a89e3848ec9d0 100644 --- a/lldb/unittests/Breakpoint/BreakpointClearConditionTest.cpp +++ b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp @@ -1,4 +1,3 @@ -//===-- BreakpointClearConditionTest.cpp //--------------------------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. @@ -7,24 +6,13 @@ // //===----------------------------------------------------------------------===// -#include "Plugins/Platform/MacOSX/PlatformMacOSX.h" -#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h" #include "TestingSupport/SubsystemRAII.h" #include "TestingSupport/TestUtilities.h" #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBBreakpointLocation.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBTarget.h" -#include "lldb/Breakpoint/StoppointCallbackContext.h" -#include "lldb/Core/Debugger.h" -#include "lldb/Core/Progress.h" -#include "lldb/Host/FileSystem.h" -#include "lldb/Host/HostInfo.h" -#include "lldb/Target/ExecutionContext.h" -#include "lldb/lldb-private-enumerations.h" -#include "lldb/lldb-types.h" #include "gtest/gtest.h" -#include #include #include @@ -34,12 +22,12 @@ using namespace lldb; class BreakpointClearConditionTest : public ::testing::Test { public: void SetUp() override { - std::call_once(TestUtilities::g_debugger_initialize_flag, - []() { SBDebugger::Initialize(); }); + m_sb_debugger = SBDebugger::Create(/*source_init_files=*/ false); }; + void TearDown() override { SBDebugger::Destroy(m_sb_debugger); } SBDebugger m_sb_debugger; - SubsystemRAII subsystems; + SubsystemRAII subsystems; }; template @@ -60,9 +48,6 @@ void test_condition(T sb_object) { } TEST_F(BreakpointClearConditionTest, BreakpointClearConditionTest) { - // Set up the debugger, make sure that was done properly. - m_sb_debugger = SBDebugger::Create(false); - // Create target SBTarget sb_target; SBError error; diff --git a/lldb/unittests/Breakpoint/CMakeLists.txt b/lldb/unittests/Breakpoint/CMakeLists.txt index 005cb4787c365..3e4161313cd9d 100644 --- a/lldb/unittests/Breakpoint/CMakeLists.txt +++ b/lldb/unittests/Breakpoint/CMakeLists.txt @@ -1,6 +1,5 @@ add_lldb_unittest(LLDBBreakpointTests BreakpointIDTest.cpp - BreakpointClearConditionTest.cpp WatchpointAlgorithmsTests.cpp LINK_COMPONENTS From bbfd5f5cca878580f9a8e398c27a6357b878df64 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 7 Oct 2025 14:45:13 -0700 Subject: [PATCH 3/7] formatting --- .../API/SBBreakpointClearConditionTest.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/lldb/unittests/API/SBBreakpointClearConditionTest.cpp b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp index a89e3848ec9d0..766640f0bdc40 100644 --- a/lldb/unittests/API/SBBreakpointClearConditionTest.cpp +++ b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp @@ -22,7 +22,7 @@ using namespace lldb; class BreakpointClearConditionTest : public ::testing::Test { public: void SetUp() override { - m_sb_debugger = SBDebugger::Create(/*source_init_files=*/ false); + m_sb_debugger = SBDebugger::Create(/*source_init_files=*/false); }; void TearDown() override { SBDebugger::Destroy(m_sb_debugger); } @@ -30,8 +30,7 @@ class BreakpointClearConditionTest : public ::testing::Test { SubsystemRAII subsystems; }; -template -void test_condition(T sb_object) { +template void test_condition(T sb_object) { const char *in_cond_str = "Here is a condition"; sb_object.SetCondition(in_cond_str); // Make sure we set the condition correctly: @@ -44,27 +43,26 @@ void test_condition(T sb_object) { out_cond_str = sb_object.GetCondition(); // And make sure an unset condition returns nullptr: EXPECT_EQ(nullptr, out_cond_str); - } + } } TEST_F(BreakpointClearConditionTest, BreakpointClearConditionTest) { // Create target SBTarget sb_target; SBError error; - sb_target = m_sb_debugger.CreateTarget("", "x86_64-apple-macosx-", "remote-macosx", - /*add_dependent=*/ false, error); + sb_target = + m_sb_debugger.CreateTarget("", "x86_64-apple-macosx-", "remote-macosx", + /*add_dependent=*/false, error); EXPECT_EQ(sb_target.IsValid(), true); // Create breakpoint - SBBreakpoint sb_breakpoint = - sb_target.BreakpointCreateByAddress(0xDEADBEEF); + SBBreakpoint sb_breakpoint = sb_target.BreakpointCreateByAddress(0xDEADBEEF); test_condition(sb_breakpoint); - + // Address breakpoints always have one location, so we can also use this // to test the location: SBBreakpointLocation sb_loc = sb_breakpoint.GetLocationAtIndex(0); EXPECT_EQ(sb_loc.IsValid(), true); test_condition(sb_loc); - } From 13674b39276715673151d0f49cc373dba22a6ebd Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 7 Oct 2025 17:27:07 -0700 Subject: [PATCH 4/7] Remove an unneeded header that ended up including the lldb private headers. This works around the -Wdocumentation error when building the test. --- lldb/unittests/API/SBBreakpointClearConditionTest.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/unittests/API/SBBreakpointClearConditionTest.cpp b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp index 766640f0bdc40..3ec9b341fe662 100644 --- a/lldb/unittests/API/SBBreakpointClearConditionTest.cpp +++ b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "TestingSupport/SubsystemRAII.h" -#include "TestingSupport/TestUtilities.h" #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBBreakpointLocation.h" #include "lldb/API/SBDebugger.h" From 84d6611ee3f188be22e551d2b72f560bdf8300a8 Mon Sep 17 00:00:00 2001 From: jimingham Date: Tue, 7 Oct 2025 17:47:25 -0700 Subject: [PATCH 5/7] Update lldb/unittests/API/SBBreakpointClearConditionTest.cpp Use the LLDB.h so the test can test documentation as a side-effect. Co-authored-by: Jonas Devlieghere --- lldb/unittests/API/SBBreakpointClearConditionTest.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lldb/unittests/API/SBBreakpointClearConditionTest.cpp b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp index 3ec9b341fe662..4405aebf2fbb7 100644 --- a/lldb/unittests/API/SBBreakpointClearConditionTest.cpp +++ b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp @@ -5,12 +5,8 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===----------------------------------------------------------------------===// - -#include "TestingSupport/SubsystemRAII.h" -#include "lldb/API/SBBreakpoint.h" -#include "lldb/API/SBBreakpointLocation.h" -#include "lldb/API/SBDebugger.h" -#include "lldb/API/SBTarget.h" +// Use the umbrella header for -Wdocumentation. +#include "lldb/API/LLDB.h" #include "gtest/gtest.h" #include #include From a99811ec617513536a3289666487e74ac15dc3d3 Mon Sep 17 00:00:00 2001 From: jimingham Date: Tue, 7 Oct 2025 17:48:10 -0700 Subject: [PATCH 6/7] Update lldb/unittests/API/SBBreakpointClearConditionTest.cpp Correct the dash pattern. Co-authored-by: Jonas Devlieghere --- lldb/unittests/API/SBBreakpointClearConditionTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/unittests/API/SBBreakpointClearConditionTest.cpp b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp index 4405aebf2fbb7..df29e9881b1e1 100644 --- a/lldb/unittests/API/SBBreakpointClearConditionTest.cpp +++ b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp @@ -1,4 +1,4 @@ -//--------------------------------------------===// +//===----------------------------------------------------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. From 113e0646204a03e1906faee9f6823a8adddeef8d Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Wed, 8 Oct 2025 10:17:13 -0700 Subject: [PATCH 7/7] Repair a bad merge from accepting a GitHub suggested change in the GitHub UI. --- lldb/unittests/API/SBBreakpointClearConditionTest.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lldb/unittests/API/SBBreakpointClearConditionTest.cpp b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp index df29e9881b1e1..993f7f90d97c0 100644 --- a/lldb/unittests/API/SBBreakpointClearConditionTest.cpp +++ b/lldb/unittests/API/SBBreakpointClearConditionTest.cpp @@ -7,6 +7,12 @@ //===----------------------------------------------------------------------===// // Use the umbrella header for -Wdocumentation. #include "lldb/API/LLDB.h" + +#include "TestingSupport/SubsystemRAII.h" +#include "lldb/API/SBBreakpoint.h" +#include "lldb/API/SBBreakpointLocation.h" +#include "lldb/API/SBDebugger.h" +#include "lldb/API/SBTarget.h" #include "gtest/gtest.h" #include #include