This is an archive of the discontinued LLVM Phabricator instance.

[pdb] Actually write a PDB to the disk
ClosedPublic

Authored by zturner on Jun 10 2016, 12:49 AM.

Details

Summary

Finally! We can write a real PDB file to the disk. There's still quite a bit of work to be done on this codepath, but in its current limited form, it can dump MSF headers and stream directory information to YAML, then read it back in and produce a PDB from that information, then dump the resulting PDB back to YAML and the dumped YAML is identical.

Diff Detail

Repository
rL LLVM

Event Timeline

zturner updated this revision to Diff 60316.Jun 10 2016, 12:49 AM
zturner retitled this revision from to [pdb] Actually write a PDB to the disk.
zturner updated this object.
zturner added reviewers: rnk, ruiu, majnemer.
zturner added a subscriber: llvm-commits.
ruiu added inline comments.Jun 14 2016, 11:06 AM
include/llvm/DebugInfo/CodeView/StreamRef.h
96 ↗(On Diff #60316)

Is this different from the default copy assignment operator?

include/llvm/DebugInfo/CodeView/StreamWriter.h
44 ↗(On Diff #60316)

Can you use static_assert instead of enable_if_t?

include/llvm/DebugInfo/PDB/Raw/PDBFile.h
122 ↗(On Diff #60316)

ArrayRef<ArrayRef<support::ulittle32_t>> is probably better for consistency.

lib/DebugInfo/PDB/Raw/PDBFile.cpp
287 ↗(On Diff #60316)

We shouldn't meet the error conditions that we are checking in this function unless there's a bug in PDB producer code or YAML reader code (YAML reader should do reasonable semantic checking), so they should be asserts.

zturner added inline comments.Jun 14 2016, 11:13 AM
include/llvm/DebugInfo/CodeView/StreamRef.h
96 ↗(On Diff #60316)

I don't think so, this might have slipped in accidentally.

include/llvm/DebugInfo/CodeView/StreamWriter.h
44 ↗(On Diff #60316)

Probably so, I will try it.

lib/DebugInfo/PDB/Raw/PDBFile.cpp
287 ↗(On Diff #60316)

Technically a PDB file is a program input, we could generate one through fuzzing for example. Also we can't assume we are the only producer of PDB files. And since this will be in library code, I don't know if I feel comfortable asserting on conditions of the program input.

zturner updated this revision to Diff 60715.Jun 14 2016, 11:58 AM
ruiu accepted this revision.Jun 14 2016, 1:04 PM
ruiu edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jun 14 2016, 1:04 PM
This revision was automatically updated to reflect the committed changes.