<div dir="ltr">Hi Aleksandr, yes, no objections to this patch. <div><br></div><div>I was responding to Pavel's comments, which I also assume are forward-looking as well, not strictly related to this patch.</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Nov 29, 2018 at 12:08 PM Aleksandr Urakov <<a href="mailto:aleksandr.urakov@jetbrains.com">aleksandr.urakov@jetbrains.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto">Yes, I agree that the current model of object files - symbol files - symtabs is not clear and orthogonal for cases like PDB, and I think that it requires some redesign too.<div dir="auto"><br></div><div dir="auto">But can we commit this patch now to proceed with several dependent (and not directly related to the theme) patches?</div><div dir="auto"><br></div><div dir="auto">When a new model of work with symbol files will be approved I'm ready to help with fixing the PDB part to conform it.</div></div><br><div class="gmail_quote"><div dir="ltr">Am Do., 29. Nov. 2018, 22:18 hat Leonard Mosescu <<a href="mailto:mosescu@google.com" target="_blank">mosescu@google.com</a>> geschrieben:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Great observations Pavel! I think it's really important to have orthogonal/composable abstractions here: the symbols should be decoupled from the container format IMO.<div><br></div><div>You know more about the ObjectFile than me so I can't say if ObjectFileBreakpad is the best interface, but here are my initial observations (in a somewhat random order):</div><div><br></div><div>1. We need clear and separate abstractions for a container (ELF, PE file, Breakpad symbols) vs. the content (debug Information).</div><div><br></div><div>2. We need to be able to consume symbols when the corresponding module binary is not available. This is common for postmortem debugging (ex. having a minidump + PDBs, but not all the .DLLs or EXE files).</div><div><br></div><div>3. I'm not a fan of the PDB model, where the symbols are searched in both the symtabs then in the symbol files. I'd rather like to see the symtab an interface for symbols regardless of where they come from.</div><div> (Zach expressed some efficiency concerns if we'd need to build a symtab from a PDB for example as opposed to accessing the PDB symfile directly, although I think we can design to address this - ie. multiple concrete symtab implementations, some of which are *internally* aware of the source container, but w/o leaking this through the interface)</div><div><br></div><div>4. The symbol vendor observation is very important. Right now LLDB has basic support for looking up DWARF symbols and none for PDBs. (for example, IMO LLDB could greatly benefit from something like Microsoft's symsrv - I'm actually planning to look into it soon)</div><div> (Whatever we do, this should be one of the key requirements IMO)</div><div><br></div><div>I have a local change to address #2 specifically for PDBs (I'll try to send out a code review soon).</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Nov 29, 2018 at 10:55 AM Pavel Labath via Phabricator via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org" rel="noreferrer" target="_blank">lldb-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">labath added a comment.<br>
<br>
I've recently started looking at adding a new symbol file format (breakpad symbols). While researching the best way to achieve that, I started comparing the operation of PDB and DWARF symbol files. I noticed a very important difference there, and I think that is the cause of our problems here. In the DWARF implementation, a symbol file is an overlay on top of an object file - it takes the data contained by the object file and presents it in a more structured way.<br>
<br>
However, that is not the case with PDB (both implementations). These take the debug information from a completely different file, which is not backed by an ObjectFile instance, and then present that. Since the SymbolFile interface requires them to be backed by an object file, they both pretend they are backed by the original EXE file, but in reality the data comes from elsewhere.<br>
<br>
If we had an ObjectFilePDB (which not also not ideal, though in a way it is a better fit to the current lldb organization), then this could expose the PDB symtab via the existing ObjectFile interface and we could reuse the existing mechanism for merging symtabs from two object files.<br>
<br>
I am asking this because now I am facing a choice in how to implement breakpad symbols. I could go the PDB way, and read the symbols without an intervening object file, or I could create an ObjectFileBreakpad and then (possibly) a SymbolFileBreakpad sitting on top of that.<br>
<br>
The drawbacks of the PDB approach I see are:<br>
<br>
- I lose the ability to do matching of the (real) object file via symbol vendors. The PDB symbol files now basically implement their own little symbol vendors inside them, which is mostly fine if you just need to find the PDB next to the exe file. However, things could get a bit messy if you wanted to implement some more complex searching on multiple paths, or downloading them from the internet.<br>
- I'll hit issues when attempting to unwind (which is the real meat of the breakpad symbols), because unwind info is currently provided via the ObjectFile interface (ObjectFile::GetUnwindTable).<br>
<br>
The drawbacks of the ObjectFile approach are:<br>
<br>
- more code - it needs a new ObjectFile and a new SymbolFile class (possibly also a SymbolVendor)<br>
- it will probably look a bit weird because Breakpad files (and PDBs) aren't really object files<br>
<br>
I'd like to hear your thoughts on this, if you have any.<br>
<br>
<br>
CHANGES SINCE LAST ACTION<br>
<a href="https://reviews.llvm.org/D53368/new/" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D53368/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D53368" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D53368</a><br>
<br>
<br>
<br>
_______________________________________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@lists.llvm.org" rel="noreferrer" target="_blank">lldb-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br>
</blockquote></div>
</blockquote></div>
</blockquote></div>