[lldb/crashlog] Make interactive mode the new default

[mib]

Summary

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

Signed-off-by: Med Ismail Bennani <[e-mail]>

[JDevlieghere]

Switching the default over seems fine. The symbol lookup stuff seems orthogonal to that and maybe should be its own patch?

lldb/examples/python/crashlog.py

452–457

It's not obvious why this is necessary (and we can't rely on open/the OS to do this for us) so let's add a comment.

1111–1113

Background lookup is lazy: it's only going to start looking for symbols after we've determined they might be of interest. Right now that means after seeing an imagine in the backtrace. Without anything else, the first backtrace is still going be unsymbolicated (unless we had the symbols already available on the host).

It seems like you're doing the symbol loading explicitly in the C++ implementation of the scripted process, so what's depending on this?

[mib]

Address @JDevlieghere comments:

• Move module loading logic to separate patch D141702
• Remove use of background symbol lookup

[JDevlieghere]

Another nice addition here would be checking if stdout is a tty (sys.stdout.isatty()) and continuing to use the textual format if it isn't. A quick experiment seems to show this does the right thing from inside LLDB:

$ echo -e "script\nimport sys\nsys.stdout.isatty()" | lldb -b
(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> >>> True
now exiting InteractiveConsole...
$ echo -e "script\nimport sys\nsys.stdout.isatty()" | lldb -b | tail
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.

now exiting InteractiveConsole...
(lldb) script
>>> >>> False

lldb/examples/python/crashlog.py

1306

Spurious print? It doesn't look like we're printing a double newline elsewhere.

[mib]

lldb/examples/python/crashlog.py

452–457

From my experience, open

1306

I wanted to leave space between the actual error message and the help

[mib]

Address @JDevlieghere comment:

• Check is stdout is a tty to run in interactive mode
• Remove extra return line

[JDevlieghere]

LGTM. You can remove one of the -b in the test to check the TTY stuff (it should be false when piping into FileCheck).

lldb/examples/python/crashlog.py

1306

I like this

[mib]

Fix test failures:

• Change -b|--batch flag to -m|--mode options that expects an enum value.

[mib]

lldb/examples/python/crashlog.py

452–457

I started replying to your message but I ended up adding a comment of the file directly.

[mib]

I changed the implementation to be able to fix the tests. Could you guys please take another look ?

[bulbazord]

I like this. I have no opinion on having -i/-b versus --mode/-m to indicate which mode crashlog.py should use, though I think that using an enum makes it easier to add new modes down the line so I'm okay with this implementation (note: I can't think of any other kinds of modes I would add later).

LGTM, @JDevlieghere any objections?

lldb/examples/python/crashlog.py

47

nit: no need to remove this line.

[JDevlieghere]

lldb/examples/python/crashlog.py

1148

This can be interpreted as the textual crashlogs going away which isn't the case or the goal. I think this should say "textual" (although that might be confusing with the JSON/text input) or maybe "batch".

[mib]

Address @JDevlieghere feedback: rename legacy loading mode into batch.

lldb/examples/python/crashlog.py

 # On MacOSX sh, bash:
 #   PYTHONPATH=/path/to/LLDB.framework/Resources/Python ./crashlog.py ~/Library/Logs/DiagnosticReports/a.crash
 #----------------------------------------------------------------------
 
 import abc
 import concurrent.futures
 import contextlib
 import datetime
+import enum
 import json
 import optparse
 import os
 import platform
 import plistlib
 import re
 import shlex
 import string
 import subprocess
 import sys
 import threading
 import time
 import uuid
 
-
 print_lock = threading.RLock()
 
 try:
     # First try for LLDB in case PYTHONPATH is already correctly setup.
     import lldb
 except ImportError:
     # Ask the command line driver for the path to the lldb module. Copy over
     # the environment so that SDKROOT is propagated to xcrun.
def is_valid_json(path):
             try:
                 return json.loads(buffer)
             except:
                 # The first line can contain meta data. Try stripping it and
                 # try again.
                 head, _, tail = buffer.partition('\n')
                 return json.loads(tail)
 
-        with open(path, 'r', encoding='utf-8') as f:
+        # Python's `open` function doesn't recognize user paths ('~/'), and
+        # expects either an absolute path or a relative path to the current
+        # working directory, so we need to expand the file path  ourself.
+        # The reasoning behind this is that on non-Unix systems, `~foo` is a
+        # valid filename, that wouldn't require file expansion.
+        with open(os.path.normpath(os.path.expanduser(path)), 'r', encoding='utf-8') as f:
             buffer = f.read()
         try:
             return parse_json(buffer)
         except:
             return None
 
     def parse(self):
         try:
def load_crashlog_in_scripted_process(debugger, crash_log_file, options, result):
     ci = debugger.GetCommandInterpreter()
     if not ci:
         raise InteractiveCrashLogException("couldn't get command interpreter")
 
     ci.HandleCommand('script from lldb.macosx import crashlog_scripted_process', result)
     if not result.Succeeded():
         raise InteractiveCrashLogException("couldn't import crashlog scripted process module")
 
     structured_data = lldb.SBStructuredData()
     structured_data.SetFromJSON(json.dumps({ "file_path" : crashlog_path,
                                              "load_all_images": options.load_all_images }))
     launch_info = lldb.SBLaunchInfo(None)
     launch_info.SetProcessPluginName("ScriptedProcess")
     launch_info.SetScriptedProcessClassName("crashlog_scripted_process.CrashLogScriptedProcess")
     launch_info.SetScriptedProcessDictionary(structured_data)
     error = lldb.SBError()
     process = target.Launch(launch_info, error)
 
     if not process or error.Fail():
if not options.skip_status:
 
             commands_stream = lldb.SBStream()
             commands_stream.Print("process status --verbose\n")
             commands_stream.Print("thread backtrace --extended true\n")
             error = debugger.SetInputString(commands_stream.GetData())
             if error.Success():
                 debugger.RunCommandInterpreter(True, False, run_options, 0, False, True)
 
