Page MenuHomePhabricator

[NativePDB] Add support for parsing typedefs and make lldb-test work with the native reader.
ClosedPublic

Authored by zturner on Jan 8 2019, 3:59 PM.

Details

Summary

Typedefs are represented as S_UDT records in the globals stream. This creates a strange situation where "types" are actually represented as "symbols", so they need special handling.

In order to test this, we don't just use lldb and print out some variables causing the AST to get created, because variables whose type is a typedef will have debug info referencing the original type, not the typedef. So we use lldb-test instead which will parse all debug info in the entire file. This exposed some problems with lldb-test and the native reader, mainly that certain types of obscure symbols which we can find when iterating every single record would trigger crashes. These have been fixed as well so that lldb-test can be used to test this functionality.

After this patch, I think most of the remaining DIA PDB tests can be re-written as native PDB tests, so hopefully this means we cane eliminate the DIA reader quite soon.

Diff Detail

Repository
rL LLVM

Event Timeline

zturner created this revision.Jan 8 2019, 3:59 PM
labath added a comment.Jan 9 2019, 6:38 AM

Is there any chance we can get someone who knows something about pdbs take a look at this (@amccarth ? @aleksandr.urakov ?) ?

lldb/lit/SymbolFile/NativePDB/typedefs.cpp
3 ↗(On Diff #180759)

What's the reason for this? Does the test need msvc linker or something?

lemo added inline comments.Jan 9 2019, 4:14 PM
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
626 ↗(On Diff #180759)

leftover comment?

lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.h
70–74 ↗(On Diff #180759)

this, in isolation at least, reads very strange : for m_kind==Union return cvclass.Name and for everything else return cvunion.Name ?

zturner marked 2 inline comments as done.Jan 9 2019, 4:18 PM
zturner added inline comments.
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
626 ↗(On Diff #180759)

Yep, thanks.

lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.h
70–74 ↗(On Diff #180759)

Oops, that's a bug. It's supposed to say

if (m_kind == Struct || m_kind == Class)
  return cvclass.Name;

I think the rest of the function makes sense after that. Thanks for catching that.

zturner marked an inline comment as done.Jan 9 2019, 4:19 PM
zturner added inline comments.
lldb/lit/SymbolFile/NativePDB/typedefs.cpp
3 ↗(On Diff #180759)

It used to say --compiler=msvc until I realized it worked with clang-cl. Then I forgot to remove this. Thanks!

This revision was not accepted when it landed; it landed in state Needs Review.Jan 10 2019, 1:02 PM
This revision was automatically updated to reflect the committed changes.
amccarth added inline comments.Jan 10 2019, 4:56 PM
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
1289 ↗(On Diff #180759)

The body of this loop is a little odd because it's disposing of all the results and is just going through the motions to get side-effects to happen. I realize the root problem is with the design of the API. Perhaps a comment for the uninitiated might point future readers in the right direction.