[lldb/crashlog] Make interactive mode the new default #94575
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
|
@llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) ChangesThis patch makes interactive mode as the default when using the crashlog command. It replaces the existing By default, when the option is not explicitely set by the user, the interactive mode is selected, however, lldb will fallback to batch mode if the command interpreter is not interactive or if stdout is not a tty. This also adds some railguards to prevent users from using interactive only options with the batch mode and updates the tests accordingly. rdar://97801509 Differential Revision: https://reviews.llvm.org/D141658 Full diff: https://github.com/llvm/llvm-project/pull/94575.diff 6 Files Affected:
diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py
index 5281d6d949baf..9f5d5648192b6 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -31,6 +31,7 @@
import concurrent.futures
import contextlib
import datetime
+import enum
import json
import os
import platform
@@ -45,7 +46,6 @@
import time
import uuid
-
print_lock = threading.RLock()
try:
@@ -1582,9 +1582,12 @@ def synchronous(debugger):
debugger.RunCommandInterpreter(True, False, run_options, 0, False, True)
-def CreateSymbolicateCrashLogOptions(
- command_name, description, add_interactive_options
-):
+class CrashLogLoadingMode(str, enum.Enum):
+ batch = "batch"
+ interactive = "interactive"
+
+
+def CreateSymbolicateCrashLogOptions(command_name, description):
usage = "crashlog [options] <FILE> [FILE ...]"
arg_parser = argparse.ArgumentParser(
description=description,
@@ -1600,6 +1603,13 @@ def CreateSymbolicateCrashLogOptions(
help="crash report(s) to symbolicate",
)
+ arg_parser.add_argument(
+ "-m",
+ "--mode",
+ choices=[mode.value for mode in CrashLogLoadingMode],
+ help="change how the symbolicated process and threads are displayed to the user",
+ default=CrashLogLoadingMode.interactive,
+ )
arg_parser.add_argument(
"--version",
"-V",
@@ -1736,36 +1746,35 @@ def CreateSymbolicateCrashLogOptions(
help=argparse.SUPPRESS,
default=False,
)
- if add_interactive_options:
- arg_parser.add_argument(
- "-i",
- "--interactive",
- action="store_true",
- help="parse a crash log and load it in a ScriptedProcess",
- default=False,
- )
- arg_parser.add_argument(
- "-b",
- "--batch",
- action="store_true",
- help="dump symbolicated stackframes without creating a debug session",
- default=True,
- )
- arg_parser.add_argument(
- "--target",
- "-t",
- dest="target_path",
- help="the target binary path that should be used for interactive crashlog (optional)",
- default=None,
- )
- arg_parser.add_argument(
- "--skip-status",
- "-s",
- dest="skip_status",
- action="store_true",
- help="prevent the interactive crashlog to dump the process status and thread backtrace at launch",
- default=False,
- )
+ arg_parser.add_argument(
+ "--target",
+ "-t",
+ dest="target_path",
+ help="the target binary path that should be used for interactive crashlog (optional)",
+ default=None,
+ )
+ arg_parser.add_argument(
+ "--skip-status",
+ "-s",
+ dest="skip_status",
+ action="store_true",
+ help="prevent the interactive crashlog to dump the process status and thread backtrace at launch",
+ default=False,
+ )
+ legacy_group = arg_parser.add_mutually_exclusive_group()
+ legacy_group.add_argument(
+ "-i",
+ "--interactive",
+ action="store_true",
+ help=argparse.SUPPRESS,
+ )
+ legacy_group.add_argument(
+ "-b",
+ "--batch",
+ action="store_true",
+ help=argparse.SUPPRESS,
+ )
+
return arg_parser
@@ -1778,7 +1787,7 @@ def CrashLogOptionParser():
created that has all of the shared libraries loaded at the load addresses found in the crash log file. This allows
you to explore the program as if it were stopped at the locations described in the crash log and functions can
be disassembled and lookups can be performed using the addresses found in the crash log."""
- return CreateSymbolicateCrashLogOptions("crashlog", description, True)
+ return CreateSymbolicateCrashLogOptions("crashlog", description)
def SymbolicateCrashLogs(debugger, command_args, result, is_command):
@@ -1794,8 +1803,36 @@ def SymbolicateCrashLogs(debugger, command_args, result, is_command):
result.SetError(str(e))
return
+ # To avoid breaking previous users, we should keep supporting previous flag
+ # even if we don't use them / advertise them anymore.
+ if options.interactive:
+ options.mode = CrashLogLoadingMode.interactive
+ elif options.batch:
+ options.mode = CrashLogLoadingMode.batch
+
+ if (
+ options.mode
+ and options.mode != CrashLogLoadingMode.interactive
+ and (options.target_path or options.skip_status)
+ ):
+ print(
+ "Target path (-t) and skipping process status (-s) options can only used in intercative mode (-m=interactive)."
+ )
+ print("Aborting symbolication.")
+ arg_parser.print_help()
+ return
+
+ if options.version:
+ print(debugger.GetVersionString())
+ return
+
+ if options.debug:
+ print("command_args = %s" % command_args)
+ print("options", options)
+ print("args", options.reports)
+
# Interactive mode requires running the crashlog command from inside lldb.
- if options.interactive and not is_command:
+ if options.mode == CrashLogLoadingMode.interactive and not is_command:
lldb_exec = (
subprocess.check_output(["/usr/bin/xcrun", "-f", "lldb"])
.decode("utf-8")
@@ -1814,14 +1851,7 @@ def SymbolicateCrashLogs(debugger, command_args, result, is_command):
)
)
- if options.version:
- print(debugger.GetVersionString())
- return
- if options.debug:
- print("command_args = %s" % command_args)
- print("options", options)
- print("args", options.reports)
if options.debug_delay > 0:
print("Waiting %u seconds for debugger to attach..." % options.debug_delay)
@@ -1829,20 +1859,18 @@ def SymbolicateCrashLogs(debugger, command_args, result, is_command):
error = lldb.SBError()
def should_run_in_interactive_mode(options, ci):
- if options.interactive:
+ if options.mode:
+ return options.mode == CrashLogLoadingMode.interactive
+ elif ci and ci.IsInteractive():
return True
- elif options.batch:
- return False
- # elif ci and ci.IsInteractive():
- # return True
else:
- return False
+ return sys.stdout.isatty()
ci = debugger.GetCommandInterpreter()
if options.reports:
for crashlog_file in options.reports:
- crashlog_path = os.path.expanduser(crashlog_file)
+ crashlog_path = os.path.normpath(os.path.expanduser(crashlog_file))
if not os.path.exists(crashlog_path):
raise FileNotFoundError(
"crashlog file %s does not exist" % crashlog_path
diff --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test
index 5a946a38b1952..d925324822de7 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test
@@ -1,7 +1,7 @@
# RUN: %clang_host -g %S/Inputs/test.c -o %t.out
# RUN: cp %S/Inputs/altered_threadState.crash %t.crash
# RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}'
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -b %t.crash' 2>&1 | FileCheck %s
# CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
diff --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
index c2e23e82be7f5..d5c6d915316e8 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
@@ -2,12 +2,12 @@
# RUN: cp %S/Inputs/a.out.ips %t.crash
# RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -c %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog --mode batch %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog --mode batch -c %t.crash' 2>&1 | FileCheck %s
# RUN: cp %S/Inputs/a.out.ips %t.nometadata.crash
# RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.nometadata.crash --offsets '{"main":20, "bar":9, "foo":16}' --json --no-metadata
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.nometadata.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog --mode batch %t.nometadata.crash' 2>&1 | FileCheck %s
# CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
diff --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
index 5b5cef40716ca..2e4b46c8c2409 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
@@ -2,7 +2,7 @@
# RUN: cp %S/Inputs/no_threadState.ips %t.crash
# RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog --mode batch %t.crash' 2>&1 | FileCheck %s
# CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
diff --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
index 81e06868eaee7..98c933b793336 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
@@ -3,7 +3,7 @@
# RUN: mkdir -p %t.dir
# RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > %t.dir/multithread-test
# RUN: %lldb -b -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -s -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
+# RUN: -o 'crashlog -a -s --mode interactive -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
# RUN: -o 'command source -s 0 %s' 2>&1 | FileCheck %s
# CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
diff --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
index e9d1c5e98fb32..eec30a1da64c6 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
@@ -1,7 +1,7 @@
# RUN: %clang_host -g %S/Inputs/test.c -o %t.out
# RUN: cp %S/Inputs/a.out.crash %t.crash
# RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}'
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -b %t.crash' 2>&1 | FileCheck %s
# CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
|
|
FWIW, this was already approved in https://reviews.llvm.org/D141658 but I've updated it slightly to match to current version of the crashlog script. |
bulbazord
reviewed
Jun 6, 2024
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
Outdated
Show resolved
Hide resolved
medismailben
force-pushed
the
crashlog-default-interactive-mode
branch
from
20f6d74 to
f50656c
Compare
|
Closed this by mistake. |
medismailben
force-pushed
the
crashlog-default-interactive-mode
branch
from
bc3292a to
f00c339
Compare
bulbazord
reviewed
Jun 10, 2024
medismailben
force-pushed
the
crashlog-default-interactive-mode
branch
from
f00c339 to
fd69e8f
Compare
bulbazord
reviewed
Jun 10, 2024
lldb/examples/python/crashlog.py
Outdated
| if options.debug_delay > 0: | ||
| print("Waiting %u seconds for debugger to attach..." % options.debug_delay) | ||
| time.sleep(options.debug_delay) | ||
| error = lldb.SBError() | ||
|
|
||
| def should_run_in_interactive_mode(options, ci): | ||
| if options.interactive: | ||
| if options.mode == CrashLogLoadingMode.interactive: |
There was a problem hiding this comment.
I guess I'm somewhat confused by the purpose of this function... The only way options.mode is ever not interactive is if it's explicitly set by the user to be something else right? Why might we want to ignore that?
bulbazord
approved these changes
Jun 10, 2024
This patch makes interactive mode as the default when using the crashlog command. It replaces the existing `-i|--interactive` flag with a new `-m|--mode` option, that can either be `interactive` or `batch`. By default, when the option is not explicitely set by the user, the interactive mode is selected, however, lldb will fallback to batch mode if the command interpreter is not interactive or if stdout is not a tty. This also adds some railguards to prevent users from using interactive only options with the batch mode and updates the tests accordingly. rdar://97801509 Differential Revision: https://reviews.llvm.org/D141658 Signed-off-by: Med Ismail Bennani <[email protected]>
medismailben
force-pushed
the
crashlog-default-interactive-mode
branch
from
fd69e8f to
3b239e6
Compare
medismailben
added a commit
that referenced
this pull request
Jun 21, 2024
This reverts commit aafa0ef.
medismailben
added a commit
that referenced
this pull request
Jun 21, 2024
Reverts #94575 since introduces test failure: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/6166/
AlexisPerry
pushed a commit
to llvm-project-tlp/llvm-project
that referenced
this pull request
Jul 9, 2024
This patch makes interactive mode as the default when using the crashlog command. It replaces the existing `-i|--interactive` flag with a new `-m|--mode` option, that can either be `interactive` or `batch`. By default, when the option is not explicitely set by the user, the interactive mode is selected, however, lldb will fallback to batch mode if the command interpreter is not interactive or if stdout is not a tty. This also adds some railguards to prevent users from using interactive only options with the batch mode and updates the tests accordingly. rdar://97801509 Differential Revision: https://reviews.llvm.org/D141658 Signed-off-by: Med Ismail Bennani <[email protected]>
AlexisPerry
pushed a commit
to llvm-project-tlp/llvm-project
that referenced
this pull request
Jul 9, 2024
…96263) Reverts llvm#94575 since introduces test failure: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/6166/
medismailben
added a commit
to medismailben/llvm-project
that referenced
this pull request
Dec 2, 2024
This patch makes interactive mode as the default when using the crashlog command. It replaces the existing `-i|--interactive` flag with a new `-m|--mode` option, that can either be `interactive` or `batch`. By default, when the option is not explicitely set by the user, the interactive mode is selected, however, lldb will fallback to batch mode if the command interpreter is not interactive or if stdout is not a tty. This also adds some railguards to prevent users from using interactive only options with the batch mode and updates the tests accordingly. rdar://97801509 Differential Revision: https://reviews.llvm.org/D141658 Signed-off-by: Med Ismail Bennani <[email protected]>
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.
This patch makes interactive mode as the default when using the crashlog command. It replaces the existing
-i|--interactiveflag with a new-m|--modeoption, that can either beinteractiveorbatch.By default, when the option is not explicitely set by the user, the interactive mode is selected, however, lldb will fallback to batch mode if the command interpreter is not interactive or if stdout is not a tty.
This also adds some railguards to prevent users from using interactive only options with the batch mode and updates the tests accordingly.
rdar://97801509
Differential Revision: https://reviews.llvm.org/D141658