Looks good - a few comments inline.

The latest version looks good to me. Please update the description (it still says it uses the one CU per symbols file)

Looks good - the all-zero UUID case is a bit unfortunate but it seems we have to handle it (like Greg suggested)

Looks good. A few questions/suggestions inline.

Incorporating Pavel's suggestions (range-based loop, explicit type instead of auto)

Removed sigsegv.exe from the test inputs

I love the idea of custom dump commands, thanks Greg.

How large is the PDB file here?

Incorporating feedback + adding a test case

In D55142#1316153, @labath wrote:

I don't see any tests :(.
Also, the three bullet points in the description sound like rather independent pieces of functionality. Would it be possible to split them up into separate patches? That would make things easier to review, particularly for those who don't look at this code very often :).

Looks like a great start

I noticed a small problem, this change breaks "lldb -c <corefile>". The inline comment explains the root cause.

Shouldn't we also handle the general case where raw size < virtual size? (which would naturally subsume this fix)

Cool. Looking forward to the LLDB integration!

Greg, what do you have in mind as the next step? LLDB integration?

nice!

Using a new, smaller test pdb (Stripped.pdb)

Thanks Zach for your assistance with this!

Incorporating CR feedback

looks good. a few comments inline.

drive by CR notes:

1. does this handle forwarding imports? (it doesn't seem to from a quick glance at the code)
2. can you please add, or extend the existing test to cover the transitive case? A simple dag would suffice (ex. make both dllA and dllB implicitly import dllC)

Looks good to me, minor notes inline.

Nice :)

+Eric Christopher for a DWARF expert's perspective

Yay! Nice to see this is coming along, thanks Zach!

Hi Greg, looking at request_evaluate() I noticed that it will evaluate the
string as a lldb command if prefixed by ` .

Looks good (with one inline request for a comment)

I'm curious too: where did the PDB70 age create matching problems?

In D51176#1211433, @jingham wrote:

The other option would be to move the code that populates the section load list into the mini dump dynamic loader. That has the benefit of keeping this consistent with the other process plugins, but OTOH is just moving code around...

Added the comment requested by zturner

It's an interesting idea, thanks! I don't object moving code around if
there's a strong case for it, but I'd like to keep the fix small and simple
for now, but it's worth considering if the current minidump loading path
will need more flexibility.

In D51176#1211358, @clayborg wrote:

So this might actually work. Take a look around and see what else the dynamic loader is used for and make sure that they won't be needed for anything else. If not, this fix should work, but we should document it.

In D51176#1211307, @clayborg wrote:

Dynamic loaders will fill out the section load list in the target that saids "__TEXT" from "/tmp/a.out" was loaded at address 0x1000020200. So yes they are needed. If the process minidump is manually setting the section load list itself, then there really is no need for a dynamic loader and this fix might work.

That's not changing the module list, that's changing the loaded section information. It is the dynamic loader's job to figure out what got loaded where, plus your stack trace show this happening after we've selected a plugin, not in the process of negotiating for the plugin. Clearing the section load list before setting to work seems like an appropriate thing for a selected plugin to do.

Dynamic loaders are needed for loading breakpad minidumps that are for MacOSX and iOS processes. They should also be needed for loading any minidumps that have stack traces.

The LLDB MI plugin didn't work very well as was quite flaky when I tested
it a while back.

Really cool! Are you planning to add some documentation for it? (set up
instructions, etc...)

Updated the LIT file too

Incorporating feedback, thanks.

Thanks Zach. I can't find llvm::contains() though, any pointers to it?

The new test cases did catch a real bug:

Thanks Greg, looks good to me (a couple of inline comments left at your discretion)

FYI, Breakpad & Crashpad will start generating the Microsoft flavor of ARM
minidumps soon.

I never *ran LLDB tests*, not sure where they are and what they are.

Looks really good, a few comments inline.

The problem is that shared libraries differ on these machines and
LLDB either fails to load some libraries *or loads wrong ones*.

Great timing! ARM support would be most welcome. Are you planning to
support the Breakpad flavor or ARM minidumps or the Microsoft one? (Mark
Mentovai just reminded me that the ARM support was added independently and
some of the structures are different)

The problem is not returning an error from Minidump::Create() - if that was
the case this could easily be improved indeed. The two-phase initialization
is a consequence of the LLDB plugin lookup:

Thanks Adrian for the thorough review.