[lldb/crashlog] Enforce image loading policy

[medismailben]

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 ismail@bennani.ma

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)

Changes

Starting 27f27d1, we changed the image loading logic to conform to the various options (-a|--load-all & -c|--crashed-only) and set the resolve attribute of the matching images.

Then, we would try to load all those images concurrently. However, the symbolicator.Image.add_module method didn't check the resolve attribute to skip unwanted images, causing all images to be loaded in the target everytime. This matches the -a|--load-all option behaviour but 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 checking the Image.resolve attribute to conform to the user selection, and reinstates the expected default behaviour, by only loading the crashed thread images and skipping all the others.

rdar://123694062

---

Full diff: https://github.com/llvm/llvm-project/pull/91109.diff

1 Files Affected:

• (modified) lldb/examples/python/symbolication.py (+6)

diff --git a/lldb/examples/python/symbolication.py b/lldb/examples/python/symbolication.py
index f6dcc8b9a79437..f247ebf3da733a 100755
--- a/lldb/examples/python/symbolication.py
+++ b/lldb/examples/python/symbolication.py
@@ -399,6 +399,12 @@ def add_module(self, target, obj_dir=None):
         if not self.path and self.uuid == uuid.UUID(int=0):
             return "error: invalid image"
 
+        if not self.resolve:
+            # Since this method get called concurrently on every crashlog image,
+            # we should only add the images marked to be resolved and skip the
+            # others.
+            return None
+
         if target:
             # Try and find using UUID only first so that paths need not match
             # up

[bulbazord]

Suggestion: Default loaded_images to None. You probably don't want loaded_images to default to []. It will persist between calls, e.g.:

>>> def foo(x, lst=[]):
...     lst.append(x)
...     print(lst)
...
>>> foo(1)
[1]
>>> foo(2)
[1, 2]

[medismailben]

Good point