https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/91109
>From 625245737c31ac4e2cf04417bc8457caa461ef31 Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <ism...@bennani.ma> Date: Thu, 9 May 2024 10:02:50 -0700 Subject: [PATCH] [lldb/crashlog] Enforce image loading policy 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 <ism...@bennani.ma> --- 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 c992348b24be1..d147d0e322a33 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: @@ -526,6 +526,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 @@ -1408,36 +1451,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: @@ -1498,7 +1512,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): _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits