Skip to content

Commit 69545a2

Browse files
da-viperda-viper
committed
[lldb-dap] Fix source references
The protocol expects that `sourceReference` be less than `(2^31)-1`, but we currently represent memory address as source reference, this can be truncated either when sending through json or by the client. Instead, generate new source references based on the memory address. Make the `CreateSource` function return an optional source. # Conflicts: # lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
1 parent 0662045 commit 69545a2

16 files changed

+152
-63
lines changed

‎lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,19 +67,19 @@ def test_break_on_invalid_source_reference(self):
6767
"Invalid sourceReference.",
6868
)
6969

70-
# Verify that setting a breakpoint on a source reference without a symbol also fails
70+
# Verify that setting a breakpoint on a source reference that is not created fails
7171
response = self.dap_server.request_setBreakpoints(
72-
Source(source_reference=0), [1]
72+
Source(source_reference=200), [1]
7373
)
7474
self.assertIsNotNone(response)
7575
breakpoints = response["body"]["breakpoints"]
7676
self.assertEqual(len(breakpoints), 1)
77-
breakpoint = breakpoints[0]
77+
break_point = breakpoints[0]
7878
self.assertFalse(
79-
breakpoint["verified"], "Expected breakpoint to not be verified"
79+
break_point["verified"], "Expected breakpoint to not be verified"
8080
)
81-
self.assertIn("message", breakpoint, "Expected message to be present")
81+
self.assertIn("message", break_point, "Expected message to be present")
8282
self.assertEqual(
83-
breakpoint["message"],
84-
"Breakpoints in assembly without a valid symbol are not supported yet.",
83+
break_point["message"],
84+
"Invalid sourceReference.",
8585
)

