Support member function types in PdbAstBuilder

[zloyrobot]

Summary

This path implements missing case in PdbAstBuilder::CreateType for LF_MFUNCTION. This is necessary, for example, in stack unwinding of struct methods.

[amccarth]

Please add some detail to the commit message and consider re-titling it. It looks like this patch is actually adding missing functionality rather than "fixing" a bug in how structs are handled. If that's correct, then please make the commit message reflect that.

Sorry I didn't get a chance to finish reviewing this today, but I'll try to get it done tomorrow.

[amccarth]

Thanks for the improved commit message. Again, sorry about the delay.

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h

183

Suggestion: Consider CreateMemberFunctionType or CreateMethodType to avoid confusion with CreateProcedureType.

[zloyrobot]

In D61128#1482765, @amccarth wrote:

Thanks for the improved commit message. Again, sorry about the delay.

No problem, thanks for review!

[zloyrobot]

In D61128#1483663, @zloyrobot wrote:

In D61128#1482765, @amccarth wrote:

Thanks for the improved commit message. Again, sorry about the delay.

No problem, thanks for review!

I afraid I don't have commit access to merge this path. Could somebody help me?

[labath]

Looks like this is failing on the windows bot http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/4635/steps/test/logs/stdio.

Looking at the test, it seems that you are asserting that you will unwind into a particular offset in ntdll.dll. That sounds like an incredibly fragile assertion, which will only be true for people running the exact same version of ntdll that you happen to have...

[zloyrobot]

In D61128#1499934, @labath wrote:

Looks like this is failing on the windows bot http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/4635/steps/test/logs/stdio.

Looking at the test, it seems that you are asserting that you will unwind into a particular offset in ntdll.dll. That sounds like an incredibly fragile assertion, which will only be true for people running the exact same version of ntdll that you happen to have...

My bad, these assertions (ntdll and kernel32) should be removed from test. I already asked @aleksandr.urakov to do it.

lldb/lit/SymbolFile/NativePDB/Inputs/stack_unwinding01.lldbinit

+b stack_unwinding01.cpp:12
+run
+thread backtrace
+c
+thread backtrace
+c
+thread backtrace
+quit
 No newline at end of file

lldb/lit/SymbolFile/NativePDB/stack_unwinding01.cpp

+// clang-format off
+// REQUIRES: lld
+
+// RUN: %build --compiler=clang-cl --nodefaultlib -o %t.exe -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+// RUN:     %p/Inputs/stack_unwinding01.lldbinit 2>&1 | FileCheck %s
+
+
+struct Struct {
+  void simple_method(int a, int b) {
+     a += 1;
+     simple_method(a, b);
+  }
+};
+
+
+
+int main(int argc, char **argv) {
+  Struct s;
+  s.simple_method(1,2);
+  return 0;
+}
+
+
+// CHECK: (lldb) thread backtrace
+// CHECK-NEXT: * thread #1, stop reason = breakpoint 1.1
+// CHECK-NEXT:   * frame #0: {{.*}} stack_unwinding01.cpp.tmp.exe`Struct::simple_method at stack_unwinding01.cpp:12
+// CHECK-NEXT:     frame #1: {{.*}} stack_unwinding01.cpp.tmp.exe`main(argc={{.*}}, argv={{.*}}) at stack_unwinding01.cpp:20
+// CHECK-NEXT:     frame #2: {{.*}} kernel32.dll`BaseThreadInitThunk + 34
+// CHECK-NEXT:     frame #3: {{.*}} ntdll.dll`RtlUserThreadStart + 52
+
+
+// CHECK: (lldb) thread backtrace
+// CHECK-NEXT: * thread #1, stop reason = breakpoint 1.1
+// CHECK-NEXT:   * frame #0: {{.*}} stack_unwinding01.cpp.tmp.exe`Struct::simple_method at stack_unwinding01.cpp:12
+// CHECK-NEXT:     frame #1: {{.*}} stack_unwinding01.cpp.tmp.exe`Struct::simple_method at stack_unwinding01.cpp:12
+// CHECK-NEXT:     frame #2: {{.*}} stack_unwinding01.cpp.tmp.exe`main(argc={{.*}}, argv={{.*}}) at stack_unwinding01.cpp:20
+// CHECK-NEXT:     frame #3: {{.*}} kernel32.dll`BaseThreadInitThunk + 34
+// CHECK-NEXT:     frame #4: {{.*}} ntdll.dll`RtlUserThreadStart + 52
+
+// CHECK: (lldb) thread backtrace
+// CHECK-NEXT: * thread #1, stop reason = breakpoint 1.1
+// CHECK-NEXT:   * frame #0: {{.*}} stack_unwinding01.cpp.tmp.exe`Struct::simple_method at stack_unwinding01.cpp:12
+// CHECK-NEXT:     frame #1: {{.*}} stack_unwinding01.cpp.tmp.exe`Struct::simple_method at stack_unwinding01.cpp:12
+// CHECK-NEXT:     frame #2: {{.*}} stack_unwinding01.cpp.tmp.exe`Struct::simple_method at stack_unwinding01.cpp:12
+// CHECK-NEXT:     frame #3: {{.*}} stack_unwinding01.cpp.tmp.exe`main(argc={{.*}}, argv={{.*}}) at stack_unwinding01.cpp:20
+// CHECK-NEXT:     frame #4: {{.*}} kernel32.dll`BaseThreadInitThunk + 34
+// CHECK-NEXT:     frame #5: {{.*}} ntdll.dll`RtlUserThreadStart + 52
 No newline at end of file

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h

