[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

Seems okay to me.
@JDevlieghere How does this look to you?



================
Comment at: lldb/examples/python/scripted_process/crashlog_scripted_process.py:34-37
+                        for section in image.section_infos:
+                            if section.start_addr and section.name == "__TEXT":
+                                self.loaded_images.append({"uuid": str(image.uuid),
+                                                           "load_addr": section.start_addr})
----------------
mib wrote:
> bulbazord wrote:
> > I don't understand the intent of this part. It looks like you're changing the format of `self.loaded_images` here. It's still a List, but instead of containing images it contains specific information about specific sections of each image. If the format has changed, don't consumers of `get_loaded_images` need to be modified as well?
> The list for this specific scripted process class was wrong, it expects the current format. Previously we didn't make use of `loaded_images` for the crashlog_scripted_process class, we just relied on some ad hoc heuristic to load the modules. 
> The other consumers of `get_loaded_images` are fine.
Ah, that makes sense. If one wanted to know what the format of `loaded_images` *should* be, how would they find out? Is that information documented or encoded anywhere? Or is it more ad-hoc?


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

https://reviews.llvm.org/D141702