‎lldb/tools/lldb-dap/Breakpoint.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,11 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() {
6464
"0x" + llvm::utohexstr(bp_addr.GetLoadAddress(m_bp.GetTarget()));
6565
breakpoint.instructionReference = formatted_addr;
6666

67-
auto source = CreateSource(bp_addr, m_dap.target);
68-
if (!IsAssemblySource(source)) {
67+
std::optional<protocol::Source> source =
68+
CreateSource(bp_addr, m_dap.target, [this](lldb::addr_t addr) {
69+
return m_dap.CreateSourceReference(addr);
70+
});
71+
if (source && !IsAssemblySource(*source)) {
6972
auto line_entry = bp_addr.GetLineEntry();
7073
const auto line = line_entry.GetLine();
7174
if (line != LLDB_INVALID_LINE_NUMBER)

‎lldb/tools/lldb-dap/DAP.cpp‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,25 @@ DAP::SendFormattedOutput(OutputType o, const char *format, ...) {
497497
o, llvm::StringRef(buffer, std::min<int>(actual_length, sizeof(buffer))));
498498
}
499499

500+
int32_t DAP::CreateSourceReference(lldb::addr_t address) {
501+
auto iter = llvm::find(source_references, address);
502+
if (iter != source_references.end())
503+
return std::distance(source_references.begin(), iter) + 1;
504+
505+
source_references.emplace_back(address);
506+
return static_cast<int32_t>(source_references.size());
507+
}
508+
509+
std::optional<lldb::addr_t> DAP::GetSourceReferenceAddress(int32_t reference) {
510+
if (reference <= LLDB_DAP_INVALID_SRC_REF)
511+
return std::nullopt;
512+
513+
if (static_cast<size_t>(reference) > source_references.size())
514+
return std::nullopt;
515+
516+
return source_references[reference - 1];
517+
}
518+
500519
ExceptionBreakpoint *DAP::GetExceptionBPFromStopReason(lldb::SBThread &thread) {
501520
const auto num = thread.GetStopReasonDataCount();
502521
// Check to see if have hit an exception breakpoint and change the

‎lldb/tools/lldb-dap/DAP.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,9 @@ struct DAP {
105105
/// Map step in target id to list of function targets that user can choose.
106106
llvm::DenseMap<lldb::addr_t, std::string> step_in_targets;
107107

108+
/// list of addresses mapped by sourceReference(index - 1)
109+
std::vector<lldb::addr_t> source_references;
110+
108111
/// A copy of the last LaunchRequest so we can reuse its arguments if we get a
109112
/// RestartRequest. Restarting an AttachRequest is not supported.
110113
std::optional<protocol::LaunchRequestArguments> last_launch_request;
@@ -219,7 +222,9 @@ struct DAP {
219222
void __attribute__((format(printf, 3, 4)))
220223
SendFormattedOutput(OutputType o, const char *format, ...);
221224

222-
static int64_t GetNextSourceReference();
225+
int32_t CreateSourceReference(lldb::addr_t address);
226+
227+
std::optional<lldb::addr_t> GetSourceReferenceAddress(int32_t reference);
223228

224229
ExceptionBreakpoint *GetExceptionBPFromStopReason(lldb::SBThread &thread);
225230

‎lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ static lldb::SBAddress GetDisassembleStartAddress(lldb::SBTarget target,
8585
}
8686

8787
static DisassembledInstruction ConvertSBInstructionToDisassembledInstruction(
88-
lldb::SBTarget &target, lldb::SBInstruction &inst, bool resolve_symbols) {
88+
DAP &dap, lldb::SBInstruction &inst, bool resolve_symbols) {
89+
lldb::SBTarget target = dap.target;
8990
if (!inst.IsValid())
9091
return GetInvalidInstruction();
9192

@@ -138,14 +139,17 @@ static DisassembledInstruction ConvertSBInstructionToDisassembledInstruction(
138139
si << " ; " << c;
139140
}
140141

141-
protocol::Source source = CreateSource(addr, target);
142+
std::optional<protocol::Source> source =
143+
CreateSource(addr, target, [&dap](lldb::addr_t addr) {
144+
return dap.CreateSourceReference(addr);
145+
});
142146
lldb::SBLineEntry line_entry = GetLineEntryForAddress(target, addr);
143147

144148
// If the line number is 0 then the entry represents a compiler generated
145149
// location.
146-
if (!IsAssemblySource(source) && line_entry.GetStartAddress() == addr &&
147-
line_entry.IsValid() && line_entry.GetFileSpec().IsValid() &&
148-
line_entry.GetLine() != 0) {
150+
if (source && !IsAssemblySource(*source) &&
151+
line_entry.GetStartAddress() == addr && line_entry.IsValid() &&
152+
line_entry.GetFileSpec().IsValid() && line_entry.GetLine() != 0) {
149153

150154
disassembled_inst.location = std::move(source);
151155
const auto line = line_entry.GetLine();
@@ -221,7 +225,7 @@ DisassembleRequestHandler::Run(const DisassembleArguments &args) const {
221225
original_address_index = i;
222226

223227
instructions.push_back(ConvertSBInstructionToDisassembledInstruction(
224-
dap.target, inst, resolve_symbols));
228+
dap, inst, resolve_symbols));
225229
}
226230

227231
// Check if we miss instructions at the beginning.

‎lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,16 @@ void LocationsRequestHandler::operator()(
137137
return;
138138
}
139139

140-
body.try_emplace("source", CreateSource(line_entry.GetFileSpec()));
140+
const std::optional<protocol::Source> source =
141+
CreateSource(line_entry.GetFileSpec());
142+
if (!source) {
143+
response["success"] = false;
144+
response["message"] = "Failed to resolve file path for location";
145+
dap.SendJSON(llvm::json::Value(std::move(response)));
146+
return;
147+
}
148+
149+
body.try_emplace("source", *source);
141150
if (int line = line_entry.GetLine())
142151
body.try_emplace("line", line);
143152
if (int column = line_entry.GetColumn())
@@ -152,7 +161,16 @@ void LocationsRequestHandler::operator()(
152161
return;
153162
}
154163

155-
body.try_emplace("source", CreateSource(decl.GetFileSpec()));
164+
const std::optional<protocol::Source> source =
165+
CreateSource(decl.GetFileSpec());
166+
if (!source) {
167+
response["success"] = false;
168+
response["message"] = "Failed to resolve file path for location";
169+
dap.SendJSON(llvm::json::Value(std::move(response)));
170+
return;
171+
}
172+
173+
body.try_emplace("source", *source);
156174
if (int line = decl.GetLine())
157175
body.try_emplace("line", line);
158176
if (int column = decl.GetColumn())

‎lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,34 +29,42 @@ namespace lldb_dap {
2929
/// the source code for a given source reference.
3030
llvm::Expected<protocol::SourceResponseBody>
3131
SourceRequestHandler::Run(const protocol::SourceArguments &args) const {
32-
const auto source =
32+
33+
uint32_t source_ref =
3334
args.source->sourceReference.value_or(args.sourceReference);
35+
const std::optional<lldb::addr_t> source_addr_opt =
36+
dap.GetSourceReferenceAddress(source_ref);
3437

35-
if (!source)
38+
if (!source_addr_opt)
3639
return llvm::make_error<DAPError>(
37-
"invalid arguments, expected source.sourceReference to be set");
40+
llvm::formatv("Unknown source reference {}", source_ref));
3841

39-
lldb::SBAddress address(source, dap.target);
42+
lldb::SBAddress address(*source_addr_opt, dap.target);
4043
if (!address.IsValid())
4144
return llvm::make_error<DAPError>("source not found");
4245

4346
lldb::SBSymbol symbol = address.GetSymbol();
44-
45-
lldb::SBStream stream;
46-
lldb::SBExecutionContext exe_ctx(dap.target);
47+
lldb::SBInstructionList insts;
4748

4849
if (symbol.IsValid()) {
49-
lldb::SBInstructionList insts = symbol.GetInstructions(dap.target);
50-
insts.GetDescription(stream, exe_ctx);
50+
insts = symbol.GetInstructions(dap.target);
5151
} else {
5252
// No valid symbol, just return the disassembly.
53-
lldb::SBInstructionList insts = dap.target.ReadInstructions(
53+
insts = dap.target.ReadInstructions(
5454
address, dap.k_number_of_assembly_lines_for_nodebug);
55-
insts.GetDescription(stream, exe_ctx);
5655
}
5756

57+
if (!insts || insts.GetSize() == 0)
58+
return llvm::make_error<DAPError>(
59+
llvm::formatv("no instruction source for address {}",
60+
address.GetLoadAddress(dap.target)));
61+
62+
lldb::SBStream stream;
63+
lldb::SBExecutionContext exe_ctx(dap.target);
64+
insts.GetDescription(stream, exe_ctx);
5865
return protocol::SourceResponseBody{/*content=*/stream.GetData(),
59-
/*mimeType=*/"text/x-lldb.disassembly"};
66+
/*mimeType=*/
67+
"text/x-lldb.disassembly"};
6068
}
6169

6270
} // namespace lldb_dap

‎lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
6969
break;
7070
}
7171

72-
stack_frames.emplace_back(CreateStackFrame(frame, frame_format));
72+
stack_frames.emplace_back(CreateStackFrame(dap, frame, frame_format));
7373
}
7474

7575
if (include_all && reached_end_of_stack) {

‎lldb/tools/lldb-dap/JSONUtils.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ llvm::json::Object CreateEventObject(const llvm::StringRef event_name) {
543543
// },
544544
// "required": [ "id", "name", "line", "column" ]
545545
// }
546-
llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
546+
llvm::json::Value CreateStackFrame(DAP &dap, lldb::SBFrame &frame,
547547
lldb::SBFormat &format) {
548548
llvm::json::Object object;
549549
int64_t frame_id = MakeDAPFrameID(frame);
@@ -573,8 +573,11 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
573573
EmplaceSafeString(object, "name", frame_name);
574574

575575
auto target = frame.GetThread().GetProcess().GetTarget();
576-
auto source = CreateSource(frame.GetPCAddress(), target);
577-
if (!IsAssemblySource(source)) {
576+
std::optional<protocol::Source> source =
577+
CreateSource(frame.GetPCAddress(), target, [&dap](lldb::addr_t addr) {
578+
return dap.CreateSourceReference(addr);
579+
});
580+
if (source && !IsAssemblySource(*source)) {
578581
// This is a normal source with a valid line entry.
579582
auto line_entry = frame.GetLineEntry();
580583
object.try_emplace("line", line_entry.GetLine());
@@ -598,7 +601,8 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
598601
object.try_emplace("column", 1);
599602
}
600603

601-
object.try_emplace("source", std::move(source));
604+
if (source)
605+
object.try_emplace("source", std::move(source).value());
602606

603607
const auto pc = frame.GetPC();
604608
if (pc != LLDB_INVALID_ADDRESS) {

‎lldb/tools/lldb-dap/JSONUtils.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,9 @@ llvm::json::Object CreateEventObject(const llvm::StringRef event_name);
234234
/// "line" - the source file line number as an integer
235235
/// "column" - the source file column number as an integer
236236
///
237+
/// \param[in] dap
238+
/// The DAP session associated with the stopped thread.
239+
///
237240
/// \param[in] frame
238241
/// The LLDB stack frame to use when populating out the "StackFrame"
239242
/// object.
@@ -245,7 +248,7 @@ llvm::json::Object CreateEventObject(const llvm::StringRef event_name);
245248
/// \return
246249
/// A "StackFrame" JSON object with that follows the formal JSON
247250
/// definition outlined by Microsoft.
248-
llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
251+
llvm::json::Value CreateStackFrame(DAP &dap, lldb::SBFrame &frame,
249252
lldb::SBFormat &format);
250253

251254
/// Create a "StackFrame" label object for a LLDB thread.

0 commit comments

Comments
 (0)