lldb::TypeSP CreateTagType(PdbTypeSymId type_id,
                              const llvm::codeview::EnumRecord &er,
                              CompilerType ct);
   lldb::TypeSP CreateTagType(PdbTypeSymId type_id,
                              const llvm::codeview::UnionRecord &ur,
                              CompilerType ct);
   lldb::TypeSP CreateArrayType(PdbTypeSymId type_id,
                                const llvm::codeview::ArrayRecord &ar,
                                CompilerType ct);
+  lldb::TypeSP CreateFunctionType(PdbTypeSymId type_id,
+                                  const llvm::codeview::MemberFunctionRecord &pr,
+                                  CompilerType ct);
   lldb::TypeSP CreateProcedureType(PdbTypeSymId type_id,
                                    const llvm::codeview::ProcedureRecord &pr,
                                    CompilerType ct);
   lldb::TypeSP CreateClassStructUnion(PdbTypeSymId type_id,
                                       const llvm::codeview::TagRecord &record,
                                       size_t size, CompilerType ct);
 
   lldb::FunctionSP GetOrCreateFunction(PdbCompilandSymId func_id,

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

TypeSP SymbolFileNativePDB::CreateArrayType(PdbTypeSymId type_id,
   TypeSP array_sp = std::make_shared<lldb_private::Type>(
       toOpaqueUid(type_id), this, ConstString(), ar.Size, nullptr,
       LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl, ct,
       lldb_private::Type::eResolveStateFull);
   array_sp->SetEncodingType(element_type.get());
   return array_sp;
 }
 
+
+TypeSP SymbolFileNativePDB::CreateFunctionType(PdbTypeSymId type_id,
+                                               const MemberFunctionRecord &mfr,
+                                               CompilerType ct) {
+  Declaration decl;
+  return std::make_shared<lldb_private::Type>(
+      toOpaqueUid(type_id), this, ConstString(), 0, nullptr, LLDB_INVALID_UID,
+      lldb_private::Type::eEncodingIsUID, decl, ct,
+      lldb_private::Type::eResolveStateFull);
+}
+
 TypeSP SymbolFileNativePDB::CreateProcedureType(PdbTypeSymId type_id,
                                                 const ProcedureRecord &pr,
                                                 CompilerType ct) {
   Declaration decl;
   return std::make_shared<lldb_private::Type>(
       toOpaqueUid(type_id), this, ConstString(), 0, nullptr, LLDB_INVALID_UID,
       lldb_private::Type::eEncodingIsUID, decl, ct,
       lldb_private::Type::eResolveStateFull);
if (cvt.kind() == LF_ARRAY) {
     return CreateArrayType(type_id, ar, ct);
   }
 
   if (cvt.kind() == LF_PROCEDURE) {
     ProcedureRecord pr;
     llvm::cantFail(TypeDeserializer::deserializeAs<ProcedureRecord>(cvt, pr));
     return CreateProcedureType(type_id, pr, ct);
   }
+  if (cvt.kind() == LF_MFUNCTION) {
+    MemberFunctionRecord mfr;
+    llvm::cantFail(TypeDeserializer::deserializeAs<MemberFunctionRecord>(cvt, mfr));
+    return CreateFunctionType(type_id, mfr, ct);
+  }
 
   return nullptr;
 }
 
 TypeSP SymbolFileNativePDB::CreateAndCacheType(PdbTypeSymId type_id) {
   // If they search for a UDT which is a forward ref, try and resolve the full
   // decl and just map the forward ref uid to the full decl record.
   llvm::Optional<PdbTypeSymId> full_decl_uid;