Skip to content

Commit 14bf8a7

Browse files
committed
lldb: guard stale SBProcess target access
SBSaveCoreOptions can retain a ProcessSP after DeleteTarget destroys the Target, leaving Process with a weak_ptr<Target> whose pointee has already been destroyed. Stale SBProcess handles then reach through Process::GetTarget() and crash while dereferencing that expired target relationship. Add a nullable Process::GetTargetSP() helper for SB API callers, use it to make SBProcess::GetTarget() return an invalid target and make SBProcess::SaveCore() fail with an error when the target has already been deleted. Add an API regression test that keeps a core-backed process alive through SBSaveCoreOptions, deletes the target, and verifies both paths fail safely instead of crashing.
1 parent 8de4c61 commit 14bf8a7

4 files changed

Lines changed: 123 additions & 7 deletions

File tree

lldb/include/lldb/Target/Process.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,6 +1244,11 @@ class Process : public std::enable_shared_from_this<Process>,
12441244

12451245
/// Get the target object pointer for this module.
12461246
///
1247+
/// \return
1248+
/// A Target object pointer to the target that owns this
1249+
/// module.
1250+
lldb::TargetSP GetTargetSP() const { return m_target_wp.lock(); }
1251+
12471252
/// \return
12481253
/// A Target object pointer to the target that owns this
12491254
/// module.

lldb/source/API/SBProcess.cpp

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -241,12 +241,15 @@ SBTarget SBProcess::GetTarget() const {
241241
LLDB_INSTRUMENT_VA(this);
242242

243243
SBTarget sb_target;
244-
TargetSP target_sp;
245244
ProcessSP process_sp(GetSP());
246-
if (process_sp) {
247-
target_sp = process_sp->GetTarget().shared_from_this();
248-
sb_target.SetSP(target_sp);
249-
}
245+
if (!process_sp)
246+
return sb_target;
247+
248+
TargetSP target_sp = process_sp->GetTargetSP();
249+
if (!target_sp)
250+
return sb_target;
251+
252+
sb_target.SetSP(target_sp);
250253

251254
return sb_target;
252255
}
@@ -1274,8 +1277,14 @@ lldb::SBError SBProcess::SaveCore(SBSaveCoreOptions &options) {
12741277
return error;
12751278
}
12761279

1277-
std::lock_guard<std::recursive_mutex> guard(
1278-
process_sp->GetTarget().GetAPIMutex());
1280+
TargetSP target_sp = process_sp->GetTargetSP();
1281+
if (!target_sp) {
1282+
error = Status::FromErrorString(
1283+
"SBProcess is invalid because its target has been deleted");
1284+
return error;
1285+
}
1286+
1287+
std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
12791288

12801289
if (process_sp->GetState() != eStateStopped) {
12811290
error = Status::FromErrorString("the process is not stopped");

lldb/unittests/API/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ add_lldb_unittest(APITests
33
SBLineEntryTest.cpp
44
SBMutexTest.cpp
55
SBBreakpointClearConditionTest.cpp
6+
SBProcessDeleteTargetTest.cpp
67

78
SBAPITEST
89

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// Use the umbrella header for -Wdocumentation.
10+
#include "lldb/API/LLDB.h"
11+
12+
#include "TestingSupport/SubsystemRAII.h"
13+
#include "llvm/ADT/SmallString.h"
14+
#include "llvm/ADT/StringRef.h"
15+
#include "llvm/Support/FileSystem.h"
16+
#include "llvm/Support/Path.h"
17+
#include "gtest/gtest.h"
18+
19+
using namespace lldb;
20+
using namespace lldb_private;
21+
22+
namespace {
23+
24+
class SBProcessDeleteTargetTest : public testing::Test {
25+
protected:
26+
void SetUp() override {
27+
debugger = SBDebugger::Create(/*source_init_files=*/false);
28+
}
29+
30+
void TearDown() override { SBDebugger::Destroy(debugger); }
31+
32+
static bool DebuggerSupportsLLVMTarget(llvm::StringRef target) {
33+
SBStructuredData data = SBDebugger::GetBuildConfiguration()
34+
.GetValueForKey("targets")
35+
.GetValueForKey("value");
36+
for (size_t i = 0; i < data.GetSize(); ++i) {
37+
char buf[100] = {0};
38+
size_t size = data.GetItemAtIndex(i).GetStringValue(buf, sizeof(buf));
39+
if (llvm::StringRef(buf, size) == target)
40+
return true;
41+
}
42+
return false;
43+
}
44+
45+
static std::string GetCoreFixturePath(llvm::StringRef name) {
46+
llvm::SmallString<256> path(__FILE__);
47+
llvm::sys::path::remove_filename(path);
48+
llvm::sys::path::append(path, "..", "..", "test", "API");
49+
llvm::sys::path::append(path, "tools", "lldb-dap", "coreFile", name);
50+
return std::string(path.str());
51+
}
52+
53+
void LoadCore(SBTarget &target, SBProcess &process) {
54+
std::string binary_path = GetCoreFixturePath("linux-x86_64.out");
55+
std::string core_path = GetCoreFixturePath("linux-x86_64.core");
56+
ASSERT_TRUE(llvm::sys::fs::exists(binary_path));
57+
ASSERT_TRUE(llvm::sys::fs::exists(core_path));
58+
59+
SBError error;
60+
target = debugger.CreateTarget(binary_path.c_str(), /*target_triple=*/"",
61+
/*platform_name=*/"",
62+
/*add_dependent_modules=*/false, error);
63+
EXPECT_TRUE(target);
64+
EXPECT_TRUE(error.Success()) << error.GetCString();
65+
debugger.SetSelectedTarget(target);
66+
67+
process = target.LoadCore(core_path.c_str());
68+
EXPECT_TRUE(process);
69+
}
70+
71+
SubsystemRAII<lldb::SBDebugger> subsystems;
72+
SBDebugger debugger;
73+
};
74+
75+
} // namespace
76+
77+
TEST_F(SBProcessDeleteTargetTest, GetTargetAndSaveCoreFailSafelyAfterDelete) {
78+
if (!DebuggerSupportsLLVMTarget("X86"))
79+
GTEST_SKIP() << "X86 target is not enabled";
80+
81+
SBTarget target;
82+
SBProcess process;
83+
LoadCore(target, process);
84+
ASSERT_TRUE(target.IsValid());
85+
ASSERT_TRUE(process.IsValid());
86+
ASSERT_EQ(process.GetState(), eStateStopped);
87+
88+
SBSaveCoreOptions options;
89+
options.SetProcess(process);
90+
91+
ASSERT_TRUE(debugger.DeleteTarget(target));
92+
ASSERT_FALSE(target.IsValid());
93+
EXPECT_FALSE(process.IsValid());
94+
95+
EXPECT_FALSE(process.GetTarget().IsValid());
96+
97+
SBError error = process.SaveCore(options);
98+
EXPECT_TRUE(error.Fail());
99+
EXPECT_STREQ("SBProcess is invalid because its target has been deleted",
100+
error.GetCString());
101+
}

0 commit comments

Comments
 (0)