[Lldb-commits] [lldb] [lldb] Fix `thread backtrace --count` (PR #83602)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Fri Mar 1 10:11:01 PST 2024
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/83602
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
>From 223a8373dd5a5bf5c96cf3311ece39c97dd7b615 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 1 Mar 2024 10:02:25 -0800
Subject: [PATCH] [lldb] Fix `thread backtrace --count`
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
---
lldb/source/Commands/CommandObjectThread.cpp | 33 ++++++++++++-------
lldb/source/Commands/Options.td | 6 ++--
.../Commands/command-thread-backtrace.test | 14 ++++++++
3 files changed, 38 insertions(+), 15 deletions(-)
create mode 100644 lldb/test/Shell/Commands/command-thread-backtrace.test
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:
More information about the lldb-commits
mailing list