[Lldb-commits] [PATCH] D146765: [lldb/crashlog] Load inlined symbol into interactive crashlog

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.


================
Comment at: lldb/examples/python/crashlog.py:540
 
+            image = self.get_used_image(image_id)
             frame_offset = int(json_frame['imageOffset'])
----------------
is `image` not the same as `json_image` from a few lines above this?


================
Comment at: lldb/examples/python/crashlog.py:552-557
+                self.symbol_data[image_uuid]["symbols"].append({
+                    "name": json_frame['symbol'],
+                    "type": "code",
+                    "size": 0,
+                    "address": pc,
+                })
----------------
mib wrote:
> Same problem here I guess: IIUC if `pc = image['base'] + frame_offset`, then I believe `frame_offset` is not the address of the beginning of the function, but rather an address in the middle of the function (either at a callsite, or at the crash site).
Yeah so `pc` is definitely not the right address of the symbol. It should be `pc - frame_offset` right?


================
Comment at: lldb/examples/python/crashlog.py:606-611
+            if len(frame_match.groups()) == 4:
+                (frame_id, frame_img_name, frame_addr,
+                    frame_ofs) = frame_match.groups()
+            else:
+                (frame_id, frame_img_name, frame_addr,
+                    frame_ofs, frame_symbol, frame_offset) = frame_match.groups()
----------------
What is the difference between `frame_ofs` and `frame_offset`?


================
Comment at: lldb/examples/python/crashlog.py:1175-1176
+        if crashlog_parser.__class__ == JSONCrashLogParser:
+                u = uuid.UUID(image)
+                module_spec.SetUUIDFromString(u.hex)
+        else:
----------------
Are these indented too much?


================
Comment at: lldb/examples/python/crashlog.py:1194-1195
+        if not result.Succeeded():
+            raise InteractiveCrashLogException("couldn't import crash report \
+                                               inlined symbols for %s (%s)" %
+                                               (module.file.basename,
----------------
Won't this string look like `"couldn't import crash report                                                inlined symbols for %s (%s)"`? You could probably just do string concatenation instead. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146765/new/

https://reviews.llvm.org/D146765