[Lldb-commits] [lldb] Make SBBreakpoint/SBBreakpointLocation.SetCondition(nullptr) work again. (PR #162370)

via lldb-commits lldb-commits at lists.llvm.org
Tue Oct 7 14:45:32 PDT 2025


https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/162370

>From ee58ec0f67b5ef70d7c11756f5931ad08ed7c4dc Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Tue, 7 Oct 2025 13:44:45 -0700
Subject: [PATCH 1/3] 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<std::recursive_mutex> 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<std::recursive_mutex> 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<std::recursive_mutex> 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<std::recursive_mutex> 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 <iostream>
+#include <memory>
+#include <mutex>
+
+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<FileSystem, HostInfo, PlatformMacOSX> subsystems;
+};
+
+template<typename T>
+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 <jingham at apple.com>
Date: Tue, 7 Oct 2025 14:40:03 -0700
Subject: [PATCH 2/3] 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 <iostream>
 #include <memory>
 #include <mutex>
 
@@ -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<FileSystem, HostInfo, PlatformMacOSX> subsystems;
+  SubsystemRAII<lldb::SBDebugger> subsystems;
 };
 
 template<typename T>
@@ -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 <jingham at apple.com>
Date: Tue, 7 Oct 2025 14:45:13 -0700
Subject: [PATCH 3/3] 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<lldb::SBDebugger> subsystems;
 };
 
-template<typename T>
-void test_condition(T sb_object) {
+template <typename T> 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);
-  
 }



More information about the lldb-commits mailing list