Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2141,14 +2141,17 @@ TypeSP SymbolFileNativePDB::CreateTypedef(PdbGlobalSymId id) {
if (!ts)
return nullptr;

ts->GetNativePDBParser()->GetOrCreateTypedefDecl(id);
auto *typedef_decl = ts->GetNativePDBParser()->GetOrCreateTypedefDecl(id);

CompilerType ct = target_type->GetForwardCompilerType();
if (auto *clang = llvm::dyn_cast_or_null<TypeSystemClang>(ts.get()))
ct = clang->GetType(clang->getASTContext().getTypeDeclType(typedef_decl));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a bit wrong, because PdbAstParser::GetOrCreateTypedefDecl already knows this compiler type. However, it returns a decl, so we need to go back through the AST to resolve the type.
The method isn't used elsewhere, so the return type could be changed to a CompilerType.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What use does GetOrCreateType have here? If that's not the thing that creates the correct TypedefType? Or in other words, could you elaborate on how this fixes the issue?

Is it that GetOrCreateTypedefDecl creates a new decl, but the associated TypedefType is different from the type created by GetOrCreateType? Does that mean we create two different typedefs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What use does GetOrCreateType have here? If that's not the thing that creates the correct TypedefType? Or in other words, could you elaborate on how this fixes the issue?

Ah, I left out the confusing/unintuitive part:

Currently, in the PDBs produced by both MSVC and LLVM, type aliases are not in the "type system" (TPI stream/LF_* records; https://redirect.github.com/llvm/llvm-project/pull/153936 attempts to change this in LLVM).

GetOrCreateType looks in the PDB type stream TPI. The typedefs we currently have are from S_UDT records in the globals stream, so they're represented as "symbols" and GetOrCreateType would not be able to create the type.
Because of this, they're handled differently.


Does that mean we create two different typedefs?

No, clang::ASTContext::getTypeDeclType will use TypeForDecl to get back to the type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, thanks for the context

The typedefs we currently have are from S_UDT records in the globals stream, so they're represented as "symbols" and GetOrCreateType would not be able to create the type

So what does GetOrCreateType achieve in this case? If it's not able to create the TypedefType.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what does GetOrCreateType achieve in this case? If it's not able to create the TypedefType.

Oh, I see what you mean. The GetOrCreateType(udt.Type) above creates the typedef'd type. So for using Foo = Bar, it would get/create Bar.


Declaration decl;
return MakeType(toOpaqueUid(id), ConstString(udt.Name),
llvm::expectedToOptional(target_type->GetByteSize(nullptr)),
nullptr, target_type->GetID(),
lldb_private::Type::eEncodingIsTypedefUID, decl,
target_type->GetForwardCompilerType(),
lldb_private::Type::eEncodingIsTypedefUID, decl, ct,
lldb_private::Type::ResolveState::Forward);
}

Expand Down
127 changes: 127 additions & 0 deletions lldb/test/Shell/SymbolFile/NativePDB/simple-types.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
// REQUIRES: lld

// Test that simple types can be found
// RUN: %build --std=c++20 --nodefaultlib --compiler=clang-cl --arch=64 -o %t.exe -- %s
// RUN: lldb-test symbols %t.exe | FileCheck %s
// RUN: lldb-test symbols %t.exe | FileCheck --check-prefix=FUNC-PARAMS %s

bool *PB;
bool &RB = *PB;
bool *&RPB = PB;
const bool &CRB = RB;
bool *const BC = 0;
const bool *const CBC = 0;

long AL[2];

const volatile short CVS = 0;
const short CS = 0;
volatile short VS;

struct ReturnedStruct1 {};
struct ReturnedStruct2 {};

struct MyStruct {
static ReturnedStruct1 static_fn(char *) { return {}; }
ReturnedStruct2 const_member_fn(char *) const { return {}; }
void volatile_member_fn() volatile {};
void member_fn() {};
};

void (*PF)(int, bool *, const float, double, ...);

using Func = void(char16_t, MyStruct &);
Func *PF2;

using SomeTypedef = long;
SomeTypedef ST;

int main() {
bool b;
char c;
unsigned char uc;
char8_t c8;

short s;
unsigned short us;
wchar_t wc;
char16_t c16;

int i;
unsigned int ui;
long l;
unsigned long ul;
char32_t c32;

long long ll;
unsigned long long ull;

MyStruct my_struct;

decltype(nullptr) np;
}

// CHECK-DAG: Type{{.*}} , name = "std::nullptr_t", size = 0, compiler_type = 0x{{[0-9a-f]+}} nullptr_t
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, the size of this type is 0. This is explicitly set in the code. The DIA plugin doesn't set the size to 0 if types are unsized (e.g. the function types below).

I'm not sure if it's bad that the native plugin sets this to 0. When calling Type::GetByteSize, the size is set to 0 anyway (because the compiler type would return 0 there).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, the DWARF plugin doesn't set the size explicitly to 0, in which case Type::Dump omits it:

0xaacd26800:   Type{0x0000046b} , name = "decltype(nullptr)", compiler_type = 0x0000000aad18ef40 nullptr_t

Can we do the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can do this in a followup PR.


// CHECK-DAG: Type{{.*}} , name = "bool", size = 1, compiler_type = 0x{{[0-9a-f]+}} _Bool
// CHECK-DAG: Type{{.*}} , name = "char", size = 1, compiler_type = 0x{{[0-9a-f]+}} char
// CHECK-DAG: Type{{.*}} , name = "unsigned char", size = 1, compiler_type = 0x{{[0-9a-f]+}} unsigned char
// CHECK-DAG: Type{{.*}} , name = "char8_t", size = 1, compiler_type = 0x{{[0-9a-f]+}} char8_t

// CHECK-DAG: Type{{.*}} , size = 2, compiler_type = 0x{{[0-9a-f]+}} short
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing: short doesn't have a name.

// CHECK-DAG: Type{{.*}} , name = "const volatile ", size = 2, compiler_type = 0x{{[0-9a-f]+}} const volatile short
// CHECK-DAG: Type{{.*}} , name = "const ", size = 2, compiler_type = 0x{{[0-9a-f]+}} const short
// CHECK-DAG: Type{{.*}} , name = "volatile ", size = 2, compiler_type = 0x{{[0-9a-f]+}} volatile short

// CHECK-DAG: Type{{.*}} , name = "unsigned short", size = 2, compiler_type = 0x{{[0-9a-f]+}} unsigned short
// CHECK-DAG: Type{{.*}} , name = "wchar_t", size = 2, compiler_type = 0x{{[0-9a-f]+}} wchar_t
// CHECK-DAG: Type{{.*}} , name = "char16_t", size = 2, compiler_type = 0x{{[0-9a-f]+}} char16_t

// CHECK-DAG: Type{{.*}} , name = "int", size = 4, compiler_type = 0x{{[0-9a-f]+}} int
// CHECK-DAG: Type{{.*}} , name = "unsigned", size = 4, compiler_type = 0x{{[0-9a-f]+}} unsigned int
// CHECK-DAG: Type{{.*}} , name = "long", size = 4, compiler_type = 0x{{[0-9a-f]+}} long
// CHECK-DAG: Type{{.*}} , name = "unsigned long", size = 4, compiler_type = 0x{{[0-9a-f]+}} unsigned long
// CHECK-DAG: Type{{.*}} , name = "char32_t", size = 4, compiler_type = 0x{{[0-9a-f]+}} char32_t

// CHECK-DAG: Type{{.*}} , name = "int64_t", size = 8, compiler_type = 0x{{[0-9a-f]+}} long long
// CHECK-DAG: Type{{.*}} , name = "uint64_t", size = 8, compiler_type = 0x{{[0-9a-f]+}} unsigned long long
Comment on lines +86 to +87
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing/incompatible: (unsigned)? long long will generate types named after the typedefs in stdint.h. They should probably use the same name as the compiler type.


// CHECK-DAG: Type{{.*}} , name = "float", size = 4, compiler_type = 0x{{[0-9a-f]+}} float
// CHECK-DAG: Type{{.*}} , name = "const float", size = 4, compiler_type = 0x{{[0-9a-f]+}} const float

// CHECK-DAG: Type{{.*}} , name = "ReturnedStruct1", size = 1, decl = simple-types.cpp:21, compiler_type = 0x{{[0-9a-f]+}} struct ReturnedStruct1 {
// CHECK-DAG: Type{{.*}} , name = "ReturnedStruct2", size = 1, decl = simple-types.cpp:22, compiler_type = 0x{{[0-9a-f]+}} struct ReturnedStruct2 {
// CHECK-DAG: Type{{.*}} , name = "MyStruct", size = 1, decl = simple-types.cpp:24, compiler_type = 0x{{[0-9a-f]+}} struct MyStruct {

// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} struct MyStruct *const
// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} const struct MyStruct *const
// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} volatile struct MyStruct *const
// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} struct MyStruct &

// CHECK-DAG: Type{{.*}} , name = "const bool", size = 1, compiler_type = 0x{{[0-9a-f]+}} const _Bool
// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} _Bool &
// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} _Bool *
// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} _Bool *&
// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} const _Bool &
// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} _Bool *const
// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} const _Bool *const

// CHECK-DAG: Type{{.*}} , name = "SomeTypedef", size = 4, compiler_type = 0x{{[0-9a-f]+}} typedef SomeTypedef
// CHECK-DAG: Type{{.*}} , name = "Func", size = 0, compiler_type = 0x{{[0-9a-f]+}} typedef Func

// CHECK-DAG: Type{{.*}} , size = 0, compiler_type = 0x{{[0-9a-f]+}} int (void)
// CHECK-DAG: Type{{.*}} , size = 0, compiler_type = 0x{{[0-9a-f]+}} void (void)

// CHECK-DAG: Type{{.*}} , size = 0, compiler_type = 0x{{[0-9a-f]+}} void (int, _Bool *, const float, double, ...)
// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} void (*)(int, _Bool *, const float, double, ...)

// CHECK-DAG: Type{{.*}} , size = 0, compiler_type = 0x{{[0-9a-f]+}} void (char16_t, struct MyStruct &)
// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} void (*)(char16_t, struct MyStruct &)

// CHECK-DAG: Type{{.*}} , size = 0, compiler_type = 0x{{[0-9a-f]+}} struct ReturnedStruct1 (char *)
// CHECK-DAG: Type{{.*}} , size = 0, compiler_type = 0x{{[0-9a-f]+}} struct ReturnedStruct2 (char *)

// CHECK-DAG: Type{{.*}} , size = 8, compiler_type = 0x{{[0-9a-f]+}} long[2]

// double is used as a parameter to `PF`, but not created as an LLDB type
// FUNC-PARAMS-NOT: Type{{.*}} , name = "double"
Loading