Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions lldb/source/API/SBBreakpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand All @@ -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) {
Expand Down
11 changes: 9 additions & 2 deletions lldb/source/API/SBBreakpointLocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand All @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions lldb/unittests/API/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ add_lldb_unittest(APITests
SBCommandInterpreterTest.cpp
SBLineEntryTest.cpp
SBMutexTest.cpp
SBBreakpointClearConditionTest.cpp

LINK_LIBS
liblldb
Expand Down
69 changes: 69 additions & 0 deletions lldb/unittests/API/SBBreakpointClearConditionTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//===----------------------------------------------------------------------===//
//
// 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
//
//===----------------------------------------------------------------------===//
// 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 <memory>
#include <mutex>

using namespace lldb_private;
using namespace lldb;

class BreakpointClearConditionTest : public ::testing::Test {
public:
void SetUp() override {
m_sb_debugger = SBDebugger::Create(/*source_init_files=*/false);
};

void TearDown() override { SBDebugger::Destroy(m_sb_debugger); }
SBDebugger m_sb_debugger;
SubsystemRAII<lldb::SBDebugger> 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) {
// 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);
}
6 changes: 5 additions & 1 deletion lldb/unittests/Breakpoint/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
add_lldb_unittest(LLDBBreakpointTests
add_lldb_unittest(LLDBBreakpointTests
BreakpointIDTest.cpp
WatchpointAlgorithmsTests.cpp

LINK_COMPONENTS
Support
LINK_LIBS
liblldb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the API unit tests are allowed to link liblldb.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not enforced, as in the test builds and runs successfully where it is...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no problem moving it, but if you were expecting this to have been a real as opposed to a theoretical thing, it isn't...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if this is a rule, it would be good to have it documented somewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you mix static and dynamic libraries that contain the same code, you end up with two copies of the same symbols which leads to various hard-to-debug issues (like having two copies of globals). When you say it "works" you mean you don't notice any of these issues for this particular test, but that doesn't make it any less wrong :-)

I like the idea of enforcing this at the CMake layer. I've filed: #162378

lldbBreakpoint
lldbCore
LLVMTestingSupport
lldbUtilityHelpers
lldbPluginPlatformMacOSX
)
Loading