[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 <[email protected]>

Author: medismailben

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,47 @@ def create_target(self):
     def get_target(self):
         return self.target
 
+    def load_images(self, options, 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 +1449,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 +1510,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)

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):