+class CrashLogLoadingMode(str, enum.Enum):
+    batch = 'batch'
+    interactive = 'interactive'
+
 def CreateSymbolicateCrashLogOptions(
         command_name,
         description,
         add_interactive_options):
     usage = "usage: %prog [options] <FILE> [FILE ...]"
     option_parser = optparse.OptionParser(
         description=description, prog='crashlog', usage=usage)
     option_parser.add_option(
option_parser.add_option(
         help='show source for NFRAMES (default = 4)',
         default=4)
     option_parser.add_option(
         '--source-all',
         action='store_true',
         dest='source_all',
         help='show source for all threads, not just the crashed thread',
         default=False)
-    if add_interactive_options:
-        option_parser.add_option(
-            '-i',
-            '--interactive',
-            action='store_true',
-            help='parse a crash log and load it in a ScriptedProcess',
-            default=False)
-        option_parser.add_option(
-            '-b',
-            '--batch',
-            action='store_true',
-            help='dump symbolicated stackframes without creating a debug session',
-            default=True)
+        '-m',
+        '--mode',
+        type="choice",
+        choices=[mode.value for mode in CrashLogLoadingMode],
+        help="change how the symbolicated process and threads are displayed to the user (default = 'interactive')")
+    if add_interactive_options:
         option_parser.add_option(
             '--target',
             '-t',
             dest='target_path',
             help='the target binary path that should be used for interactive crashlog (optional)',
             default=None)
         option_parser.add_option(
             '--skip-status',
if not len(command_args):
         option_parser.print_help()
         return
 
     try:
         (options, args) = option_parser.parse_args(command_args)
     except:
         return
 
+    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.")
+        option_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', args)
 
     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:
+            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 args:
         for crash_log_file in args:
             if should_run_in_interactive_mode(options, ci):
                 try:
                     load_crashlog_in_scripted_process(debugger, crash_log_file,

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test

 # REQUIRES: python, native && target-aarch64 && system-darwin
 
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/application_specific_info/asi.yaml > %t.dir/asi
 # RUN: %lldb -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -t %t.dir/asi %S/Inputs/application_specific_info/asi.txt' \
+# RUN: -o 'crashlog -a --mode interactive -t %t.dir/asi %S/Inputs/application_specific_info/asi.txt' \
 # RUN: -o "thread list" -o "bt all" 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
 # CHECK: (lldb) process status --verbose
 # CHECK-NEXT: Process 96535 stopped
 # CHECK-NEXT: * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_CRASH (code=0, subcode=0x0)
 # CHECK-NEXT:     frame #0: 0x00000001a08c7224{{.*}}[artificial]

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test

 # REQUIRES: python, native && target-aarch64 && system-darwin
 
 # RUN: %lldb -o 'command script import lldb.macosx.crashlog' \
 # RUN: -o 'crashlog -V' \
-# RUN: -o 'crashlog -a -i -t /this_file_does_not_exist %S/Inputs/interactive_crashlog/multithread-test.ips' 2>&1 | FileCheck %s
+# RUN: -o 'crashlog -a --mode interactive -t /this_file_does_not_exist %S/Inputs/interactive_crashlog/multithread-test.ips' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
 # CHECK: lldb{{.*}}version
 
 # CHECK: error: couldn't create target provided by the user (/this_file_does_not_exist)

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test

 # RUN: %clang_host -g %S/Inputs/test.c -o %t.out
 
 # 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 --mode batch %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
 
 # CHECK: Thread[0] Crashing Thread Name EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x0000000000000000)
 # CHECK: [  0] {{.*}}out`foo + 16 at test.c
 # CHECK: [  1] {{.*}}out`bar + 8 at test.c
 # CHECK: [  2] {{.*}}out`main + 19 at test.c
 # CHECK: rbp = 0x00007ffeec22a530
 # CHECK: tmp2 = 0x0000000000000008
 # CHECK: invalid foo
 # CHECK: invalid bar

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test

 # RUN: %clang_host -g %S/Inputs/test.c -o %t.out
 
 # 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
 
 # CHECK: Thread[0] Crashing Thread Name EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x0000000000000000)
 # CHECK: [  0] {{.*}}out`foo + 16 at test.c
 # CHECK: [  1] {{.*}}out`bar + 8 at test.c
 # CHECK: [  2] {{.*}}out`main + 19 at test.c
 # CHECK: No thread state (register information) available

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test

 # REQUIRES: python, native && target-aarch64 && system-darwin
 
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > %t.dir/multithread-test
 # RUN: %lldb -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
+# RUN: -o 'crashlog -a --mode interactive -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
 # RUN: -o "thread list" -o "bt all" 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
 # CHECK: (lldb) process status
 # CHECK-NEXT: Process 22511 stopped
 # CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
 # CHECK-NEXT:     frame #0: 0x0000000100ec58f4 multithread-test`bar

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test

 # REQUIRES: python, native && target-aarch64 && system-darwin
 
 # 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
 
 process status
 # CHECK: Process 22511 stopped
 # CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
 # CHECK-NEXT:     frame #0: 0x0000000100ec58f4 multithread-test`bar

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test

 # 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 --mode batch %t.crash' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
 # CHECK: Thread[0] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x0000000000000000)
 # CHECK: [  0] {{.*}}out`foo + 16 at test.c
 # CHECK: [  1] {{.*}}out`bar + 8 at test.c
 # CHECK: [  2] {{.*}}out`main + 19 at test.c
 # CHECK: [  3] 0x{{[0]+}}1000000 start + 1
 # CHECK: rbp = 0x00007ffee42d8020