Skip to content

[LLVM] Create lf_alias nodes for typedef and using #153936

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[LLVM] Create lf_alias nodes for typedef and using #153936

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0e257f7
create `lf_alias` nodes for `typedef` and `using`
Walnut356 Aug 16, 2025
8afe1db
swap test data from pdb to yaml
Walnut356 Aug 21, 2025
688bacc
test node generation from llvm-ir
Walnut356 Aug 30, 2025
abae78c
remove extraneous tags
Walnut356 Sep 11, 2025
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
2 changes: 1 addition & 1 deletion llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ TYPE_RECORD(LF_ENUM, 0x1507, Enum)
TYPE_RECORD(LF_TYPESERVER2, 0x1515, TypeServer2)
TYPE_RECORD(LF_VFTABLE, 0x151d, VFTable)
TYPE_RECORD(LF_VTSHAPE, 0x000a, VFTableShape)
TYPE_RECORD(LF_ALIAS, 0x150a, Alias)

TYPE_RECORD(LF_BITFIELD, 0x1205, BitField)

Expand Down Expand Up @@ -181,7 +182,6 @@ CV_TYPE(LF_MANAGED_ST, 0x140f)
CV_TYPE(LF_ST_MAX, 0x1500)
CV_TYPE(LF_TYPESERVER, 0x1501)
CV_TYPE(LF_DIMARRAY, 0x1508)
CV_TYPE(LF_ALIAS, 0x150a)
CV_TYPE(LF_DEFARG, 0x150b)
CV_TYPE(LF_FRIENDFCN, 0x150c)
CV_TYPE(LF_NESTTYPEEX, 0x1512)
Expand Down
13 changes: 13 additions & 0 deletions llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -952,6 +952,19 @@ class EndPrecompRecord : public TypeRecord {
uint32_t Signature = 0;
};

// LF_ALIAS
class AliasRecord : public TypeRecord {
public:
AliasRecord() = default;
explicit AliasRecord(TypeRecordKind Kind) : TypeRecord(Kind) {}
AliasRecord(TypeIndex UnderlyingType, StringRef Name)
: TypeRecord(TypeRecordKind::Alias), UnderlyingType(UnderlyingType), Name(Name) {}

TypeIndex UnderlyingType;
StringRef Name;

Copy link
Member

Choose a reason for hiding this comment

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

Remove extra blank line

};

} // end namespace codeview
} // end namespace llvm

Expand Down
2 changes: 2 additions & 0 deletions llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,8 @@ class LVLogicalVisitor final {
LVElement *Element);
Error visitKnownRecord(CVType &Record, EndPrecompRecord &EndPrecomp,
TypeIndex TI, LVElement *Element);
Error visitKnownRecord(CVType &Record, AliasRecord &Alias,
TypeIndex TI, LVElement *Element);

