This is an archive of the discontinued LLVM Phabricator instance.

Add documentation for the PDB file format
ClosedPublic

Authored by zturner on Nov 7 2016, 3:50 PM.

Details

Summary

This is a first step towards getting complete documentation of the PDB format. I plan to fill out the additional files in an equal amount of detail to what you see in the MSF documentation in later iterations, but for now I wanted to define the documentation hierarchy, and fill out the high level details of the format and get one page filled out.

Diff Detail

Repository
rL LLVM

Event Timeline

zturner updated this revision to Diff 77110.Nov 7 2016, 3:50 PM
zturner retitled this revision from to Add documentation for the PDB file format.
zturner updated this object.
zturner added reviewers: majnemer, amccarth, inglorion, rnk, ruiu.
zturner added a subscriber: llvm-commits.
amccarth edited edge metadata.Nov 7 2016, 4:43 PM

I've noted a couple of nits and possibilities for clarifications, but a nice start. Thanks for writing this.

docs/PDB/MsfFile.rst
49 ↗(On Diff #77110)

Wording nit: "describing the layout of the Stream Directory" doesn't seem to capture what I think you mean. How about "listing the blocks that contain the Stream Directory"?

55 ↗(On Diff #77110)

Did you mean `[NumDirectoryBytes / 4]`?

56 ↗(On Diff #77110)

I assume the space between the header and the next block is just 0-filled padding?

docs/PDB/index.rst
17 ↗(On Diff #77110)

grammar nit: s/have/has/ because "ecosystem" is singular.

24 ↗(On Diff #77110)

delete "with"

zturner added inline comments.Nov 7 2016, 4:57 PM
docs/PDB/MsfFile.rst
55 ↗(On Diff #77110)

No, this is correct. For starters, that's intentionally a mathematical ceiling operator and not a square bracket :) That aside though, Suppose your directory is magically exactly 4KiB and your block size is also 4KiB. Then your directory fits on a single block, and thus you only need a single ulittle32 to describe the list of blocks that it lies on.

zturner updated this revision to Diff 77123.Nov 7 2016, 5:01 PM
zturner edited edge metadata.
rnk accepted this revision.Nov 8 2016, 8:03 AM
rnk edited edge metadata.

Content and layout looks good. Let's let Adrian finish reviewing the wording.

docs/PDB/index.rst
156 ↗(On Diff #77123)

I wrote some CV stuff here:
http://llvm.org/docs/SourceLevelDebugging.html#codeview-debug-info-format

It should probably be split out and moved in here. I'll take a shot at that after this lands, and then hook up these links.

This revision is now accepted and ready to land.Nov 8 2016, 8:03 AM
majnemer added inline comments.Nov 8 2016, 8:14 AM
docs/PDB/MsfFile.rst
20 ↗(On Diff #77123)

I think this name would be a little more self-descriptive if it were called FreeBlockMapAddr. This would also make it consistent with BlockMapAddr.

24 ↗(On Diff #77123)

Looking at the microsoft-pdb sources, isn't this technically an array of addresses?
https://github.com/Microsoft/microsoft-pdb/blob/master/PDB/msf/msf.cpp#L954

zturner added inline comments.Nov 8 2016, 8:26 AM
docs/PDB/MsfFile.rst
24 ↗(On Diff #77123)

It's more like a pointer to an array. It's a single uint32 which is the index of a block, and at that block is an array.

majnemer added inline comments.Nov 8 2016, 9:00 AM
docs/PDB/MsfFile.rst
24 ↗(On Diff #77123)

My reading of the PDB sources makes me thing that its an array of pointers to arrays. I don't think we ever encountered this in the wild because our PDBs were not large enough.

amccarth added inline comments.Nov 8 2016, 10:02 AM
docs/PDB/MsfFile.rst
55 ↗(On Diff #77110)

Ah, got it. Thanks for the clarification.

The ceiling operators in monospace font are really hard to distinguish from square brackets. Given that we don't have full TeX capabilities, maybe it would be better to make it more code-like: ceil(NumDirectoryBytes / BlockSize).

zturner updated this revision to Diff 77412.Nov 9 2016, 3:35 PM
zturner edited edge metadata.

Updated again

This revision was automatically updated to reflect the committed changes.