[lldb/crashlog] Make interactive mode the new default (#144839)

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 <ismail@bennani.ma>

lldb/examples/python/crashlog.py

@@ -31,6 +31,7 @@ import argparse
 import concurrent.futures
 import contextlib
 import datetime
+import enum
 import json
 import os
 import platform
@@ -45,7 +46,6 @@ import threading
 import time
 import uuid
 
-
 print_lock = threading.RLock()
 
 try:
@@ -1582,9 +1582,12 @@ def load_crashlog_in_scripted_process(debugger, crashlog_path, options, result):
                 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,12 @@ 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: interactive)",
+    )
     arg_parser.add_argument(
         "--version",
         "-V",
@@ -1736,36 +1745,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 +1786,7 @@ for use at the LLDB command line. After a crash log has been parsed and symbolic
 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 +1802,35 @@ def SymbolicateCrashLogs(debugger, command_args, result, is_command):
         result.SetError(str(e))
         return
 
+    # To avoid breaking existing users, we should keep supporting legacy flags
+    # even if we don't use them / advertise them anymore.
+    if not options.mode:
+        if options.batch:
+            options.mode = CrashLogLoadingMode.batch
+        else:
+            options.mode = CrashLogLoadingMode.interactive
+
+    if 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 interactive 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")
@@ -1821,31 +1856,26 @@ def SymbolicateCrashLogs(debugger, command_args, result, is_command):
         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)
         time.sleep(options.debug_delay)
     error = lldb.SBError()
 
     def should_run_in_interactive_mode(options, ci):
-        if options.interactive:
+        if options.mode == CrashLogLoadingMode.batch:
+            return False
+        elif options.mode == CrashLogLoadingMode.interactive or (
+            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