bulbazord added a comment.
Looks good! A few minor comments.
================
Comment at: lldb/examples/python/crashlog.py:628-629
+ description = ""
+ # Since images are parsed after threads, we need to build a
+ # map for every image with a list of all the symbols and addresses
+ if frame_img_name and frame_addr and frame_symbol:
----------------
I think this comment better describes `parse_thread` rather than
`parse_asi_backtrace` no?
================
Comment at: lldb/examples/python/crashlog.py:634-635
+ description += " + " + frame_offset
+ for image in self.images:
+ if image.identifier == frame_img_name:
+ image.symbols[frame_symbol] = {
----------------
Does it make sense for `self.images` to be a dictionary instead of a list?
Something that maps `image_name -> image_info`. That way you could do something
like `self.images[frame_img_name].symbols[frame_symbol] = { ... }` instead of
iterating over every image (with appropriate error checking in the event that
frame_img_name isn't in self.images yet).
================
Comment at: lldb/examples/python/crashlog.py:639
+ "type": "code",
+ "address": int(frame_addr, 0) - int(frame_offset)
+ }
----------------
if `frame_offset` is `None`, you're doing `int(None)` which will give you a
`TypeError`. Above you can do something like `frame_offset_value = 0` and in
the `if frame_offset:` block you can do `frame_offset_value =
int(frame_offset)`.
================
Comment at: lldb/examples/python/crashlog.py:708
+ r'))(?: ' # capture group garbage
+ r'\(' # source infor capture group
+ r'([^\:]+)' # file name
----------------
typo: `infor` -> `info`
================
Comment at: lldb/examples/python/crashlog.py:711
+ r'\:(\d+)' # line number
+ r'(?:\:' # capture group garbage
+ r'(\d+)' # column number
----------------
What does `garbage` mean in this case? I assume it's boilerplate of some kind.
================
Comment at: lldb/examples/python/crashlog.py:936
+ description += " + " + frame_offset
+ if not frame_img_name in self.symbols:
+ self.symbols[frame_img_name] = list()
----------------
================
Comment at: lldb/examples/python/crashlog.py:940
+ "name": frame_symbol,
+ "address": int(frame_addr, 0) - int(frame_offset)
+ })
----------------
Same situation here as above, `frame_offset` might be `None` and then you'll
crash with a `TypeError`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146765/new/
https://reviews.llvm.org/D146765
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
