[PATCH] D20717: pdbdump: print out COFF section headers.

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 1 11:16:37 PDT 2016


zturner added a comment.

Sorry again for the delay, kept forgetting about this.


================
Comment at: include/llvm/DebugInfo/PDB/Raw/PDBFile.h:79-80
@@ -69,2 +78,4 @@
   Expected<SymbolStream &> getPDBSymbolStream();
+  Expected<codeview::FixedStreamArray<object::coff_section>>
+      getPDBSectionHeaders();
 
----------------
I don't think this should go in `PDBFile`.  Seems to me like it should be accessible from `DBIStream`.

================
Comment at: lib/DebugInfo/PDB/Raw/PDBFile.cpp:341
@@ +340,3 @@
+PDBFile::getPDBSectionHeaders() {
+  if (!SectionHeaders) {
+    auto DbiS = getPDBDbiStream();
----------------
It's going to do this each time someone calls the function.  Now that everything is using StreamRef and zero-copy, there is no overhead to creating the `FixedStreamArray` up front, because it's not actually parsing anything until you iterate the collection.  

So I would move this all to `DBIStream` (as mentioned earlier) and parse this at the time you call `reload`.  Since that doesn't actually parse anything, you can do it once and just store the `FixedStreamArray` inside of `DBIStream` class and return it through a trivial accessor.


http://reviews.llvm.org/D20717





More information about the llvm-commits mailing list