Error visitUnknownMember(CVMemberRecord &Record, TypeIndex TI);
Error visitKnownMember(CVMemberRecord &Record, BaseClassRecord &Base,
Expand Down
13 changes: 4 additions & 9 deletions llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1728,14 +1728,17 @@ TypeIndex CodeViewDebug::lowerTypeAlias(const DIDerivedType *Ty) {

addToUDTs(Ty);

AliasRecord AR(UnderlyingTypeIndex, TypeName);
auto alias_index = TypeTable.writeLeafType(AR);
Copy link
Member

Choose a reason for hiding this comment

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

Change to AliasIndex.

Copy link
Member

Choose a reason for hiding this comment

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

The tests below do not cover this change, the fact that we properly generate LF_ALIAS when building from Clang or LLVM-IR. As @Michael137 says, you can generate that as a test. I suppose something like that would work:

REM ---- This part is generated by you offline
echo typedef unsigned char u8; int main() { u8 a = 1; } > typedef.cpp
clang-cl /c /FA /Z7 typedef.cpp
(chop down non-relevant parts of typedef.asm to your needs)

REM ---- This is part of the LLVM test
llvm-mc typedef.asm --filetype=obj -o typedef.obj
llvm-pdbutil dump -types typedef.obj | FileCheck ...

Copy link
Author

@Walnut356 Walnut356 Aug 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Should be taken care of now. I used llvm-ir and llc since the .ll file ended up being about 3-4x smaller than the equivalent .asm (3kb vs 14kb) The additional checks ensure:

  • LF_ALIAS is generated and references the correct underlying type
  • variables reference the LF_ALIAS type instead of the underlying type
  • the S_UDT node is still generated and points to the LF_ALIAS node


if (UnderlyingTypeIndex == TypeIndex(SimpleTypeKind::Int32Long) &&
TypeName == "HRESULT")
return TypeIndex(SimpleTypeKind::HResult);
if (UnderlyingTypeIndex == TypeIndex(SimpleTypeKind::UInt16Short) &&
TypeName == "wchar_t")
return TypeIndex(SimpleTypeKind::WideCharacter);

return UnderlyingTypeIndex;
return alias_index;
}

TypeIndex CodeViewDebug::lowerTypeArray(const DICompositeType *Ty) {
Expand Down Expand Up @@ -2750,14 +2753,6 @@ TypeIndex CodeViewDebug::getCompleteTypeIndex(const DIType *Ty) {
if (!Ty)
return TypeIndex::Void();

// Look through typedefs when getting the complete type index. Call
// getTypeIndex on the typdef to ensure that any UDTs are accumulated and are
// emitted only once.
if (Ty->getTag() == dwarf::DW_TAG_typedef)
(void)getTypeIndex(Ty);
while (Ty->getTag() == dwarf::DW_TAG_typedef)
Ty = cast<DIDerivedType>(Ty)->getBaseType();

// If this is a non-record type, the complete type index is the same as the
// normal type index. Just call getTypeIndex.
switch (Ty->getTag()) {
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/DebugInfo/CodeView/RecordName.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,11 @@ Error TypeNameComputer::visitKnownRecord(CVType &CVR,
return Error::success();
}

Error TypeNameComputer::visitKnownRecord(CVType &CVR, AliasRecord &Alias) {
Name = Alias.Name;
return Error::success();
}

std::string llvm::codeview::computeTypeName(TypeCollection &Types,
TypeIndex Index) {
TypeNameComputer Computer(Types);
Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,3 +568,9 @@ Error TypeDumpVisitor::visitKnownRecord(CVType &CVR,
W->printHex("Signature", EndPrecomp.getSignature());
return Error::success();
}

Error TypeDumpVisitor::visitKnownRecord(CVType &CVR, AliasRecord &Alias) {
printTypeIndex("UnderlyingType", Alias.UnderlyingType);
W->printString("Name", Alias.Name);
return Error::success();
}
7 changes: 7 additions & 0 deletions llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,3 +752,10 @@ Error TypeRecordMapping::visitKnownRecord(CVType &CVR,
error(IO.mapInteger(EndPrecomp.Signature, "Signature"));
return Error::success();
}

Error TypeRecordMapping::visitKnownRecord(CVType &CVR, AliasRecord &Alias) {
error(IO.mapInteger(Alias.UnderlyingType, "UnderlyingType"));
error(IO.mapStringZ(Alias.Name, "Name"));

return Error::success();
}
12 changes: 12 additions & 0 deletions llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2623,6 +2623,18 @@ Error LVLogicalVisitor::visitKnownRecord(CVType &Record,
return Error::success();
}

// LF_ALIAS (TPI)
Error LVLogicalVisitor::visitKnownRecord(CVType &Record, AliasRecord &Alias,
TypeIndex TI, LVElement *Element) {
LLVM_DEBUG({
printTypeBegin(Record, TI, Element, StreamTPI);
printTypeIndex("UnderlyingType", Alias.UnderlyingType, StreamTPI);
W.printString("Name", Alias.Name);
printTypeEnd(Record);
});
return Error::success();
}

Error LVLogicalVisitor::visitUnknownMember(CVMemberRecord &Record,
TypeIndex TI) {
LLVM_DEBUG({ W.printHex("UnknownMember", unsigned(Record.Kind)); });
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,11 @@ template <> void LeafRecordImpl<EndPrecompRecord>::map(IO &IO) {
IO.mapRequired("Signature", Record.Signature);
}

template <> void LeafRecordImpl<AliasRecord>::map(IO &IO) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we test this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe so, I would dig around in the yaml tests for an example to add to.

IO.mapRequired("UnderlyingType", Record.UnderlyingType);
IO.mapRequired("Name", Record.Name);
}

template <> void MemberRecordImpl<OneMethodRecord>::map(IO &IO) {
MappingTraits<OneMethodRecord>::mapping(IO, Record);
}
Expand Down
14 changes: 14 additions & 0 deletions llvm/test/DebugInfo/PDB/Inputs/typedef.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Build with clang -fno-rtti -g -O0 typedef.cpp
// Built with clang (22+) because MSVC does not output lf_alias for typedefs

void *__purecall = 0;
Copy link
Member

@Michael137 Michael137 Aug 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why not compile this file (or the IR of this) as part of the test? After all, this is testing that LLVM correctly emits LF_ALIAS right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

LLVM tests cannot depend on Clang, they have to be able to run in isolation.

Copy link
Member

Choose a reason for hiding this comment

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

Please remove this file, and then you could write - as comments - a minimal version in typedef.test below, including the explict steps to generate the input, as suggested above. There's two parts to this: testing the serialization like you do below; and testing the lowering from IR/.asm as I suggested.


typedef unsigned char u8;
using i64 = long long;

int main() {
u8 val = 15;
i64 val2 = -1;

return 0;
}
Binary file added llvm/test/DebugInfo/PDB/Inputs/typedef.pdb
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Please remove binary PDB. We usually don’t add binaries of any sort to the LLVM repo, unless we cannot generate them by our tooling.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You should be able to use the yaml2pdb tool to commit a textual YAML representation of the PDB, but this file is 6.8MB, so it probably needs to be further minimized, or we should reconsider the test structure more deeply.

Binary file not shown.
8 changes: 8 additions & 0 deletions llvm/test/DebugInfo/PDB/typedef.test
View file
Open in desktop
Copy link
Member

@aganea aganea Aug 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

As @Michael137 says above, the whole test should be self-enclosed. The test, the inputs and the post-checks should all live in this file. Please look at past commits to understand how other LF_* records were implemented / tested.

Copy link
Author

@Walnut356 Walnut356 Aug 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not super familiar with LLVM's test structure, but I more or less copied the surrounding tests. All of the native pdb tests use pre-compiled PDB files and they seem to all be testing "can LLVM's native reader read the node", not "can LLVM generate the node". They're compiled with cl.exe, but afaict they're not outputting anything that LLVM can't. The only reason this one is compiled with clang instead of cl is because I can't coerce cl to output the proper node, but that's largely incidental to "can LLVM's native reader read this node?"

Do the tests for LLVM's node generation live somewhere else, and would that test be more appropriately placed there?

Copy link
Member

Choose a reason for hiding this comment

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

Yea that's fair. Though off the top I don't see a good reason why it was done this way for the other tests. Perhaps the test infrastructure wasn't mature enough? @rnk will probably know

I'm not a PDB/CodeView expert, so I might be misunderstanding what's required to test these changes. But does Clang generate PDBs when you specify -gcodeview? If so, we have tests (e.g., search llvm/test/DebugInfo/COFF for -gcodeview) that generate and then inspect CodeView. Would that be sufficient to test your changes?

Copy link
Member

@aganea aganea Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@Walnut356 We usually cover "can LLVM read the node" and "can LLVM generate the node" at once. There are some rare cases, such as .pch.obj which clang-cl doesn't generate, but can read, which requires binaries generated by cl.exe. There are some other buggy inputs cases, for which we keep binaries. In most other cases, we can just use yaml2pdb or yaml2obj or vice-versa, obj2yaml or llvm-pdbutil for dumping the output of an existing .obj/.pdb. llvm-readobj also is able to dump some Codeview debug info.

An example of this: https://github.com/llvm/llvm-project/blob/main/llvm/test/DebugInfo/PDB/obj-globalhash.test

Another example which uses llvm-readobj here: https://github.com/llvm/llvm-project/blob/main/llvm/test/DebugInfo/COFF/lambda.ll

There might be some legacy binaries lying around but ideally if LLVM can generate the LF_ record, we can use the tooling above and shouldn't be adding binaries to the repo.

As for location, the tests should go in https://github.com/llvm/llvm-project/tree/main/llvm/test/DebugInfo

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1, right, we mostly need(ed) the binaries to bootstrap file parsing. Now that the tools can read and write these records, the cost of additional binary test inputs outweighs the increase in confidence we get from validating that our tools can read one additional codeview record from MSVC outputs.

Another way of looking at this is that we committed binary files as test inputs in a pre-xz-attack world, and now binary files are one of the biggest negatives on the LLVM project OpenSSF security scorecard:
https://securityscorecards.dev/viewer/?uri=github.com/llvm/llvm-project

So, things have actually changed, standards have increased, and what worked yesterday won't fly today. 😦

I want to unblock and enable contributions, but I don't want to make the problem worse at this point. I'm sure this feels unfair, but I hope that context helps explain the shift. In an ideal world, we'd do the work to delete the binary inputs by YAML-ifying the parts of the tests that we need to and replacing the rest using other strategies.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
RUN: llvm-pdbutil dump -type-index=0x348A,0x348C %p/Inputs/typedef.pdb \
RUN: | FileCheck --check-prefix=TYPES %s

TYPES: Types (TPI Stream)
TYPES-NEXT:============================================================
TYPES-NEXT: Showing 2 records.
TYPES-NEXT: 0x348A | LF_ALIAS [size = 12] alias = u8, underlying type = 0x0020 (unsigned char)
TYPES-NEXT: 0x348C | LF_ALIAS [size = 12] alias = i64, underlying type = 0x0013 (__int64)
7 changes: 7 additions & 0 deletions llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,13 @@ Error MinimalTypeDumpVisitor::visitKnownRecord(CVType &CVR,
return Error::success();
}

Error MinimalTypeDumpVisitor::visitKnownRecord(CVType &CVT,
AliasRecord &Alias) {
P.format(" alias = {0}, underlying type = {1}", Alias.Name,
Alias.UnderlyingType);
return Error::success();
}

Error MinimalTypeDumpVisitor::visitKnownMember(CVMemberRecord &CVR,
NestedTypeRecord &Nested) {
P.format(" [name = `{0}`, parent = {1}]", Nested.Name, Nested.Type);
Expand Down