[lldb/crashlog] Surface error using SBCommandReturnObject argument

[mib]

Summary

This patch allows the crashlog script to surface its errors to lldb by
using the provided SBCommandReturnObject argument.

rdar://95048193

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

[JDevlieghere]

lldb/examples/python/crashlog.py

1012–1015

Can we do this with a try-catch + an exception?

[kastiglione]

lldb/examples/python/crashlog.py

1013–1015

Is this error function needed? SetError prepends the string "error: ".

[mib]

lldb/examples/python/crashlog.py

1012–1015

@JDevlieghere As mentioned on D129611, if we raise an exception we won't be able to surface the error to lldb or even to IDEs. I think it's better to use the SBCommandReturnObject

@kastiglione true! I'll remove the "error: " prefix, but I think it's reasonable to keep this helper function to avoid code duplication.

[JDevlieghere]

lldb/examples/python/crashlog.py

1012–1015

Couldn't you still raise the error where it happens and then catch it and then take it's value and put it into the SBCommandReturnObject?

[mib]

Address @JDevlieghere comments:

• Use exception approach / Remove error helper function
• Add test

[kastiglione]

lgtm

lldb/examples/python/crashlog.py

1253–1255

minor but I think this can be a single statement: result.SetError(str(e))

[JDevlieghere]

lldb/examples/python/crashlog.py

1013–1015

Rather than raising a generic Exception, we should define our own. Now there's no way to differentiate between these issues and any other exception that was thrown. Maybe that's the goal, in which case you can still catch the generic Exception, but by defining our own we leave the option open to be more specific.

[mib]

• Add custom InteractiveCrashLogException
• Simplify the except block

[JDevlieghere]

LGTM

lldb/examples/python/crashlog.py

else:
         result.PutCString("error: invalid target")
 
 
 class Symbolicate:
     def __init__(self, debugger, internal_dict):
         pass
 
     def __call__(self, debugger, command, exe_ctx, result):
-        SymbolicateCrashLogs(debugger, shlex.split(command))
+        SymbolicateCrashLogs(debugger, shlex.split(command), result)
 
     def get_short_help(self):
         return "Symbolicate one or more darwin crash log files."
 
     def get_long_help(self):
         option_parser = CrashLogOptionParser()
         return option_parser.format_help()
 
for thread in crash_log.threads:
         thread.dump_symbolicated(crash_log, options)
         print()
 
     if crash_log.errors:
         print("Errors:")
         for error in crash_log.errors:
             print(error)
 
-def load_crashlog_in_scripted_process(debugger, crash_log_file, options):
-    result = lldb.SBCommandReturnObject()
+def load_crashlog_in_scripted_process(debugger, crash_log_file, options, result):
+    def error(message, sb_error=lldb.SBError()):
+        nonlocal result
+        result.SetError(sb_error, "error: %s" % message)
+        return
 
     crashlog_path = os.path.expanduser(crash_log_file)
     if not os.path.exists(crashlog_path):
-        result.PutCString("error: crashlog file %s does not exist" % crashlog_path)
+        return error("crashlog file %s does not exist" % crashlog_path)
 
     crashlog = CrashLogParser().parse(debugger, crashlog_path, False)
 
     target = lldb.SBTarget()
     # 1. Try to use the user-provided target
     if options.target_path is not None and options.target_path != "":
         target = debugger.CreateTarget(options.target_path)
     # 2. If that didn't work, try to create a target using the symbolicator
     if target is None or not target.IsValid():
         target = crashlog.create_target()
     # 3. If that didn't work, and a target is already loaded, use it
     if (target is None  or not target.IsValid()) and debugger.GetNumTargets() > 0:
         target = debugger.GetTargetAtIndex(0)
     # 4. Fail
     if target is None or not target.IsValid():
-        result.PutCString("error: couldn't create target")
-        return
+        return error("couldn't create target")
 
     ci = debugger.GetCommandInterpreter()
     if not ci:
-        result.PutCString("error: couldn't get command interpreter")
-        return
+        return error("couldn't get command interpreter")
 
-    res = lldb.SBCommandReturnObject()
-    ci.HandleCommand('script from lldb.macosx import crashlog_scripted_process', res)
-    if not res.Succeeded():
-        result.PutCString("error: couldn't import crashlog scripted process module")
-        return
+    ci.HandleCommand('script from lldb.macosx import crashlog_scripted_process', result)
+    if not result.Succeeded():
+        return error("couldn't import crashlog scripted process module")
 
     structured_data = lldb.SBStructuredData()
     structured_data.SetFromJSON(json.dumps({ "crashlog_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)
+    sb_error = lldb.SBError()
+    process = target.Launch(launch_info, sb_error)
 
-    if not process or error.Fail():
-        return
+    if not process or sb_error.Fail():
+        return error("couldn't launch Scripted Process", sb_error)
 
     @contextlib.contextmanager
     def synchronous(debugger):
         async_state = debugger.GetAsync()
         debugger.SetAsync(False)
         try:
             yield
         finally:

 for the first frame of the crashed thread.
 If this script is imported into the LLDB command interpreter, a "crashlog" command will be added to the interpreter
 for use at the LLDB command line. After a crash log has been parsed and symbolicated, a target will have been
 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)
 
-def SymbolicateCrashLogs(debugger, command_args):
+def SymbolicateCrashLogs(debugger, command_args, result):
     option_parser = CrashLogOptionParser()
 
     if not len(command_args):
         option_parser.print_help()
         return
 
     try:
         (options, args) = option_parser.parse_args(command_args)
def should_run_in_interactive_mode(options, ci):
             return False
 
     ci = debugger.GetCommandInterpreter()
 
     if args:
         for crash_log_file in args:
             if should_run_in_interactive_mode(options, ci):
                 load_crashlog_in_scripted_process(debugger, crash_log_file,
-                                                  options)
+                                                  options, result)
             else:
                 crash_log = CrashLogParser().parse(debugger, crash_log_file, options.verbose)
-                SymbolicateCrashLog(crash_log, options)
+                SymbolicateCrashLog(crash_log, options, result)
 
 if __name__ == '__main__':
     # Create a new debugger instance
     debugger = lldb.SBDebugger.Create()
-    SymbolicateCrashLogs(debugger, sys.argv[1:])
+    result = lldb.SBCommandReturnObject()
+    SymbolicateCrashLogs(debugger, sys.argv[1:], result)
     lldb.SBDebugger.Destroy(debugger)
 
 def __lldb_init_module(debugger, internal_dict):
     debugger.HandleCommand(
         'command script add -c lldb.macosx.crashlog.Symbolicate crashlog')
     debugger.HandleCommand(
         'command script add -f lldb.macosx.crashlog.save_crashlog save_crashlog')
     print('"crashlog" and "save_crashlog" commands have been installed, use '
           'the "--help" options on these commands for detailed help.')