[Lldb-commits] [lldb] r350764 - Change lldb-test to use ParseAllDebugSymbols.

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Wed Jan 9 13:20:44 PST 2019


Author: zturner
Date: Wed Jan  9 13:20:44 2019
New Revision: 350764

URL: http://llvm.org/viewvc/llvm-project?rev=350764&view=rev
Log:
Change lldb-test to use ParseAllDebugSymbols.

ParseDeclsForContext was originally created to serve the very specific
case where the context is a function block. It was never intended to be
used for arbitrary DeclContexts, however due to the generic name, the
DWARF and PDB plugins implemented it in this way "just in case". Then,
lldb-test came along and decided to use it in that way.

Related to this, there are a set of functions in the SymbolFile class
interface whose requirements and expectations are not documented. For
example, if you call ParseCompileUnitFunctions, there's an inherent
requirement that you create entries in the underlying clang AST for
these functions as well as their signature types, because in order to
create an lldb_private::Function object, you have to pass it a
CompilerType for the parameter representing the signature.

On the other hand, there is no similar requirement (either inherent or
documented) if one were to call ParseDeclsForContext. Specifically, if
one calls ParseDeclsForContext, and some variable declarations, types,
and other things are added to the clang AST, is it necessary to create
lldb::Variable, lldb::Type, etc objects representing them? Nobody knows.
There is, however, an accidental requirement, because since all of the
plugins implemented this just in case, lldb-test came along and used
ParsedDeclsForContext, and then wrote check lines that depended on this.

When I went to try and implemented the NativePDB reader, I did not
adhere to this (in fact, from a layering perspective I went out of my
way to avoid it), and as a result the existing DIA PDB tests don't work
when the native PDB reader is enabled, because they expect that calling
ParseDeclsForContext will modify the *module's* view of symbols, and not
just the internal AST.

All of this confusion, however, can be avoided if we simply stick to
using ParseDeclsForContext for its original intended use case (blocks),
and use a different function (ParseAllDebugSymbols) for its intended use
case which is, unsuprisingly, to parse all the debug symbols (which is
all lldb-test really wanted to do anyway).

In the future, I would like to change ParseDeclsForContext to
ParseDeclsForFunctionBlock, then delete all of the dead code inside that
handles other types of DeclContexts (and probably even assert if the
DeclContext is anything other than a block).

A few PDB tests needed to be fixed up as a result of this, and this also
exposed a couple of bugs in the DIA PDB reader (doesn't matter much
since it should be going away soon, but worth mentioning) where the
appropriate AST entries weren't being created always.

Differential Revision: https://reviews.llvm.org/D56418

Modified:
    lldb/trunk/lit/SymbolFile/PDB/enums-layout.test
    lldb/trunk/lit/SymbolFile/PDB/type-quals.test
    lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
    lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
    lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/lit/SymbolFile/PDB/enums-layout.test
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/enums-layout.test?rev=350764&r1=350763&r2=350764&view=diff
==============================================================================
--- lldb/trunk/lit/SymbolFile/PDB/enums-layout.test (original)
+++ lldb/trunk/lit/SymbolFile/PDB/enums-layout.test Wed Jan  9 13:20:44 2019
@@ -1,44 +1,45 @@
 REQUIRES: system-windows, msvc
 RUN: %build --compiler=msvc --arch=32 --nodefaultlib --output=%T/SimpleTypesTest.cpp.enums.exe %S/Inputs/SimpleTypesTest.cpp
-RUN: lldb-test symbols %T/SimpleTypesTest.cpp.enums.exe | FileCheck %s
+RUN: lldb-test symbols %T/SimpleTypesTest.cpp.enums.exe | FileCheck --check-prefix=ENUM %s
+RUN: lldb-test symbols %T/SimpleTypesTest.cpp.enums.exe | FileCheck --check-prefix=ENUM_CONST %s
+RUN: lldb-test symbols %T/SimpleTypesTest.cpp.enums.exe | FileCheck --check-prefix=ENUM_EMPTY %s
+RUN: lldb-test symbols %T/SimpleTypesTest.cpp.enums.exe | FileCheck --check-prefix=ENUM_UCHAR %s
+RUN: lldb-test symbols %T/SimpleTypesTest.cpp.enums.exe | FileCheck --check-prefix=ENUM_CLASS %s
+RUN: lldb-test symbols %T/SimpleTypesTest.cpp.enums.exe | FileCheck --check-prefix=ENUM_STRUCT %s
 
 ; FIXME: PDB does not have information about scoped enumeration (Enum class) so the  
 ; compiler type used is the same as the one for unscoped enumeration.
 
-CHECK: Module [[CU:.*]]
-CHECK-DAG: {{^[0-9A-F]+}}: SymbolVendor ([[CU]])
-CHECK:      Type{{.*}} , name = "Enum", size = 4, decl = simpletypestest.cpp:19, compiler_type = {{.*}} enum Enum {
-CHECK-NEXT:    RED,
-CHECK-NEXT:    GREEN,
-CHECK-NEXT:    BLUE
-CHECK-NEXT:}
-
-CHECK:      Type{{.*}} , name = "EnumConst", size = 4,  decl = simpletypestest.cpp:22, compiler_type = {{.*}} enum EnumConst {
-CHECK-NEXT:    LOW,
-CHECK-NEXT:    NORMAL,
-CHECK-NEXT:    HIGH
-CHECK-NEXT:}
-
-CHECK:      Type{{.*}} , name = "EnumEmpty", size = 4,  decl = simpletypestest.cpp:25, compiler_type = {{.*}} enum EnumEmpty {
-CHECK-NEXT:}
-
-CHECK:      Type{{.*}} , name = "EnumUChar", size = 1,  decl = simpletypestest.cpp:28, compiler_type = {{.*}} enum EnumUChar {
-CHECK-NEXT:    ON,
-CHECK-NEXT:    OFF,
-CHECK-NEXT:    AUTO
-CHECK-NEXT:}
+ENUM:      Type{{.*}} , name = "Enum", size = 4, decl = simpletypestest.cpp:19, compiler_type = {{.*}} enum Enum {
+ENUM_NEXT:    RED,
+ENUM_NEXT:    GREEN,
+ENUM_NEXT:    BLUE
+ENUM_NEXT:}
+
+ENUM_CONST:      Type{{.*}} , name = "EnumConst", size = 4,  decl = simpletypestest.cpp:22, compiler_type = {{.*}} enum EnumConst {
+ENUM_CONST-NEXT:    LOW,
+ENUM_CONST-NEXT:    NORMAL,
+ENUM_CONST-NEXT:    HIGH
+ENUM_CONST-NEXT:}
+
+ENUM_EMPTY:      Type{{.*}} , name = "EnumEmpty", size = 4,  decl = simpletypestest.cpp:25, compiler_type = {{.*}} enum EnumEmpty {
+ENUM_EMPTY-NEXT:}
+
+ENUM_UCHAR:      Type{{.*}} , name = "EnumUChar", size = 1,  decl = simpletypestest.cpp:28, compiler_type = {{.*}} enum EnumUChar {
+ENUM_UCHAR-NEXT:    ON,
+ENUM_UCHAR-NEXT:    OFF,
+ENUM_UCHAR-NEXT:    AUTO
+ENUM_UCHAR-NEXT:}
 
 ; Note that `enum EnumClass` is tested instead of `enum class EnumClass`
-CHECK:      Type{{.*}} , name = "EnumClass", size = 4,  decl = simpletypestest.cpp:32, compiler_type = {{.*}} enum EnumClass {
-CHECK-NEXT:    YES,
-CHECK-NEXT:    NO,
-CHECK-NEXT:    DEFAULT
-CHECK-NEXT:}
-
-CHECK:      Type{{.*}} , name = "EnumStruct", size = 4,  decl = simpletypestest.cpp:35, compiler_type = {{.*}} enum EnumStruct {
-CHECK-NEXT:    red,
-CHECK-NEXT:    blue,
-CHECK-NEXT:    black
-CHECK-NEXT:}
-
-CHECK-DAG: {{^[0-9A-F]+}}:   CompileUnit{{[{]0x[0-9a-f]+[}]}}, language = "c++", file = '{{.*}}\SimpleTypesTest.cpp'
+ENUM_CLASS:      Type{{.*}} , name = "EnumClass", size = 4,  decl = simpletypestest.cpp:32, compiler_type = {{.*}} enum EnumClass {
+ENUM_CLASS-NEXT:    YES,
+ENUM_CLASS-NEXT:    NO,
+ENUM_CLASS-NEXT:    DEFAULT
+ENUM_CLASS-NEXT:}
+
+ENUM_STRUCT:      Type{{.*}} , name = "EnumStruct", size = 4,  decl = simpletypestest.cpp:35, compiler_type = {{.*}} enum EnumStruct {
+ENUM_STRUCT-NEXT:    red,
+ENUM_STRUCT-NEXT:    blue,
+ENUM_STRUCT-NEXT:    black
+ENUM_STRUCT-NEXT:}

Modified: lldb/trunk/lit/SymbolFile/PDB/type-quals.test
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/type-quals.test?rev=350764&r1=350763&r2=350764&view=diff
==============================================================================
--- lldb/trunk/lit/SymbolFile/PDB/type-quals.test (original)
+++ lldb/trunk/lit/SymbolFile/PDB/type-quals.test Wed Jan  9 13:20:44 2019
@@ -5,35 +5,35 @@ RUN: lldb-test symbols %T/TypeQualsTest.
 
 CHECK: Module [[MOD:.*]]
 CHECK-DAG: {{^[0-9A-F]+}}: SymbolVendor ([[MOD]])
-CHECK:      Type{{.*}} , name = "const int", size = 4, compiler_type = {{.*}} const int
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} const int *
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} const int **const
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} const int *const
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} const int *const *
-CHECK:      Type{{.*}} , name = "Func1", {{.*}}, compiler_type = {{.*}} void (const int *, const int *, const int **const, const int *const *)
-
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} volatile int *
-CHECK:      Type{{.*}} , name = "Func2", {{.*}}, compiler_type = {{.*}} void (volatile int *, volatile int *)
-
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} int *
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} int *&
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} int &
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} const int &
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} int &&
-CHECK:      Type{{.*}} , name = "Func3", {{.*}}, compiler_type = {{.*}} void (int *&, int &, const int &, int &&)
+CHECK-DAG:      Type{{.*}} , name = "const int", size = 4, compiler_type = {{.*}} const int
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} const int *
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} const int **const
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} const int *const
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} const int *const *
+CHECK-DAG:      Type{{.*}} , name = "Func1", {{.*}}, compiler_type = {{.*}} void (const int *, const int *, const int **const, const int *const *)
+
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} volatile int *
+CHECK-DAG:      Type{{.*}} , name = "Func2", {{.*}}, compiler_type = {{.*}} void (volatile int *, volatile int *)
+
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} int *
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} int *&
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} int &&
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} int &
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} const int &
+CHECK-DAG:      Type{{.*}} , name = "Func3", {{.*}}, compiler_type = {{.*}} void (int *&, int &, const int &, int &&)
 
 // FIXME: __unaligned is not supported.
-CHECK:      Type{{.*}} , name = "Func4", {{.*}}, compiler_type = {{.*}} void (int *, int *)
+CHECK-DAG:      Type{{.*}} , name = "Func4", {{.*}}, compiler_type = {{.*}} void (int *, int *)
 
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} int *__restrict
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} int &__restrict
-CHECK:      Type{{.*}} , name = "Func5", {{.*}}, compiler_type = {{.*}} void (int, int *__restrict, int &__restrict)
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} int *__restrict
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} int &__restrict
+CHECK-DAG:      Type{{.*}} , name = "Func5", {{.*}}, compiler_type = {{.*}} void (int, int *__restrict, int &__restrict)
 
-CHECK:      Type{{.*}} , name = "Func6", {{.*}}, compiler_type = {{.*}} void (const volatile int *__restrict)
+CHECK-DAG:      Type{{.*}} , name = "Func6", {{.*}}, compiler_type = {{.*}} void (const volatile int *__restrict)
 
-CHECK:      Type{{.*}} , size = 400, compiler_type = {{.*}} volatile int *[100]
-CHECK:      Type{{.*}} , size = 4000, compiler_type = {{.*}} volatile int *[10][100]
+CHECK-DAG:      Type{{.*}} , size = 400, compiler_type = {{.*}} volatile int *[100]
+CHECK-DAG:      Type{{.*}} , size = 4000, compiler_type = {{.*}} volatile int *[10][100]
 
-CHECK:      Type{{.*}} , size = 4, compiler_type = {{.*}} long *__restrict
+CHECK-DAG:      Type{{.*}} , size = 4, compiler_type = {{.*}} long *__restrict
 
 CHECK-DAG: {{^[0-9A-F]+}}:   CompileUnit{{[{]0x[0-9a-f]+[}]}}, language = "c++", file = '{{.*}}\TypeQualsTest.cpp'

Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp?rev=350764&r1=350763&r2=350764&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Wed Jan  9 13:20:44 2019
@@ -314,6 +314,16 @@ lldb_private::Function *SymbolFilePDB::P
                                  func_type_uid, mangled, func_type, func_range);
 
   sc.comp_unit->AddFunction(func_sp);
+
+  TypeSystem *type_system = GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
+  if (!type_system)
+    return nullptr;
+  ClangASTContext *clang_type_system =
+    llvm::dyn_cast_or_null<ClangASTContext>(type_system);
+  if (!clang_type_system)
+    return nullptr;
+  clang_type_system->GetPDBParser()->GetDeclForSymbol(pdb_func);
+
   return func_sp.get();
 }
 
@@ -1035,6 +1045,9 @@ SymbolFilePDB::ParseVariables(const lldb
           if (variable_list)
             variable_list->AddVariableIfUnique(var_sp);
           ++num_added;
+          PDBASTParser *ast = GetPDBAstParser();
+          if (ast)
+            ast->GetDeclForSymbol(*pdb_data);
         }
       }
     }
@@ -1623,6 +1636,16 @@ SymbolFilePDB::GetTypeSystemForLanguage(
   return type_system;
 }
 
+PDBASTParser *SymbolFilePDB::GetPDBAstParser() {
+  auto type_system = GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
+  auto clang_type_system = llvm::dyn_cast_or_null<ClangASTContext>(type_system);
+  if (!clang_type_system)
+    return nullptr;
+
+  return clang_type_system->GetPDBParser();
+}
+
+
 lldb_private::CompilerDeclContext SymbolFilePDB::FindNamespace(
     const lldb_private::SymbolContext &sc,
     const lldb_private::ConstString &name,

Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h?rev=350764&r1=350763&r2=350764&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h Wed Jan  9 13:20:44 2019
@@ -20,6 +20,8 @@
 #include "llvm/DebugInfo/PDB/PDB.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 
+class PDBASTParser;
+
 class SymbolFilePDB : public lldb_private::SymbolFile {
 public:
   //------------------------------------------------------------------
@@ -224,6 +226,8 @@ private:
   void GetCompileUnitIndex(const llvm::pdb::PDBSymbolCompiland &pdb_compiland,
                            uint32_t &index);
 
+  PDBASTParser *GetPDBAstParser();
+
   std::unique_ptr<llvm::pdb::PDBSymbolCompiland>
   GetPDBCompilandByUID(uint32_t uid);
 

Modified: lldb/trunk/tools/lldb-test/lldb-test.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-test/lldb-test.cpp?rev=350764&r1=350763&r2=350764&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-test/lldb-test.cpp (original)
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp Wed Jan  9 13:20:44 2019
@@ -518,6 +518,7 @@ Error opts::symbols::dumpModule(lldb_pri
 
 Error opts::symbols::dumpAST(lldb_private::Module &Module) {
   SymbolVendor &plugin = *Module.GetSymbolVendor();
+   Module.ParseAllDebugSymbols();
 
   auto symfile = plugin.GetSymbolFile();
   if (!symfile)
@@ -536,9 +537,6 @@ Error opts::symbols::dumpAST(lldb_privat
   if (!tu)
     return make_string_error("Can't retrieve translation unit declaration.");
 
-  symfile->ParseDeclsForContext(CompilerDeclContext(
-      clang_ast_ctx, static_cast<clang::DeclContext *>(tu)));
-
   tu->print(outs());
 
   return Error::success();




More information about the lldb-commits mailing list