This is an archive of the discontinued LLVM Phabricator instance.

pdbdump: print out TPI hashes
ClosedPublic

Authored by ruiu on Jun 2 2016, 8:28 PM.

Details

Summary

This patch also fixes a bug in FixedArrayStream. It used to trigger
an assert check if an array is empty.

Diff Detail

Repository
rL LLVM

Event Timeline

ruiu updated this revision to Diff 59487.Jun 2 2016, 8:28 PM
ruiu retitled this revision from to pdbdump: print out TPI hashes.
ruiu updated this object.
ruiu added a reviewer: zturner.
ruiu added a subscriber: llvm-commits.
zturner added inline comments.Jun 2 2016, 8:51 PM
include/llvm/DebugInfo/CodeView/StreamArray.h
188 ↗(On Diff #59487)

I actually fixed this same bug in a patch that went in. So I guess you will have a merge conflict. I did it a little bit differently, but I guess both ways work. When you do merge, feel free to keep whichever you prefer. This way might be a little faster since it drops 2 conditionals from the ++ operator (although it has the downside that if you increment an iterator that is already at end, it will no longer compare equal to end. Not sure if we care, since incrementing end is already undefined behavior.)

include/llvm/DebugInfo/PDB/Raw/TpiStream.h
33 ↗(On Diff #59487)

the first member of OffCb isn't necessarily a type index though, so I don't think this comment is entirely accurate. I'm guessing that in the Microsoft code they are using an OffCb structure parsing this hash table, but I guess that's just because the Offset field coincidentally has the same type as a type index. I think we should probably just delete the comment. Also, I've been putting structs like this in RawTypes.h so that they can be re-used from other code when we need them.

72 ↗(On Diff #59487)

I was doing this at first too, but I think some people had objections to overloading the << operator. I guess you should make a printTypeIndexOffset() function in llvm-pdbdump.cpp

ruiu added inline comments.Jun 2 2016, 9:13 PM
include/llvm/DebugInfo/CodeView/StreamArray.h
188 ↗(On Diff #59487)

I'm not an C++ expert, but I think in the first place we shouldn't increment an iterator that is already at end(). At least I believe if an iterator is a pointer pointing to a plain C array, the result of incrementing an iterator at end() is undefined.

include/llvm/DebugInfo/PDB/Raw/TpiStream.h
33 ↗(On Diff #59487)

Will do.

72 ↗(On Diff #59487)

Will do.

ruiu updated this revision to Diff 59593.Jun 3 2016, 11:53 AM
  • Address review comments.
zturner accepted this revision.Jun 3 2016, 12:36 PM
zturner edited edge metadata.

Looks good after rebasing and moving the print method

include/llvm/DebugInfo/CodeView/StreamArray.h
248 ↗(On Diff #59593)

Perhaps assert that Index < Array.size() instead.

tools/llvm-pdbdump/llvm-pdbdump.cpp
698 ↗(On Diff #59593)

I think you will need to rebase and move this method to a pure virtual method on OutputStyle and implement it in LLVMOutputStyle. Sorry :-/ Good news is I should be done with all the intrusive changes now and can work in isolation entirely on the yaml outputter.

This revision is now accepted and ready to land.Jun 3 2016, 12:36 PM
This revision was automatically updated to reflect the committed changes.