-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[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.
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
@llvm/pr-subscribers-debuginfo Author: Walnut (Walnut356) ChangesPart 1 of 2 in splitting #152484 In short: Local and global variables can only store their type via a type node. Typedefs currently only use 2 important notes:
Full diff: https://github.com/llvm/llvm-project/pull/153936.diff 13 Files Affected:
diff --git a/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def b/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def
index a31111eb80a4e..aa3beea75c75b 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def
+++ b/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def
@@ -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)
@@ -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)
diff --git a/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h b/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
index 5a84fac5f5903..0e739650bd089 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
@@ -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;
+
+};
+
} // end namespace codeview
} // end namespace llvm
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h
index eb6371e911be4..164ea990336e0 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h
@@ -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,
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index c5d6e40eb7c1e..29978c9e5270f 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -1728,6 +1728,9 @@ TypeIndex CodeViewDebug::lowerTypeAlias(const DIDerivedType *Ty) {
addToUDTs(Ty);
+ AliasRecord AR(UnderlyingTypeIndex, TypeName);
+ auto alias_index = TypeTable.writeLeafType(AR);
+
if (UnderlyingTypeIndex == TypeIndex(SimpleTypeKind::Int32Long) &&
TypeName == "HRESULT")
return TypeIndex(SimpleTypeKind::HResult);
@@ -1735,7 +1738,7 @@ TypeIndex CodeViewDebug::lowerTypeAlias(const DIDerivedType *Ty) {
TypeName == "wchar_t")
return TypeIndex(SimpleTypeKind::WideCharacter);
- return UnderlyingTypeIndex;
+ return alias_index;
}
TypeIndex CodeViewDebug::lowerTypeArray(const DICompositeType *Ty) {
@@ -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()) {
diff --git a/llvm/lib/DebugInfo/CodeView/RecordName.cpp b/llvm/lib/DebugInfo/CodeView/RecordName.cpp
index e06b036ede63a..80c9cfa2253fb 100644
--- a/llvm/lib/DebugInfo/CodeView/RecordName.cpp
+++ b/llvm/lib/DebugInfo/CodeView/RecordName.cpp
@@ -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);
diff --git a/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp b/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
index 776676410e782..629b7e5746ff4 100644
--- a/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
+++ b/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
@@ -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();
+}
diff --git a/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp b/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
index 0bc65f8d0359a..530a119de85fe 100644
--- a/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
+++ b/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
@@ -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();
+}
diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
index 24eaa1234727d..6cd53d42a43cd 100644
--- a/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
@@ -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)); });
diff --git a/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp b/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
index f4ca1b22eafa0..84f33a3fea6a8 100644
--- a/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
+++ b/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
@@ -614,6 +614,11 @@ template <> void LeafRecordImpl<EndPrecompRecord>::map(IO &IO) {
IO.mapRequired("Signature", Record.Signature);
}
+template <> void LeafRecordImpl<AliasRecord>::map(IO &IO) {
+ IO.mapRequired("UnderlyingType", Record.UnderlyingType);
+ IO.mapRequired("Name", Record.Name);
+}
+
template <> void MemberRecordImpl<OneMethodRecord>::map(IO &IO) {
MappingTraits<OneMethodRecord>::mapping(IO, Record);
}
diff --git a/llvm/test/DebugInfo/PDB/Inputs/typedef.cpp b/llvm/test/DebugInfo/PDB/Inputs/typedef.cpp
new file mode 100644
index 0000000000000..40220da3d9e05
--- /dev/null
+++ b/llvm/test/DebugInfo/PDB/Inputs/typedef.cpp
@@ -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;
+
+typedef unsigned char u8;
+using i64 = long long;
+
+int main() {
+ u8 val = 15;
+ i64 val2 = -1;
+
+ return 0;
+}
diff --git a/llvm/test/DebugInfo/PDB/Inputs/typedef.pdb b/llvm/test/DebugInfo/PDB/Inputs/typedef.pdb
new file mode 100644
index 0000000000000..3b9cf69aecff8
Binary files /dev/null and b/llvm/test/DebugInfo/PDB/Inputs/typedef.pdb differ
diff --git a/llvm/test/DebugInfo/PDB/typedef.test b/llvm/test/DebugInfo/PDB/typedef.test
new file mode 100644
index 0000000000000..43f4db1a186c4
--- /dev/null
+++ b/llvm/test/DebugInfo/PDB/typedef.test
@@ -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)
diff --git a/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp b/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
index db3a752d58165..b7caf167c3d61 100644
--- a/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
+++ b/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
@@ -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);
|
@llvm/pr-subscribers-objectyaml Author: Walnut (Walnut356) ChangesPart 1 of 2 in splitting #152484 In short: Local and global variables can only store their type via a type node. Typedefs currently only use 2 important notes:
Full diff: https://github.com/llvm/llvm-project/pull/153936.diff 13 Files Affected:
diff --git a/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def b/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def
index a31111eb80a4e..aa3beea75c75b 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def
+++ b/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def
@@ -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)
@@ -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)
diff --git a/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h b/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
index 5a84fac5f5903..0e739650bd089 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
@@ -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;
+
+};
+
} // end namespace codeview
} // end namespace llvm
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h
index eb6371e911be4..164ea990336e0 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h
@@ -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,
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index c5d6e40eb7c1e..29978c9e5270f 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -1728,6 +1728,9 @@ TypeIndex CodeViewDebug::lowerTypeAlias(const DIDerivedType *Ty) {
addToUDTs(Ty);
+ AliasRecord AR(UnderlyingTypeIndex, TypeName);
+ auto alias_index = TypeTable.writeLeafType(AR);
+
if (UnderlyingTypeIndex == TypeIndex(SimpleTypeKind::Int32Long) &&
TypeName == "HRESULT")
return TypeIndex(SimpleTypeKind::HResult);
@@ -1735,7 +1738,7 @@ TypeIndex CodeViewDebug::lowerTypeAlias(const DIDerivedType *Ty) {
TypeName == "wchar_t")
return TypeIndex(SimpleTypeKind::WideCharacter);
- return UnderlyingTypeIndex;
+ return alias_index;
}
TypeIndex CodeViewDebug::lowerTypeArray(const DICompositeType *Ty) {
@@ -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()) {
diff --git a/llvm/lib/DebugInfo/CodeView/RecordName.cpp b/llvm/lib/DebugInfo/CodeView/RecordName.cpp
index e06b036ede63a..80c9cfa2253fb 100644
--- a/llvm/lib/DebugInfo/CodeView/RecordName.cpp
+++ b/llvm/lib/DebugInfo/CodeView/RecordName.cpp
@@ -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);
diff --git a/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp b/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
index 776676410e782..629b7e5746ff4 100644
--- a/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
+++ b/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
@@ -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();
+}
diff --git a/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp b/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
index 0bc65f8d0359a..530a119de85fe 100644
--- a/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
+++ b/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
@@ -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();
+}
diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
index 24eaa1234727d..6cd53d42a43cd 100644
--- a/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
@@ -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)); });
diff --git a/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp b/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
index f4ca1b22eafa0..84f33a3fea6a8 100644
--- a/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
+++ b/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
@@ -614,6 +614,11 @@ template <> void LeafRecordImpl<EndPrecompRecord>::map(IO &IO) {
IO.mapRequired("Signature", Record.Signature);
}
+template <> void LeafRecordImpl<AliasRecord>::map(IO &IO) {
+ IO.mapRequired("UnderlyingType", Record.UnderlyingType);
+ IO.mapRequired("Name", Record.Name);
+}
+
template <> void MemberRecordImpl<OneMethodRecord>::map(IO &IO) {
MappingTraits<OneMethodRecord>::mapping(IO, Record);
}
diff --git a/llvm/test/DebugInfo/PDB/Inputs/typedef.cpp b/llvm/test/DebugInfo/PDB/Inputs/typedef.cpp
new file mode 100644
index 0000000000000..40220da3d9e05
--- /dev/null
+++ b/llvm/test/DebugInfo/PDB/Inputs/typedef.cpp
@@ -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;
+
+typedef unsigned char u8;
+using i64 = long long;
+
+int main() {
+ u8 val = 15;
+ i64 val2 = -1;
+
+ return 0;
+}
diff --git a/llvm/test/DebugInfo/PDB/Inputs/typedef.pdb b/llvm/test/DebugInfo/PDB/Inputs/typedef.pdb
new file mode 100644
index 0000000000000..3b9cf69aecff8
Binary files /dev/null and b/llvm/test/DebugInfo/PDB/Inputs/typedef.pdb differ
diff --git a/llvm/test/DebugInfo/PDB/typedef.test b/llvm/test/DebugInfo/PDB/typedef.test
new file mode 100644
index 0000000000000..43f4db1a186c4
--- /dev/null
+++ b/llvm/test/DebugInfo/PDB/typedef.test
@@ -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)
diff --git a/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp b/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
index db3a752d58165..b7caf167c3d61 100644
--- a/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
+++ b/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
@@ -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);
|
@llvm/pr-subscribers-platform-windows Author: Walnut (Walnut356) ChangesPart 1 of 2 in splitting #152484 In short: Local and global variables can only store their type via a type node. Typedefs currently only use 2 important notes:
Full diff: https://github.com/llvm/llvm-project/pull/153936.diff 13 Files Affected:
diff --git a/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def b/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def
index a31111eb80a4e..aa3beea75c75b 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def
+++ b/llvm/include/llvm/DebugInfo/CodeView/CodeViewTypes.def
@@ -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)
@@ -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)
diff --git a/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h b/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
index 5a84fac5f5903..0e739650bd089 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
@@ -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;
+
+};
+
} // end namespace codeview
} // end namespace llvm
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h
index eb6371e911be4..164ea990336e0 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h
@@ -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,
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index c5d6e40eb7c1e..29978c9e5270f 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -1728,6 +1728,9 @@ TypeIndex CodeViewDebug::lowerTypeAlias(const DIDerivedType *Ty) {
addToUDTs(Ty);
+ AliasRecord AR(UnderlyingTypeIndex, TypeName);
+ auto alias_index = TypeTable.writeLeafType(AR);
+
if (UnderlyingTypeIndex == TypeIndex(SimpleTypeKind::Int32Long) &&
TypeName == "HRESULT")
return TypeIndex(SimpleTypeKind::HResult);
@@ -1735,7 +1738,7 @@ TypeIndex CodeViewDebug::lowerTypeAlias(const DIDerivedType *Ty) {
TypeName == "wchar_t")
return TypeIndex(SimpleTypeKind::WideCharacter);
- return UnderlyingTypeIndex;
+ return alias_index;
}
TypeIndex CodeViewDebug::lowerTypeArray(const DICompositeType *Ty) {
@@ -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()) {
diff --git a/llvm/lib/DebugInfo/CodeView/RecordName.cpp b/llvm/lib/DebugInfo/CodeView/RecordName.cpp
index e06b036ede63a..80c9cfa2253fb 100644
--- a/llvm/lib/DebugInfo/CodeView/RecordName.cpp
+++ b/llvm/lib/DebugInfo/CodeView/RecordName.cpp
@@ -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);
diff --git a/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp b/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
index 776676410e782..629b7e5746ff4 100644
--- a/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
+++ b/llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
@@ -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();
+}
diff --git a/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp b/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
index 0bc65f8d0359a..530a119de85fe 100644
--- a/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
+++ b/llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
@@ -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();
+}
diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
index 24eaa1234727d..6cd53d42a43cd 100644
--- a/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
@@ -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)); });
diff --git a/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp b/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
index f4ca1b22eafa0..84f33a3fea6a8 100644
--- a/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
+++ b/llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
@@ -614,6 +614,11 @@ template <> void LeafRecordImpl<EndPrecompRecord>::map(IO &IO) {
IO.mapRequired("Signature", Record.Signature);
}
+template <> void LeafRecordImpl<AliasRecord>::map(IO &IO) {
+ IO.mapRequired("UnderlyingType", Record.UnderlyingType);
+ IO.mapRequired("Name", Record.Name);
+}
+
template <> void MemberRecordImpl<OneMethodRecord>::map(IO &IO) {
MappingTraits<OneMethodRecord>::mapping(IO, Record);
}
diff --git a/llvm/test/DebugInfo/PDB/Inputs/typedef.cpp b/llvm/test/DebugInfo/PDB/Inputs/typedef.cpp
new file mode 100644
index 0000000000000..40220da3d9e05
--- /dev/null
+++ b/llvm/test/DebugInfo/PDB/Inputs/typedef.cpp
@@ -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;
+
+typedef unsigned char u8;
+using i64 = long long;
+
+int main() {
+ u8 val = 15;
+ i64 val2 = -1;
+
+ return 0;
+}
diff --git a/llvm/test/DebugInfo/PDB/Inputs/typedef.pdb b/llvm/test/DebugInfo/PDB/Inputs/typedef.pdb
new file mode 100644
index 0000000000000..3b9cf69aecff8
Binary files /dev/null and b/llvm/test/DebugInfo/PDB/Inputs/typedef.pdb differ
diff --git a/llvm/test/DebugInfo/PDB/typedef.test b/llvm/test/DebugInfo/PDB/typedef.test
new file mode 100644
index 0000000000000..43f4db1a186c4
--- /dev/null
+++ b/llvm/test/DebugInfo/PDB/typedef.test
@@ -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)
diff --git a/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp b/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
index db3a752d58165..b7caf167c3d61 100644
--- a/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
+++ b/llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
@@ -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);
|
// 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; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@@ -614,6 +614,11 @@ template <> void LeafRecordImpl<EndPrecompRecord>::map(IO &IO) { | |||
IO.mapRequired("Signature", Record.Signature); | |||
} | |||
|
|||
template <> void LeafRecordImpl<AliasRecord>::map(IO &IO) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we test this?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the surface seems simple enough, so lgtm. Don't know much about PDB internals though, so will defer approval to someone more familiar with it
|
||
TypeIndex UnderlyingType; | ||
StringRef Name; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove extra blank line
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
llvm/test/DebugInfo/PDB/typedef.test
Outdated
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the change! I don't have MSVC in front of me at the moment to interrogate when it uses LF_ALIAS records to check the corner cases, but I assume this is what we want to do. I worry about the possibility of ODR violations in cases like:
template <typename T> struct Foo { T x; };
int main() {
typedef int myint_t;
Foo<int> v1;
Foo<myint_t> v2; // Hopefully we only have one Foo<int> instantiation in the debug info...
}
Does Clang handle the case I have in mind by generating IR in such a way that the Foo composite type records refer to the canonical int type, or the typedef?
// 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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
llvm/test/DebugInfo/PDB/typedef.test
Outdated
There was a problem hiding this comment.
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.
@@ -614,6 +614,11 @@ template <> void LeafRecordImpl<EndPrecompRecord>::map(IO &IO) { | |||
IO.mapRequired("Signature", Record.Signature); | |||
} | |||
|
|||
template <> void LeafRecordImpl<AliasRecord>::map(IO &IO) { |
There was a problem hiding this comment.
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.
Here is a gist containing the llvm IR and pdb info. Looks like there's only 1 struct definition and the typedef is completely ignored. It also doesn't appear in the debug info. I also tried assigning Explicitly designating the type |
Thanks, I think that fully addresses any functional concerns I have. |
@@ -1728,14 +1728,17 @@ TypeIndex CodeViewDebug::lowerTypeAlias(const DIDerivedType *Ty) { | |||
|
|||
addToUDTs(Ty); | |||
|
|||
AliasRecord AR(UnderlyingTypeIndex, TypeName); | |||
auto alias_index = TypeTable.writeLeafType(AR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to AliasIndex
.
There was a problem hiding this comment.
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 ...
There was a problem hiding this comment.
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 theLF_ALIAS
node
// 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; |
There was a problem hiding this comment.
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.
Part 1 of 2 in splitting #152484
In short: Local and global variables can only store their type via a type node. Typedefs currently only use
S_UDT
, which is a symbol node. This patch makes the compiler outputlf_alias
nodes (equivalent to DWARF'sDW_TAG_typedef
), which exist in the type stream. This allows debuggers to tell the difference betweenuint8_t var;
andunsigned char var;
.2 important notes:
lf_alias
and native does not. The followup patch will addlf_alias
support to the native parser.