llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: None (jimingham) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/162370.diff 4 Files Affected: - (modified) lldb/source/API/SBBreakpoint.cpp (+9-2) - (modified) lldb/source/API/SBBreakpointLocation.cpp (+9-2) - (added) lldb/unittests/Breakpoint/BreakpointClearConditionTest.cpp (+85) - (modified) lldb/unittests/Breakpoint/CMakeLists.txt (+6-1) ``````````diff 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 ) `````````` </details> https://github.com/llvm/llvm-project/pull/162370 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
