This is an archive of the discontinued LLVM Phabricator instance.

[NativePDB] Add support for handling S_CONSTANT records
ClosedPublic

Authored by zturner on Nov 12 2018, 3:31 PM.

Details

Summary

clang-cl does not emit these, but MSVC does, so we need to be able to handle them.

Because clang-cl does not generate them, it was a bit hard to write a test. So what I had to do was get an PDB file with some S_CONSTANT records in using cl and link, dump it using llvm-pdbutil dump -globals -sym-data to get the bytes of the records, generate the same object file using clang-cl but with -S to emit an assembly file, and replace all the S_LDATA32 records with the bytes of the S_CONSTANT records. This way, we can compile the file using llvm-mc and link it with lld-link.

Diff Detail

Repository
rL LLVM

Event Timeline

zturner created this revision.Nov 12 2018, 3:31 PM
lemo accepted this revision.Nov 12 2018, 3:37 PM
lemo added inline comments.
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
329 ↗(On Diff #173775)

Just a suggestion: I'm not a big fan of returning std::pair<>. I'd use a simple struct instead.

This revision is now accepted and ready to land.Nov 12 2018, 3:37 PM

LGTM too, thanks!

aleksandr.urakov accepted this revision.Nov 13 2018, 3:09 AM
shafik added a subscriber: shafik.Nov 13 2018, 9:47 AM
shafik added inline comments.
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
329 ↗(On Diff #173775)

I have to concur on this, although the use of std::tie helps alleviate the pain i.e. no second and first it does feel more verbose on the calling side to declare two variables and then do the tie and it relies on the calling to choose meaningful names which can't be relied on if the use spreads and people are not as careful.

This revision was automatically updated to reflect the committed changes.