Skip to content

Commit

Permalink
[lldb/crashlog] Fix sticky image parsing logic
Browse files Browse the repository at this point in the history
Prior to this patch, when a user loaded multiple crash report in lldb,
they could get in a situation where all the targets would keep the same
architecture and executable path as the first one that we've created.

The reason behind this was that even if we created a new CrashLog
object, which is derived from a Symbolicator class that has a newly
constructoted image list as a default argument, because that default
argument is only created once when the function is defined, every CrashLog
object would share the same list.

That will cause use to append newly parsed  images to the same
Symbolicator image list accross multiple CrashLog objects.

To address this, this patch changes the default argument value for the
image parameter to `None` and only initialize it as an empty list when
no argument was passed.

This also removes the image list stored in each CrashLog parsers since
they shouldn't have any state and should be re-usable. So now, the only
source of truth is stored in the CrashLog object.

rdar://84984949

Differential Revision: https://reviews.llvm.org/D157044

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
  • Loading branch information
medismailben committed Aug 12, 2023
1 parent 4677041 commit 75bed96
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 9 deletions.
10 changes: 3 additions & 7 deletions lldb/examples/python/crashlog.py
Expand Up @@ -549,8 +549,6 @@ def create(debugger, path, options):
def __init__(self, debugger, path, options):
self.path = os.path.expanduser(path)
self.options = options
# List of DarwinImages sorted by their index.
self.images = list()
self.crashlog = CrashLog(debugger, self.path, self.options.verbose)

@abc.abstractmethod
Expand Down Expand Up @@ -645,7 +643,6 @@ def parse_images(self, json_images):
darwin_image.arch = json_image["arch"]
if path == self.crashlog.process_path:
self.crashlog.process_arch = darwin_image.arch
self.images.append(darwin_image)
self.crashlog.images.append(darwin_image)

def parse_main_image(self, json_data):
Expand All @@ -672,7 +669,7 @@ def parse_frames(self, thread, json_frames):
location = 0
if "symbolLocation" in json_frame and json_frame["symbolLocation"]:
location = int(json_frame["symbolLocation"])
image = self.images[image_id]
image = self.crashlog.images[image_id]
image.symbols[symbol] = {
"name": symbol,
"type": "code",
Expand Down Expand Up @@ -780,7 +777,7 @@ def parse_asi_backtrace(self, thread, bt):
if frame_offset:
description += " + " + frame_offset
frame_offset_value = int(frame_offset, 0)
for image in self.images:
for image in self.crashlog.images:
if image.identifier == frame_img_name:
image.symbols[frame_symbol] = {
"name": frame_symbol,
Expand Down Expand Up @@ -829,6 +826,7 @@ def parse_errors(self, json_data):
if "reportNotes" in json_data:
self.crashlog.errors = json_data["reportNotes"]


class TextCrashLogParser(CrashLogParser):
parent_process_regex = re.compile(r"^Parent Process:\s*(.*)\[(\d+)\]")
thread_state_regex = re.compile(r"^Thread \d+ crashed with")
Expand Down Expand Up @@ -888,7 +886,6 @@ def get(cls):
)
exception_extra_regex = re.compile(r"^Exception\s+.*:\s+(.*)")


class CrashLogParseMode:
NORMAL = 0
THREAD = 1
Expand Down Expand Up @@ -1209,7 +1206,6 @@ def parse_images(self, line):
"address": symbol["address"] - int(img_lo, 0),
}

self.images.append(image)
self.crashlog.images.append(image)
return True
else:
Expand Down
5 changes: 3 additions & 2 deletions lldb/examples/python/symbolication.py
Expand Up @@ -501,7 +501,7 @@ def create_target(self, debugger):


class Symbolicator:
def __init__(self, debugger=None, target=None, images=list()):
def __init__(self, debugger=None, target=None, images=None):
"""A class the represents the information needed to symbolicate
addresses in a program.
Expand All @@ -510,7 +510,8 @@ def __init__(self, debugger=None, target=None, images=list()):
"""
self.debugger = debugger
self.target = target
self.images = images # a list of images to be used when symbolicating
# a list of images to be used when symbolicating
self.images = images if images else list()
self.addr_mask = 0xFFFFFFFFFFFFFFFF

@classmethod
Expand Down

0 comments on commit 75bed96

Please sign in to comment.