[lldb] Fix thread backtrace --count
#83602
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The help output for `thread backtrace` specifies that you can pass -1 to
`--count` to display all the frames.
```
-c <count> ( --count <count> )
How many frames to display (-1 for all)
```
However, that doesn't work:
```
(lldb) thread backtrace --count -1
error: invalid integer value for option 'c'
```
The problem is that we store the option value as an unsigned and the
code to parse the string correctly rejects it. There's two ways to fix
this:
1. Make `m_count` a signed value so that it accepts negative values and
appease the parser. The function that prints the frames takes an
unsigned so a negative value will just become a really large
positive value, which is what the current implementation relies on.
2. Keep `m_count` unsigned and instead use 0 the magic value to show
all frames. I don't really see a point in not showing any frames at
all, plus that's already broken (`error: error displaying backtrace
for thread: "0x0001"`).
This patch implements (2) and at the same time improve the error
reporting so that we print the invalid value when we cannot parse it.
rdar://123881767
|
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesThe help output for However, that doesn't work: The problem is that we store the option value as an unsigned and the code to parse the string correctly rejects it. There's two ways to fix this:
This patch implements (2) and at the same time improve the error reporting so that we print the invalid value when we cannot parse it. rdar://123881767 Full diff: https://github.com/llvm/llvm-project/pull/83602.diff 3 Files Affected:
diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp
index 9cfff059d6bfa4..6d84315a471d95 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -67,13 +67,18 @@ class CommandObjectThreadBacktrace : public CommandObjectIterateOverThreads {
if (option_arg.getAsInteger(0, m_count)) {
m_count = UINT32_MAX;
error.SetErrorStringWithFormat(
- "invalid integer value for option '%c'", short_option);
+ "invalid integer value for option '%c': %s", short_option,
+ option_arg.data());
}
+ // A count of 0 means all frames.
+ if (m_count == 0)
+ m_count = UINT32_MAX;
break;
case 's':
if (option_arg.getAsInteger(0, m_start))
error.SetErrorStringWithFormat(
- "invalid integer value for option '%c'", short_option);
+ "invalid integer value for option '%c': %s", short_option,
+ option_arg.data());
break;
case 'e': {
bool success;
@@ -81,7 +86,8 @@ class CommandObjectThreadBacktrace : public CommandObjectIterateOverThreads {
OptionArgParser::ToBoolean(option_arg, false, &success);
if (!success)
error.SetErrorStringWithFormat(
- "invalid boolean value for option '%c'", short_option);
+ "invalid boolean value for option '%c': %s", short_option,
+ option_arg.data());
} break;
default:
llvm_unreachable("Unimplemented option");
@@ -228,9 +234,9 @@ class CommandObjectThreadBacktrace : public CommandObjectIterateOverThreads {
thread->GetIndexID());
return false;
}
- if (m_options.m_extended_backtrace) {
- if (!INTERRUPT_REQUESTED(GetDebugger(),
- "Interrupt skipped extended backtrace")) {
+ if (m_options.m_extended_backtrace) {
+ if (!INTERRUPT_REQUESTED(GetDebugger(),
+ "Interrupt skipped extended backtrace")) {
DoExtendedBacktrace(thread, result);
}
}
@@ -272,8 +278,9 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
bool avoid_no_debug =
OptionArgParser::ToBoolean(option_arg, true, &success);
if (!success)
- error.SetErrorStringWithFormat("invalid boolean value for option '%c'",
- short_option);
+ error.SetErrorStringWithFormat(
+ "invalid boolean value for option '%c': %s", short_option,
+ option_arg);
else {
m_step_in_avoid_no_debug = avoid_no_debug ? eLazyBoolYes : eLazyBoolNo;
}
@@ -284,8 +291,9 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
bool avoid_no_debug =
OptionArgParser::ToBoolean(option_arg, true, &success);
if (!success)
- error.SetErrorStringWithFormat("invalid boolean value for option '%c'",
- short_option);
+ error.SetErrorStringWithFormat(
+ "invalid boolean value for option '%c': %s", short_option,
+ option_arg);
else {
m_step_out_avoid_no_debug = avoid_no_debug ? eLazyBoolYes : eLazyBoolNo;
}
@@ -293,8 +301,9 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
case 'c':
if (option_arg.getAsInteger(0, m_step_count))
- error.SetErrorStringWithFormat("invalid step count '%s'",
- option_arg.str().c_str());
+ error.SetErrorStringWithFormat(
+ "invalid integer value for option '%c': %s", short_option,
+ option_arg.data());
break;
case 'm': {
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 91d5eea830dedf..62bbfdc117834f 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -805,7 +805,7 @@ let Command = "script add" in {
def script_add_function : Option<"function", "f">, Group<1>,
Arg<"PythonFunction">,
Desc<"Name of the Python function to bind to this command name.">;
- def script_add_class : Option<"class", "c">, Groups<[2,3]>,
+ def script_add_class : Option<"class", "c">, Groups<[2,3]>,
Arg<"PythonClass">,
Desc<"Name of the Python class to bind to this command name.">;
def script_add_help : Option<"help", "h">, Group<1>, Arg<"HelpText">,
@@ -816,7 +816,7 @@ let Command = "script add" in {
EnumArg<"ScriptedCommandSynchronicity">,
Desc<"Set the synchronicity of this command's executions with regard to "
"LLDB event system.">;
- def script_add_completion_type : Option<"completion-type", "C">,
+ def script_add_completion_type : Option<"completion-type", "C">,
Groups<[1,2]>, EnumArg<"CompletionType">,
Desc<"Specify which completion type the command should use - if none is "
"specified, the command won't use auto-completion.">;
@@ -1037,7 +1037,7 @@ let Command = "target stop hook add" in {
let Command = "thread backtrace" in {
def thread_backtrace_count : Option<"count", "c">, Group<1>, Arg<"Count">,
- Desc<"How many frames to display (-1 for all)">;
+ Desc<"How many frames to display (0 for all)">;
def thread_backtrace_start : Option<"start", "s">, Group<1>,
Arg<"FrameIndex">, Desc<"Frame in which to start the backtrace">;
def thread_backtrace_extended : Option<"extended", "e">, Group<1>,
diff --git a/lldb/test/Shell/Commands/command-thread-backtrace.test b/lldb/test/Shell/Commands/command-thread-backtrace.test
new file mode 100644
index 00000000000000..dacef8d7fa6a8b
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-thread-backtrace.test
@@ -0,0 +1,14 @@
+# RUN: %clang_host -g %S/Inputs/main.c -o %t
+
+# RUN: not %lldb %t -b -o 'b foo' -o 'r' -o 'thread backtrace --count -1' 2>&1 | FileCheck %s --check-prefix COUNT
+# COUNT: error: invalid integer value for option 'c': -1
+
+# RUN: not %lldb %t -b -o 'b foo' -o 'r' -o 'thread backtrace --extended nah' 2>&1 | FileCheck %s --check-prefix EXTENDED
+# EXTENDED: error: invalid boolean value for option 'e': nah
+
+# RUN: not %lldb %t -b -o 'b foo' -o 'r' -o 'thread backtrace --start -1' 2>&1 | FileCheck %s --check-prefix START
+# START: error: invalid integer value for option 's': -1
+
+# RUN: %lldb %t -b -o 'b foo' -o 'r' -o 'thread backtrace --count 0' | FileCheck %s
+# CHECK: frame #0:
+# CHECK: frame #1:
|
jimingham
approved these changes
Mar 1, 2024
JDevlieghere
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Mar 1, 2024
The help output for `thread backtrace` specifies that you can pass -1 to
`--count` to display all the frames.
```
-c <count> ( --count <count> )
How many frames to display (-1 for all)
```
However, that doesn't work:
```
(lldb) thread backtrace --count -1
error: invalid integer value for option 'c'
```
The problem is that we store the option value as an unsigned and the
code to parse the string correctly rejects it. There's two ways to fix
this:
1. Make `m_count` a signed value so that it accepts negative values and
appease the parser. The function that prints the frames takes an
unsigned so a negative value will just become a really large positive
value, which is what the current implementation relies on.
2. Keep `m_count` unsigned and instead use 0 the magic value to show all
frames. I don't really see a point in not showing any frames at all,
plus that's already broken (`error: error displaying backtrace for
thread: "0x0001"`).
This patch implements (2) and at the same time improve the error
reporting so that we print the invalid value when we cannot parse it.
rdar://123881767
(cherry picked from commit af00945)
MaskRay
added a commit
that referenced
this pull request
Mar 1, 2024
JDevlieghere
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Mar 13, 2024
[lldb] Fix `thread backtrace --count` (llvm#83602)
kastiglione
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Mar 14, 2024
(cherry-picked from commit ebaf26d)
kastiglione
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Mar 15, 2024
(cherry-picked from commit ebaf26d)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The help output for
thread backtracespecifies that you can pass -1 to--countto display all the frames.However, that doesn't work:
The problem is that we store the option value as an unsigned and the code to parse the string correctly rejects it. There's two ways to fix this:
m_counta signed value so that it accepts negative values and appease the parser. The function that prints the frames takes an unsigned so a negative value will just become a really large positive value, which is what the current implementation relies on.m_countunsigned and instead use 0 the magic value to show all frames. I don't really see a point in not showing any frames at all, plus that's already broken (error: error displaying backtrace for thread: "0x0001").This patch implements (2) and at the same time improve the error reporting so that we print the invalid value when we cannot parse it.
rdar://123881767