This is an archive of the discontinued LLVM Phabricator instance.

Remove faulty assertion in llvm-pdbutil
ClosedPublic

Authored by amccarth on Apr 13 2018, 4:41 PM.

Details

Summary

If a class's first data member is an instance of an empty class, then an assertion in the PrettyClassLayoutGraphicalDumper would fail. The storage is reserved, but it's not marked as in use.

As far as I understand, it's the assertion that's faulty, so I removed it and updated the nearby comment. If you think the MapUse is computed wrong in this case, let me know and I'll investigate further.

Found by running llvm-pdbutil against its own PDB, and this assertion would fail on HashAdjusters, which is a HashTable whose first data member is a TraitsT, which is a PdbHashTraits<T>, which is an empty struct. (The struct has a specialization for uint32_t, but that specialization doesn't apply here because the T is actually ulittle32_t.)

Diff Detail

Repository
rL LLVM

Event Timeline

amccarth created this revision.Apr 13 2018, 4:41 PM

I meant to mention that the instance of the empty class is reported as 8 bytes of padding (in a 64-bit build), in case that's relevant to anybody's understanding of the issue.

If the tests pass, this is probably fine. Honestly this code is really
complex and while I think it’s useful, it may not be useful enough to
justify the complexity so I’m thinking about getting rid of it

This revision was not accepted when it landed; it landed in state Needs Review.Apr 16 2018, 10:05 AM
This revision was automatically updated to reflect the committed changes.