This is an archive of the discontinued LLVM Phabricator instance.

pdbdump: print out COFF section headers.
ClosedPublic

Authored by ruiu on May 26 2016, 6:53 PM.

Details

Summary

Unlike other sections that can grow to any size, the COFF section header
stream has maximum length because each record is fixed size and the COFF
file format limits the maximum number of sections. So I decided to not
create a specific stream class for it. Instead, I added a member function
to PDBFile class which returns a vector of COFF headers.

I think it is enough, but I'm open to other options, so let me know if
you think the other way.

Diff Detail

Event Timeline

ruiu updated this revision to Diff 58741.May 26 2016, 6:53 PM
ruiu retitled this revision from to pdbdump: print out COFF section headers..
ruiu updated this object.
ruiu added a reviewer: zturner.
ruiu added a subscriber: llvm-commits.
majnemer added inline comments.
tools/llvm-pdbdump/llvm-pdbdump.cpp
722

Might be nice to dump the flags.

zturner edited edge metadata.May 26 2016, 6:56 PM

Admittedly these section headers aren't too big, but if we want to standardize on the new stream interface stuff, maybe you can wait until my memory patch goes in, and then use FixedStreamArray<object::coff_section>.

Eventually I'd like to even delete the methods of StreamReader that allocate memory

It's in now as of r270951, btw, so sync up and see if it works.

ruiu updated this revision to Diff 58790.May 27 2016, 8:24 AM
ruiu edited edge metadata.
  • Updated to use FixedArrayStream.

Sorry again for the delay, kept forgetting about this.

include/llvm/DebugInfo/PDB/Raw/PDBFile.h
79–80 ↗(On Diff #58790)

I don't think this should go in PDBFile. Seems to me like it should be accessible from DBIStream.

lib/DebugInfo/PDB/Raw/PDBFile.cpp
341 ↗(On Diff #58790)

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.

ruiu added inline comments.Jun 1 2016, 3:27 PM
include/llvm/DebugInfo/PDB/Raw/PDBFile.h
79–80 ↗(On Diff #58790)

So is publics stream?

lib/DebugInfo/PDB/Raw/PDBFile.cpp
341 ↗(On Diff #58790)

Ah, that sounds like a good idea. Will do.

zturner added inline comments.Jun 1 2016, 3:42 PM
include/llvm/DebugInfo/PDB/Raw/PDBFile.h
79–80 ↗(On Diff #58790)

I think publics stream is fine at the top level. After all, it is a top-level stream, you just have to look in the DBI stream to figure out its index. The coff section headers though is just an array that is embedded entirely inside the DBI Stream.

It's kind of a judgement call though, I wouldn't object to publics stream being accessible from DBI stream, but I think "streams" are the only thing that should be accessible from the file.

ruiu updated this revision to Diff 59423.Jun 2 2016, 10:50 AM
  • Move getSectionHeaders function from PDBFile to DbiStream.
zturner accepted this revision.Jun 2 2016, 11:19 AM
zturner edited edge metadata.
This revision is now accepted and ready to land.Jun 2 2016, 11:19 AM
This revision was automatically updated to reflect the committed changes.