From a3c98bb82ec839e7db2ed5cff96c48468cf6fabf Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani Date: Thu, 26 Oct 2023 21:21:54 -0700 Subject: [PATCH 1/5] [lldb/test] Fix failures following ec456ba9ca0a This patch fixes the various crashlog test failures following ec456ba9ca0a, which renamed the process member variable in the Scripted Thread python base class. This patch updates the crashlog scripted process implementation to reflect that change. Signed-off-by: Med Ismail Bennani (cherry picked from commit 4ec9cda656cc2fde41d4a4415ae363d9a3290c80) --- lldb/examples/python/crashlog_scripted_process.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lldb/examples/python/crashlog_scripted_process.py b/lldb/examples/python/crashlog_scripted_process.py index 43f767de138cd..c69985b1a072d 100644 --- a/lldb/examples/python/crashlog_scripted_process.py +++ b/lldb/examples/python/crashlog_scripted_process.py @@ -159,14 +159,14 @@ def resolve_stackframes(thread, addr_mask, target): return frames def create_stackframes(self): - if not (self.scripted_process.load_all_images or self.has_crashed): + if not (self.originating_process.load_all_images or self.has_crashed): return None if not self.backing_thread or not len(self.backing_thread.frames): return None self.frames = CrashLogScriptedThread.resolve_stackframes( - self.backing_thread, self.scripted_process.addr_mask, self.target + self.backing_thread, self.originating_process.addr_mask, self.target ) return self.frames @@ -182,7 +182,7 @@ def __init__(self, process, args, crashlog_thread): else: self.name = self.backing_thread.name self.queue = self.backing_thread.queue - self.has_crashed = self.scripted_process.crashed_thread_idx == self.idx + self.has_crashed = self.originating_process.crashed_thread_idx == self.idx self.create_stackframes() def get_state(self): @@ -195,8 +195,8 @@ def get_stop_reason(self) -> Dict[str, Any]: return {"type": lldb.eStopReasonNone} # TODO: Investigate what stop reason should be reported when crashed stop_reason = {"type": lldb.eStopReasonException, "data": {}} - if self.scripted_process.exception: - stop_reason["data"]["mach_exception"] = self.scripted_process.exception + if self.originating_process.exception: + stop_reason["data"]["mach_exception"] = self.originating_process.exception return stop_reason def get_register_context(self) -> str: @@ -209,5 +209,5 @@ def get_register_context(self) -> str: def get_extended_info(self): if self.has_crashed: - self.extended_info = self.scripted_process.extended_thread_info + self.extended_info = self.originating_process.extended_thread_info return self.extended_info From 84491ad9125646def0d5f9ac9d8066647693b419 Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani Date: Thu, 9 May 2024 10:13:20 -0700 Subject: [PATCH 2/5] [lldb/crashlog] Enforce image loading policy (#91109) In `27f27d1`, we changed the image loading logic to conform to the various options (`-a|--load-all` & `-c|--crashed-only`) and loaded them concurrently. However, instead of the subset of images that matched the user option, the thread pool would always run on all the crashlog images, causing them to be all loaded in the target everytime. This matches the `-a|--load-all` option behaviour but depending on the report, it can cause lldb to load thousands of images, which can take a very long time if the images are downloaded over the network. This patch fixes that issue by keeping a list of `images_to_load` based of the user-provided option. This list will be used with our executor thread pool to load the images according to the user selection, and reinstates the expected default behaviour, by only loading the crashed thread images and skipping all the others. This patch also unifies the way we load images into a single method that's shared by both the batch mode & the interactive scripted process. rdar://123694062 Signed-off-by: Med Ismail Bennani (cherry picked from commit 785b143a402a282822c3d5e30bb4e2b1980c0b1e) --- lldb/examples/python/crashlog.py | 82 +++++++++++-------- .../python/crashlog_scripted_process.py | 38 +++------ 2 files changed, 63 insertions(+), 57 deletions(-) diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py index 923d7ec2e9373..ecf5c3664419d 100755 --- a/lldb/examples/python/crashlog.py +++ b/lldb/examples/python/crashlog.py @@ -252,7 +252,7 @@ def add_ident(self, ident): self.idents.append(ident) def did_crash(self): - return self.reason is not None + return self.crashed def __str__(self): if self.app_specific_backtrace: @@ -522,6 +522,49 @@ def create_target(self): def get_target(self): return self.target + def load_images(self, options, loaded_images=None): + if not loaded_images: + loaded_images = [] + images_to_load = self.images + if options.load_all_images: + for image in self.images: + image.resolve = True + elif options.crashed_only: + for thread in self.threads: + if thread.did_crash(): + images_to_load = [] + for ident in thread.idents: + for image in self.find_images_with_identifier(ident): + image.resolve = True + images_to_load.append(image) + + futures = [] + with tempfile.TemporaryDirectory() as obj_dir: + with concurrent.futures.ThreadPoolExecutor() as executor: + + def add_module(image, target, obj_dir): + return image, image.add_module(target, obj_dir) + + for image in images_to_load: + if image not in loaded_images: + if image.uuid == uuid.UUID(int=0): + continue + futures.append( + executor.submit( + add_module, + image=image, + target=self.target, + obj_dir=obj_dir, + ) + ) + + for future in concurrent.futures.as_completed(futures): + image, err = future.result() + if err: + print(err) + else: + loaded_images.append(image) + class CrashLogFormatException(Exception): pass @@ -1404,36 +1447,7 @@ def SymbolicateCrashLog(crash_log, options): if not target: return - if options.load_all_images: - for image in crash_log.images: - image.resolve = True - elif options.crashed_only: - for thread in crash_log.threads: - if thread.did_crash(): - for ident in thread.idents: - for image in crash_log.find_images_with_identifier(ident): - image.resolve = True - - futures = [] - loaded_images = [] - with tempfile.TemporaryDirectory() as obj_dir: - with concurrent.futures.ThreadPoolExecutor() as executor: - - def add_module(image, target, obj_dir): - return image, image.add_module(target, obj_dir) - - for image in crash_log.images: - futures.append( - executor.submit( - add_module, image=image, target=target, obj_dir=obj_dir - ) - ) - for future in concurrent.futures.as_completed(futures): - image, err = future.result() - if err: - print(err) - else: - loaded_images.append(image) + crash_log.load_images(options) if crash_log.backtraces: for thread in crash_log.backtraces: @@ -1494,7 +1508,11 @@ def load_crashlog_in_scripted_process(debugger, crashlog_path, options, result): structured_data = lldb.SBStructuredData() structured_data.SetFromJSON( json.dumps( - {"file_path": crashlog_path, "load_all_images": options.load_all_images} + { + "file_path": crashlog_path, + "load_all_images": options.load_all_images, + "crashed_only": options.crashed_only, + } ) ) launch_info = lldb.SBLaunchInfo(None) diff --git a/lldb/examples/python/crashlog_scripted_process.py b/lldb/examples/python/crashlog_scripted_process.py index c69985b1a072d..26c5c37b7371d 100644 --- a/lldb/examples/python/crashlog_scripted_process.py +++ b/lldb/examples/python/crashlog_scripted_process.py @@ -29,27 +29,7 @@ def set_crashlog(self, crashlog): if hasattr(self.crashlog, "asb"): self.extended_thread_info = self.crashlog.asb - if self.load_all_images: - for image in self.crashlog.images: - image.resolve = True - else: - for thread in self.crashlog.threads: - if thread.did_crash(): - for ident in thread.idents: - for image in self.crashlog.find_images_with_identifier(ident): - image.resolve = True - - with tempfile.TemporaryDirectory() as obj_dir: - for image in self.crashlog.images: - if image not in self.loaded_images: - if image.uuid == uuid.UUID(int=0): - continue - err = image.add_module(self.target, obj_dir) - if err: - # Append to SBCommandReturnObject - print(err) - else: - self.loaded_images.append(image) + crashlog.load_images(self.options, self.loaded_images) for thread in self.crashlog.threads: if ( @@ -70,6 +50,10 @@ def set_crashlog(self, crashlog): self.app_specific_thread, self.addr_mask, self.target ) + class CrashLogOptions: + load_all_images = False + crashed_only = True + def __init__(self, exe_ctx: lldb.SBExecutionContext, args: lldb.SBStructuredData): super().__init__(exe_ctx, args) @@ -88,13 +72,17 @@ def __init__(self, exe_ctx: lldb.SBExecutionContext, args: lldb.SBStructuredData # Return error return + self.options = self.CrashLogOptions() + load_all_images = args.GetValueForKey("load_all_images") if load_all_images and load_all_images.IsValid(): if load_all_images.GetType() == lldb.eStructuredDataTypeBoolean: - self.load_all_images = load_all_images.GetBooleanValue() + self.options.load_all_images = load_all_images.GetBooleanValue() - if not self.load_all_images: - self.load_all_images = False + crashed_only = args.GetValueForKey("crashed_only") + if crashed_only and crashed_only.IsValid(): + if crashed_only.GetType() == lldb.eStructuredDataTypeBoolean: + self.options.crashed_only = crashed_only.GetBooleanValue() self.pid = super().get_process_id() self.crashed_thread_idx = 0 @@ -159,7 +147,7 @@ def resolve_stackframes(thread, addr_mask, target): return frames def create_stackframes(self): - if not (self.originating_process.load_all_images or self.has_crashed): + if not (self.originating_process.options.load_all_images or self.has_crashed): return None if not self.backing_thread or not len(self.backing_thread.frames): From 3076936eb3d1820de9a3ea8d8a8120cd5c4920ef Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani Date: Thu, 9 May 2024 10:19:34 -0700 Subject: [PATCH 3/5] [lldb/crashlog] Update incorrect help message for `--no-crashed-only` option (#91162) This patch rephrases the crashlog `--no-crashed-only` option help message. This option is mainly used in batch mode to symbolicate and dump all the threads backtraces, instead of only doing it for the crashed thread which is the default behavior. rdar://127391524 Signed-off-by: Med Ismail Bennani (cherry picked from commit 8585bf7542f1098bd03a667a408d42d2a815d305) --- lldb/examples/python/crashlog.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py index ecf5c3664419d..706648b25c170 100755 --- a/lldb/examples/python/crashlog.py +++ b/lldb/examples/python/crashlog.py @@ -1645,7 +1645,8 @@ def CreateSymbolicateCrashLogOptions( "--no-crashed-only", action="store_false", dest="crashed_only", - help="do not symbolicate the crashed thread", + help="in batch mode, symbolicate all threads, not only the crashed one", + default=False, ) arg_parser.add_argument( "--disasm-depth", From e6fd01f175a24d79b0f4bb0fe128eca3d95a7e2b Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani Date: Thu, 9 May 2024 13:51:52 -0700 Subject: [PATCH 4/5] [lldb/crashlog] Fix module binary resolution (#91631) This patch fixes a bug in when resolving and loading modules from the binary image list. When loading a module, we would first use the UUID from the binary image list with `dsymForUUID` to fetch the dSYM bundle from our remote build records and copy the executable locally. If we failed to find a matching dSYM bundle for that UUID on the build record, let's say if that module was built locally, we use Spotlight (`mdfind`) to find the dSYM bundle once again using the UUID. Prior to this patch, we would set the image path to be the same as the symbol file. This resulted in trying to load the dSYM as a module in lldb, which isn't allowed. This patch address that by looking for a binary matching the image identifier, next to the dSYM bundle and try to load that instead. rdar://127433616 Signed-off-by: Med Ismail Bennani (cherry picked from commit f4a7e1f9bac1f11e1db1c0a895f3f681838f89f2) --- lldb/examples/python/crashlog.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py index 706648b25c170..134ed94991256 100755 --- a/lldb/examples/python/crashlog.py +++ b/lldb/examples/python/crashlog.py @@ -414,9 +414,20 @@ def locate_module_and_debug_symbols(self): with print_lock: print('falling back to binary inside "%s"' % dsym) self.symfile = dsym - for filename in os.listdir(dwarf_dir): - self.path = os.path.join(dwarf_dir, filename) - if self.find_matching_slice(): + # Look for the executable next to the dSYM bundle. + parent_dir = os.path.dirname(dsym) + executables = [] + for root, _, files in os.walk(parent_dir): + for file in files: + abs_path = os.path.join(root, file) + if os.path.isfile(abs_path) and os.access( + abs_path, os.X_OK + ): + executables.append(abs_path) + for binary in executables: + basename = os.path.basename(binary) + if basename == self.identifier: + self.path = binary found_matching_slice = True break if found_matching_slice: From e3ec0809136ad537bc81af4bdf60a03042981705 Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani Date: Thu, 9 May 2024 14:13:44 -0700 Subject: [PATCH 5/5] [lldb/crashlog] Fix test failure when creating a target using command options (#91653) This should fix the various crashlog test failures on the bots: ``` lldb-shell :: ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test lldb-shell :: ScriptInterpreter/Python/Crashlog/interactive_crashlog_json.test lldb-shell :: ScriptInterpreter/Python/Crashlog/interactive_crashlog_legacy.test lldb-shell :: ScriptInterpreter/Python/Crashlog/last_exception_backtrace_crashlog.test lldb-shell :: ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test ``` When we create a target by using the command option, we don't set it in the crashlog object which later on cause us to fail loading the images. rdar://127832961 Signed-off-by: Med Ismail Bennani (cherry picked from commit db9421381980cdf3d6914f8898a77d3237325019) --- lldb/examples/python/crashlog.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py index 134ed94991256..08f39d516d32f 100755 --- a/lldb/examples/python/crashlog.py +++ b/lldb/examples/python/crashlog.py @@ -1490,6 +1490,7 @@ def load_crashlog_in_scripted_process(debugger, crashlog_path, options, result): raise InteractiveCrashLogException( "couldn't create target provided by the user (%s)" % options.target_path ) + crashlog.target = target # 2. If the user didn't provide a target, try to create a target using the symbolicator if not target or not target.IsValid():