[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
cmtice added a comment. I apologize; I was looking at several commits, and I accidentally added my comment to the wrong one. No, this commit is not the one that caused my problem (I am very sorry about the confusion). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127702/new/ https://reviews.llvm.org/D127702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
yinghuitan added a comment. @cmtice, are you sure the failure is caused by/related with this patch? This is a pure lldb-vscode change which is not used by normal lldb. Also, no mention of code in this patch in the error message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127702/new/ https://reviews.llvm.org/D127702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
cmtice added a comment.
Just FYI, this commit appears to cause
lldb//test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
to fail. The error I'm seeing is:
Traceback (most recent call last):
File
"../lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py",
line 72, in test_target_auto_install_main_executable
self.expect("process launch", substrs=["exited with status = 74"])
File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2295, in
expect
self.runCmd(
File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1998, in
runCmd
self.assertTrue(self.res.Succeeded(),
AssertionError: False is not True : Command 'process launch
Error output:
error: failed to get reply to handshake packet within timeout of 0.0 seconds
' did not return successfully
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127702/new/
https://reviews.llvm.org/D127702
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
yinghuitan added a comment. Fixed in https://reviews.llvm.org/D128234 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127702/new/ https://reviews.llvm.org/D127702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
yinghuitan added a comment. Sorry, I was fooled by the buildbot which says everything is green. Working on a fix for the build break now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127702/new/ https://reviews.llvm.org/D127702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
thakis added a comment. Here too: https://lab.llvm.org/buildbot#builders/83/builds/20210 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127702/new/ https://reviews.llvm.org/D127702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
thakis added a comment. Looks like this might not build: http://45.33.8.238/linux/79194/step_4.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127702/new/ https://reviews.llvm.org/D127702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c6e138aa893: Support logpoints in lldb-vscode (authored by
yinghuitan).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127702/new/
https://reviews.llvm.org/D127702
Files:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_logpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp
lldb/tools/lldb-vscode/BreakpointBase.cpp
lldb/tools/lldb-vscode/BreakpointBase.h
lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
lldb/tools/lldb-vscode/SourceBreakpoint.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1532,6 +1532,8 @@
body.try_emplace("supportsLoadedSourcesRequest", false);
// The debug adapter supports sending progress reporting events.
body.try_emplace("supportsProgressReporting", true);
+ // The debug adapter supports 'logMessage' in breakpoint.
+ body.try_emplace("supportsLogPoints", true);
response.try_emplace("body", std::move(body));
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
@@ -2079,9 +2081,10 @@
}
}
// At this point the breakpoint is new
-src_bp.SetBreakpoint(path.data());
-AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
-g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
+g_vsc.source_breakpoints[path][src_bp.line] = src_bp;
+SourceBreakpoint &new_bp = g_vsc.source_breakpoints[path][src_bp.line];
+new_bp.SetBreakpoint(path.data());
+AppendBreakpoint(new_bp.bp, response_breakpoints, path, new_bp.line);
}
}
}
@@ -2304,10 +2307,11 @@
// Any breakpoints that are left in "request_bps" are breakpoints that
// need to be set.
for (auto &pair : request_bps) {
-pair.second.SetBreakpoint();
// Add this breakpoint info to the response
-AppendBreakpoint(pair.second.bp, response_breakpoints);
g_vsc.function_breakpoints[pair.first()] = std::move(pair.second);
+FunctionBreakpoint &new_bp = g_vsc.function_breakpoints[pair.first()];
+new_bp.SetBreakpoint();
+AppendBreakpoint(new_bp.bp, response_breakpoints);
}
llvm::json::Object body;
Index: lldb/tools/lldb-vscode/SourceBreakpoint.cpp
===
--- lldb/tools/lldb-vscode/SourceBreakpoint.cpp
+++ lldb/tools/lldb-vscode/SourceBreakpoint.cpp
@@ -24,6 +24,8 @@
SetCondition();
if (!hitCondition.empty())
SetHitCondition();
+ if (!logMessage.empty())
+SetLogMessage();
}
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
===
--- lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
+++ lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
@@ -25,6 +25,8 @@
SetCondition();
if (!hitCondition.empty())
SetHitCondition();
+ if (!logMessage.empty())
+SetLogMessage();
}
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/BreakpointBase.h
===
--- lldb/tools/lldb-vscode/BreakpointBase.h
+++ lldb/tools/lldb-vscode/BreakpointBase.h
@@ -13,11 +13,16 @@
#include "lldb/API/SBBreakpoint.h"
#include "llvm/Support/JSON.h"
#include
+#include
namespace lldb_vscode {
struct BreakpointBase {
-
+ // logMessage part can be either a raw text or an expression.
+ struct LogMessagePart {
+llvm::StringRef text;
+bool is_expr;
+ };
// An optional expression for conditional breakpoints.
std::string condition;
// An optional expression that controls how many hits of the breakpoint are
@@ -27,6 +32,7 @@
// (stop) but log the message instead. Expressions within {} are
// interpolated.
std::string logMessage;
+ std::vector logMessageParts;
// The LLDB breakpoint associated wit this source breakpoint
lldb::SBBreakpoint bp;
@@ -35,8 +41,12 @@
void SetCondition();
void SetHitCondition();
+ void SetLogMessage();
void UpdateBreakpoint(const BreakpointBase &request_bp);
static const char *GetBreakpointLabel();
+ static bool BreakpointHitCallback(void *baton, lldb::SBProcess &process,
+lldb::SBThread &thread,
+lldb::SBBreakpointLocation &location);
};
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/BreakpointBase.cpp
===
--- lldb/tools/lldb-vscode/BreakpointBase.cpp
+++ lldb/tools
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
yinghuitan updated this revision to Diff 438448.
yinghuitan added a comment.
Using a single structured LogMessagePart per suggestion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127702/new/
https://reviews.llvm.org/D127702
Files:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_logpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp
lldb/tools/lldb-vscode/BreakpointBase.cpp
lldb/tools/lldb-vscode/BreakpointBase.h
lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
lldb/tools/lldb-vscode/SourceBreakpoint.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1532,6 +1532,8 @@
body.try_emplace("supportsLoadedSourcesRequest", false);
// The debug adapter supports sending progress reporting events.
body.try_emplace("supportsProgressReporting", true);
+ // The debug adapter supports 'logMessage' in breakpoint.
+ body.try_emplace("supportsLogPoints", true);
response.try_emplace("body", std::move(body));
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
@@ -2079,9 +2081,10 @@
}
}
// At this point the breakpoint is new
-src_bp.SetBreakpoint(path.data());
-AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
-g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
+g_vsc.source_breakpoints[path][src_bp.line] = src_bp;
+SourceBreakpoint &new_bp = g_vsc.source_breakpoints[path][src_bp.line];
+new_bp.SetBreakpoint(path.data());
+AppendBreakpoint(new_bp.bp, response_breakpoints, path, new_bp.line);
}
}
}
@@ -2304,10 +2307,11 @@
// Any breakpoints that are left in "request_bps" are breakpoints that
// need to be set.
for (auto &pair : request_bps) {
-pair.second.SetBreakpoint();
// Add this breakpoint info to the response
-AppendBreakpoint(pair.second.bp, response_breakpoints);
g_vsc.function_breakpoints[pair.first()] = std::move(pair.second);
+FunctionBreakpoint &new_bp = g_vsc.function_breakpoints[pair.first()];
+new_bp.SetBreakpoint();
+AppendBreakpoint(new_bp.bp, response_breakpoints);
}
llvm::json::Object body;
Index: lldb/tools/lldb-vscode/SourceBreakpoint.cpp
===
--- lldb/tools/lldb-vscode/SourceBreakpoint.cpp
+++ lldb/tools/lldb-vscode/SourceBreakpoint.cpp
@@ -24,6 +24,8 @@
SetCondition();
if (!hitCondition.empty())
SetHitCondition();
+ if (!logMessage.empty())
+SetLogMessage();
}
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
===
--- lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
+++ lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
@@ -25,6 +25,8 @@
SetCondition();
if (!hitCondition.empty())
SetHitCondition();
+ if (!logMessage.empty())
+SetLogMessage();
}
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/BreakpointBase.h
===
--- lldb/tools/lldb-vscode/BreakpointBase.h
+++ lldb/tools/lldb-vscode/BreakpointBase.h
@@ -13,11 +13,16 @@
#include "lldb/API/SBBreakpoint.h"
#include "llvm/Support/JSON.h"
#include
+#include
namespace lldb_vscode {
struct BreakpointBase {
-
+ // logMessage part can be either a raw text or an expression.
+ struct LogMessagePart {
+llvm::StringRef text;
+bool is_expr;
+ };
// An optional expression for conditional breakpoints.
std::string condition;
// An optional expression that controls how many hits of the breakpoint are
@@ -27,6 +32,7 @@
// (stop) but log the message instead. Expressions within {} are
// interpolated.
std::string logMessage;
+ std::vector logMessageParts;
// The LLDB breakpoint associated wit this source breakpoint
lldb::SBBreakpoint bp;
@@ -35,8 +41,12 @@
void SetCondition();
void SetHitCondition();
+ void SetLogMessage();
void UpdateBreakpoint(const BreakpointBase &request_bp);
static const char *GetBreakpointLabel();
+ static bool BreakpointHitCallback(void *baton, lldb::SBProcess &process,
+lldb::SBThread &thread,
+lldb::SBBreakpointLocation &location);
};
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/BreakpointBase.cpp
===
--- lldb/tools/lldb-vscode/BreakpointBase.cpp
+++ lldb/tools/lldb-vscode/BreakpointBase.cpp
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
clayborg added a comment.
I would like to get to one array solution of LogMessagePart entries and avoid
having two arrays of strings that need to stay in sync (rawTextMessages and
evalExpressions). Simpler to understand and no need to document the
interdependence of rawTextMessages and evalExpressions.
Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:91
+assert(curly_braces_range.first >= last_raw_text_start);
+size_t raw_text_len = curly_braces_range.first - last_raw_text_start;
+rawTextMessages.emplace_back(llvm::StringRef(
yinghuitan wrote:
> clayborg wrote:
> > If the is a '{' at the start of the string, this code seems like it pushes
> > an empty string. this must be needed because we have two vectors
> > (rawTextMessages and evalExpressions)?
> Correct. We assume there is always a prefix raw text before any expressions,
> so rawTextMessages is always one element larger than expressions, I will
> enhance the documentation (see my format comment above):
> ```
> assert(rawTextMessages.size() == evalExpressions.size() + 1);
> ```
I would like to avoid having two arrays that need to stay in sync here if
possible. See comment on using just a single array of "LogMessagePart" entries.
Simpler code without the need to document that rawTextMessages and
evalExpressions need to be in some specific order.
Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:124
+
+ for (size_t i = 0; i < bp->evalExpressions.size(); ++i) {
+const llvm::StringRef &expr = bp->evalExpressions[i];
yinghuitan wrote:
> clayborg wrote:
> > If we use a single vector of LogMessagePart objects, this code becomes
> > simpler:
> > ```
> > for (const LogMessagePart &part: bp->logMessageParts) {
> > if (part.is_expr) {
> > lldb::SBValue value = frame.EvaluateExpression(part.text.str().c_str());
> > const char *expr_result = value.GetValue();
> > if (expr_result)
> > output += expr_result;
> >} else {
> > output += part.text.str();
> >}
> > }
> Can you elaborate how is the suggested code simpler than what we have here?
> The only difference seems to be that the suggested code added a if...else
> branch while the code here does not branch? Seems not much difference.
There is only one array and there is no need to document that the usage of two
arrays and their codependence, such as you are doing with invariants above:
```
assert(bp->rawTextMessages.size() == bp->evalExpressions.size() + 1);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127702/new/
https://reviews.llvm.org/D127702
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
yinghuitan added inline comments.
Herald added a subscriber: Michael137.
Comment at:
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_logpoints.py:60
+[loop_line, after_loop_line],
+[{'logMessage': logMessage}, {}]
+)
clayborg wrote:
> Any reason we have an empty dictionary at the end here?
We are setting two source line breakpoints here, the first one has logMessage
while the second one does not have any condition. I am explicitly using {} for
second breakpoint to be clear.
Comment at:
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_logpoints.py:120
+[loop_line, after_loop_line],
+[{'logMessage': logMessage}, {}]
+)
clayborg wrote:
> Any reason we have an empty dictionary at the end here?
{} means no logMessage for second breakpoint (after_loop_line).
Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:46
+ // ascending order without any overlap between them.
+ std::vector> matched_curly_braces_ranges;
+
clayborg wrote:
> Might be easier to use a std::map here instead of a vector of pairs?
I do not think so. We never need fast key lookup from the data structure. All
the operations are checking/popping/pushing the back of the vector. It is more
like a stack.
Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:91
+assert(curly_braces_range.first >= last_raw_text_start);
+size_t raw_text_len = curly_braces_range.first - last_raw_text_start;
+rawTextMessages.emplace_back(llvm::StringRef(
clayborg wrote:
> If the is a '{' at the start of the string, this code seems like it pushes an
> empty string. this must be needed because we have two vectors
> (rawTextMessages and evalExpressions)?
Correct. We assume there is always a prefix raw text before any expressions, so
rawTextMessages is always one element larger than expressions, I will enhance
the documentation (see my format comment above):
```
assert(rawTextMessages.size() == evalExpressions.size() + 1);
```
Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:124
+
+ for (size_t i = 0; i < bp->evalExpressions.size(); ++i) {
+const llvm::StringRef &expr = bp->evalExpressions[i];
clayborg wrote:
> If we use a single vector of LogMessagePart objects, this code becomes
> simpler:
> ```
> for (const LogMessagePart &part: bp->logMessageParts) {
> if (part.is_expr) {
> lldb::SBValue value = frame.EvaluateExpression(part.text.str().c_str());
> const char *expr_result = value.GetValue();
> if (expr_result)
> output += expr_result;
>} else {
> output += part.text.str();
>}
> }
Can you elaborate how is the suggested code simpler than what we have here? The
only difference seems to be that the suggested code added a if...else branch
while the code here does not branch? Seems not much difference.
Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:128
+lldb::SBValue value = frame.EvaluateExpression(expr.str().c_str());
+output += value.GetValue();
+output += bp->rawTextMessages[i + 1];
clayborg wrote:
> This will crash if "value.GetValue()" returns NULL.
Good catch, will handle null.
Comment at: lldb/tools/lldb-vscode/BreakpointBase.h:31-32
std::string logMessage;
+ std::vector rawTextMessages;
+ std::vector evalExpressions;
// The LLDB breakpoint associated wit this source breakpoint
clayborg wrote:
> clayborg wrote:
> > How do we know what order to emit things with here between
> > "rawTextMessages" and "evalExpressions" in the breakpoint hit callback?
> Seems like it would be easier to understand the code if we used:
> ```
> struct LogMessagePart {
> llvm::StringRef text;
> bool is_expr;
> };
> std::vector logMessageParts;
> ```
>
> Right now you have two arrays and it isn't clear that each of these arrays
> must be the same size, or which one would be emitted first.
Sorry, I think I forgot to document this. The format of the logMessage is
parsed as:
```
prefixRawText/expression1/rawText1/expression2/rawText2/expressionN/rawTextN
```
Basically rawTextMessages wraps expressions. This is enforced by invariant
`assert(rawTextMessages.size() == evalExpressions.size() + 1);` in the cpp
file.
Let me add this as documentation.
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2084-2087
+g_vsc.source_breakpoints[path][src_bp.line] = src_bp;
+SourceBreakpoint &new_bp = g_vsc.source_breakpoints[path][src_bp.line];
+new_bp.SetBreakpoint(path.data());
+AppendBreakpoint(new_bp.bp, response_breakpoints, path, new_bp.line);
clayborg wrote:
> Why was this changed? I seemed clearer before this change and I can't see
> anything that is different here?
This change is required because Breakpoint
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
clayborg added inline comments. Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:126 +const llvm::StringRef &expr = bp->evalExpressions[i]; +// TODO: try local variables first. +lldb::SBValue value = frame.EvaluateExpression(expr.str().c_str()); I would definitely try an local variable paths first via: ``` lldb::SBValue value = frame.GetValueForVariablePath(part.text.str().c_str(), lldb::eDynamicDontRunTarget); ``` before doing the evaluate expression due to performance which will be important in this case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127702/new/ https://reviews.llvm.org/D127702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
Comment at:
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_logpoints.py:60
+[loop_line, after_loop_line],
+[{'logMessage': logMessage}, {}]
+)
Any reason we have an empty dictionary at the end here?
Comment at:
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_logpoints.py:120
+[loop_line, after_loop_line],
+[{'logMessage': logMessage}, {}]
+)
Any reason we have an empty dictionary at the end here?
Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:46
+ // ascending order without any overlap between them.
+ std::vector> matched_curly_braces_ranges;
+
Might be easier to use a std::map here instead of a vector of pairs?
Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:91
+assert(curly_braces_range.first >= last_raw_text_start);
+size_t raw_text_len = curly_braces_range.first - last_raw_text_start;
+rawTextMessages.emplace_back(llvm::StringRef(
If the is a '{' at the start of the string, this code seems like it pushes an
empty string. this must be needed because we have two vectors (rawTextMessages
and evalExpressions)?
Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:124
+
+ for (size_t i = 0; i < bp->evalExpressions.size(); ++i) {
+const llvm::StringRef &expr = bp->evalExpressions[i];
If we use a single vector of LogMessagePart objects, this code becomes simpler:
```
for (const LogMessagePart &part: bp->logMessageParts) {
if (part.is_expr) {
lldb::SBValue value = frame.EvaluateExpression(part.text.str().c_str());
const char *expr_result = value.GetValue();
if (expr_result)
output += expr_result;
} else {
output += part.text.str();
}
}
Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:128
+lldb::SBValue value = frame.EvaluateExpression(expr.str().c_str());
+output += value.GetValue();
+output += bp->rawTextMessages[i + 1];
This will crash if "value.GetValue()" returns NULL.
Comment at: lldb/tools/lldb-vscode/BreakpointBase.h:31-32
std::string logMessage;
+ std::vector rawTextMessages;
+ std::vector evalExpressions;
// The LLDB breakpoint associated wit this source breakpoint
How do we know what order to emit things with here between "rawTextMessages"
and "evalExpressions" in the breakpoint hit callback?
Comment at: lldb/tools/lldb-vscode/BreakpointBase.h:31-32
std::string logMessage;
+ std::vector rawTextMessages;
+ std::vector evalExpressions;
// The LLDB breakpoint associated wit this source breakpoint
clayborg wrote:
> How do we know what order to emit things with here between "rawTextMessages"
> and "evalExpressions" in the breakpoint hit callback?
Seems like it would be easier to understand the code if we used:
```
struct LogMessagePart {
llvm::StringRef text;
bool is_expr;
};
std::vector logMessageParts;
```
Right now you have two arrays and it isn't clear that each of these arrays must
be the same size, or which one would be emitted first.
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2084-2087
+g_vsc.source_breakpoints[path][src_bp.line] = src_bp;
+SourceBreakpoint &new_bp = g_vsc.source_breakpoints[path][src_bp.line];
+new_bp.SetBreakpoint(path.data());
+AppendBreakpoint(new_bp.bp, response_breakpoints, path, new_bp.line);
Why was this changed? I seemed clearer before this change and I can't see
anything that is different here?
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2312-2314
+FunctionBreakpoint &new_bp = g_vsc.function_breakpoints[pair.first()];
+new_bp.SetBreakpoint();
+AppendBreakpoint(new_bp.bp, response_breakpoints);
Why was this changed? Revert?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127702/new/
https://reviews.llvm.org/D127702
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode
yinghuitan created this revision.
yinghuitan added reviewers: clayborg, labath, jingham, jdoerfert, JDevlieghere,
aadsm, kusmour.
Herald added a project: All.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
This patch implements VSCode DAP logpoints feature (also called tracepoint
in other VS debugger).
This will provide a convenient way for user to do printf style logging
debugging without pausing debuggee.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D127702
Files:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_logpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp
lldb/tools/lldb-vscode/BreakpointBase.cpp
lldb/tools/lldb-vscode/BreakpointBase.h
lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
lldb/tools/lldb-vscode/SourceBreakpoint.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1532,6 +1532,8 @@
body.try_emplace("supportsLoadedSourcesRequest", false);
// The debug adapter supports sending progress reporting events.
body.try_emplace("supportsProgressReporting", true);
+ // The debug adapter supports 'logMessage' in breakpoint.
+ body.try_emplace("supportsLogPoints", true);
response.try_emplace("body", std::move(body));
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
@@ -2079,9 +2081,10 @@
}
}
// At this point the breakpoint is new
-src_bp.SetBreakpoint(path.data());
-AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
-g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
+g_vsc.source_breakpoints[path][src_bp.line] = src_bp;
+SourceBreakpoint &new_bp = g_vsc.source_breakpoints[path][src_bp.line];
+new_bp.SetBreakpoint(path.data());
+AppendBreakpoint(new_bp.bp, response_breakpoints, path, new_bp.line);
}
}
}
@@ -2304,10 +2307,11 @@
// Any breakpoints that are left in "request_bps" are breakpoints that
// need to be set.
for (auto &pair : request_bps) {
-pair.second.SetBreakpoint();
// Add this breakpoint info to the response
-AppendBreakpoint(pair.second.bp, response_breakpoints);
g_vsc.function_breakpoints[pair.first()] = std::move(pair.second);
+FunctionBreakpoint &new_bp = g_vsc.function_breakpoints[pair.first()];
+new_bp.SetBreakpoint();
+AppendBreakpoint(new_bp.bp, response_breakpoints);
}
llvm::json::Object body;
Index: lldb/tools/lldb-vscode/SourceBreakpoint.cpp
===
--- lldb/tools/lldb-vscode/SourceBreakpoint.cpp
+++ lldb/tools/lldb-vscode/SourceBreakpoint.cpp
@@ -24,6 +24,8 @@
SetCondition();
if (!hitCondition.empty())
SetHitCondition();
+ if (!logMessage.empty())
+SetLogMessage();
}
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
===
--- lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
+++ lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
@@ -25,6 +25,8 @@
SetCondition();
if (!hitCondition.empty())
SetHitCondition();
+ if (!logMessage.empty())
+SetLogMessage();
}
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/BreakpointBase.h
===
--- lldb/tools/lldb-vscode/BreakpointBase.h
+++ lldb/tools/lldb-vscode/BreakpointBase.h
@@ -13,6 +13,7 @@
#include "lldb/API/SBBreakpoint.h"
#include "llvm/Support/JSON.h"
#include
+#include
namespace lldb_vscode {
@@ -27,6 +28,8 @@
// (stop) but log the message instead. Expressions within {} are
// interpolated.
std::string logMessage;
+ std::vector rawTextMessages;
+ std::vector evalExpressions;
// The LLDB breakpoint associated wit this source breakpoint
lldb::SBBreakpoint bp;
@@ -35,8 +38,12 @@
void SetCondition();
void SetHitCondition();
+ void SetLogMessage();
void UpdateBreakpoint(const BreakpointBase &request_bp);
static const char *GetBreakpointLabel();
+ static bool BreakpointHitCallback(void *baton, lldb::SBProcess &process,
+lldb::SBThread &thread,
+lldb::SBBreakpointLocation &location);
};
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/BreakpointBase.cpp
===
--- lldb/tools/lldb-vscode/BreakpointBase.cpp
+++ lldb/tools/lldb-vscode/BreakpointBase.cpp
@@ -7,6 +7,7 @@
//==
