-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang][DebugInfo] Mark _BitInt's as reconstitutable when emitting -gsimple-template-names #168383
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
[clang][DebugInfo] Mark _BitInt's as reconstitutable when emitting -gsimple-template-names #168383
Conversation
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-clang-codegen Author: Michael Buch (Michael137) ChangesDepends on: As of recent, LLVM includes the bit-size as a Full diff: https://github.com/llvm/llvm-project/pull/168383.diff 4 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index bda7b7487f59b..4eb99cc342275 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -5863,15 +5863,6 @@ struct ReconstitutableType : public RecursiveASTVisitor<ReconstitutableType> {
Reconstitutable = false;
return false;
}
- bool VisitType(Type *T) {
- // _BitInt(N) isn't reconstitutable because the bit width isn't encoded in
- // the DWARF, only the byte width.
- if (T->isBitIntType()) {
- Reconstitutable = false;
- return false;
- }
- return true;
- }
bool TraverseEnumType(EnumType *ET, bool = false) {
// Unnamed enums can't be reconstituted due to a lack of column info we
// produce in the DWARF, so we can't get Clang's full name back.
diff --git a/clang/test/DebugInfo/CXX/simple-template-names.cpp b/clang/test/DebugInfo/CXX/simple-template-names.cpp
index 5a5d706e81972..3c8ff2780b66a 100644
--- a/clang/test/DebugInfo/CXX/simple-template-names.cpp
+++ b/clang/test/DebugInfo/CXX/simple-template-names.cpp
@@ -114,12 +114,28 @@ void f() {
f3<t1>();
// CHECK: !DISubprogram(name: "_STN|f3|<t1>",
-
+
f1<_BitInt(3)>();
- // CHECK: !DISubprogram(name: "f1<_BitInt(3)>",
+ // CHECK: !DISubprogram(name: "_STN|f1|<_BitInt(3)>",
f1<const unsigned _BitInt(5)>();
- // CHECK: !DISubprogram(name: "f1<const unsigned _BitInt(5)>",
+ // CHECK: !DISubprogram(name: "_STN|f1|<const unsigned _BitInt(5)>",
+
+ f1<_BitInt(120)>();
+ // CHECK: !DISubprogram(name: "_STN|f1|<_BitInt(120)>",
+
+ f1<const unsigned _BitInt(120)>();
+ // CHECK: !DISubprogram(name: "_STN|f1|<const unsigned _BitInt(120)>",
+
+ f2<_BitInt(2), 1>();
+ // CHECK: !DISubprogram(name: "_STN|f2|<_BitInt(2), (_BitInt(2))1>",
+
+ f2<_BitInt(64), 12>();
+ // CHECK: !DISubprogram(name: "_STN|f2|<_BitInt(64), (_BitInt(64))12>",
+
+ // FIXME: make block forms reconstitutable
+ f2<_BitInt(65), 1>();
+// CHECK: !DISubprogram(name: "f2<_BitInt(65), (_BitInt(65))1>",
// Add a parameter just so this differs from other attributed function types
// that don't mangle differently.
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
index a760f773055d2..9f6bcfb873b4d 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
@@ -13,6 +13,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Dwarf.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
#include <string>
@@ -78,6 +79,12 @@ template <typename DieType> struct DWARFTypePrinter {
}
return false;
}
+
+ /// If FormValue is a valid constant Form, print into \c OS the integral value
+ /// casted to the type referred to by \c Cast.
+ template <typename FormValueType>
+ void appendCastedValue(const FormValueType &FormValue, DieType Cast,
+ bool IsUnsigned);
};
template <typename DieType>
@@ -413,6 +420,31 @@ DieType DWARFTypePrinter<DieType>::appendQualifiedNameBefore(DieType D) {
return appendUnqualifiedNameBefore(D);
}
+template <typename DieType>
+template <typename FormValueType>
+void DWARFTypePrinter<DieType>::appendCastedValue(
+ const FormValueType &FormValue, DieType Cast, bool IsUnsigned) {
+ std::string ValStr;
+ if (IsUnsigned) {
+ std::optional<uint64_t> UVal = FormValue.getAsUnsignedConstant();
+ if (!UVal)
+ return;
+
+ ValStr = std::to_string(*UVal);
+ } else {
+ std::optional<int64_t> SVal = FormValue.getAsSignedConstant();
+ if (!SVal)
+ return;
+
+ ValStr = std::to_string(*SVal);
+ }
+
+ OS << '(';
+ appendQualifiedName(Cast);
+ OS << ')';
+ OS << std::move(ValStr);
+}
+
template <typename DieType>
bool DWARFTypePrinter<DieType>::appendTemplateParameters(DieType D,
bool *FirstParameter) {
@@ -438,13 +470,11 @@ bool DWARFTypePrinter<DieType>::appendTemplateParameters(DieType D,
DieType T = detail::resolveReferencedType(C);
Sep();
if (T.getTag() == dwarf::DW_TAG_enumeration_type) {
- OS << '(';
- appendQualifiedName(T);
- OS << ')';
auto V = C.find(dwarf::DW_AT_const_value);
- OS << std::to_string(*V->getAsSignedConstant());
+ appendCastedValue(*V, T, /*IsUnsigned=*/false);
continue;
}
+
// /Maybe/ we could do pointer/reference type parameters, looking for the
// symbol in the ELF symbol table to get back to the variable...
// but probably not worth it.
@@ -539,6 +569,12 @@ bool DWARFTypePrinter<DieType>::appendTemplateParameters(DieType D,
else
OS << llvm::format("'\\U%08" PRIx64 "'", Val);
}
+ // FIXME: Handle _BitInt's larger than 64-bits which are emitted as
+ // block data.
+ } else if (Name.starts_with("_BitInt")) {
+ appendCastedValue(*V, T, /*IsUnsigned=*/false);
+ } else if (Name.starts_with("unsigned _BitInt")) {
+ appendCastedValue(*V, T, /*IsUnsigned=*/true);
}
continue;
}
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp
index f566bee170236..a3fafec9e7813 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp
@@ -839,4 +839,144 @@ TEST(DWARFDie, DWARFTypePrinterTest) {
testAppendQualifiedName(Ctx->getDIEForOffset(0x1a), "t1<t3<int> >::t2");
testAppendQualifiedName(Ctx->getDIEForOffset(0x28), "t3<int>::my_int");
}
+
+TEST(DWARFDie, DWARFTypePrinterTest_BitInt) {
+ // Make sure we can reconstruct the case where a template value parameter
+ // is a _BitInt.
+
+ // DW_TAG_compile_unit
+ // DW_TAG_base_type
+ // DW_AT_name ("_BitInt(2)")
+ // DW_AT_bit_size ("2")
+ // DW_TAG_base_type
+ // DW_AT_name ("_BitInt(65)")
+ // DW_AT_bit_size ("65")
+ // DW_TAG_base_type
+ // DW_AT_name ("unsigned _BitInt(2)")
+ // DW_AT_bit_size ("2")
+ // DW_TAG_base_type
+ // DW_AT_name ("unsigned _BitInt(65)")
+ // DW_AT_bit_size ("65")
+ // DW_TAG_structure_type
+ // DW_AT_name ("foo")
+ // DW_TAG_template_value_parameter
+ // DW_AT_type ("_BitInt(2)")
+ // DW_AT_const_value (DW_FORM_sdata "-1")
+ // DW_TAG_template_value_parameter
+ // DW_AT_type ("unsigned _BitInt(2)")
+ // DW_AT_const_value (DW_FORM_udata "12")
+ // DW_TAG_template_value_parameter
+ // DW_AT_type ("_BitInt(65)")
+ // DW_AT_const_value (DW_FORM_block1 "1")
+ // DW_TAG_template_value_parameter
+ // DW_AT_type ("unsigned _BitInt(65)")
+ // DW_AT_const_value (DW_FORM_block1 "1")
+ // NULL
+ // NULL
+ const char *yamldata = R"(
+ debug_abbrev:
+ - ID: 0
+ Table:
+ - Code: 0x1
+ Tag: DW_TAG_compile_unit
+ Children: DW_CHILDREN_yes
+ - Code: 0x2
+ Tag: DW_TAG_base_type
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Code: 0x3
+ Tag: DW_TAG_structure_type
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Code: 0x4
+ Tag: DW_TAG_template_value_parameter
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref4
+ - Attribute: DW_AT_const_value
+ Form: DW_FORM_sdata
+ - Code: 0x5
+ Tag: DW_TAG_template_value_parameter
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref4
+ - Attribute: DW_AT_const_value
+ Form: DW_FORM_udata
+ - Code: 0x6
+ Tag: DW_TAG_template_value_parameter
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref4
+ - Attribute: DW_AT_const_value
+ Form: DW_FORM_block1
+ debug_info:
+ - Version: 4
+ AddrSize: 8
+ Entries:
+ - AbbrCode: 0x1
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0xDEADBEEFDEADBEEF
+ CStr: _BitInt(2)
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0xDEADBEEFDEADBEEF
+ CStr: _BitInt(65)
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0xDEADBEEFDEADBEEF
+ CStr: unsigned _BitInt(2)
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0xDEADBEEFDEADBEEF
+ CStr: unsigned _BitInt(65)
+ - AbbrCode: 0x3
+ Values:
+ - Value: 0xDEADBEEFDEADBEEF
+ CStr: foo
+ - AbbrCode: 0x4
+ Values:
+ - Value: 0x0000000c
+ - Value: 0xffffffffffffffff
+ - AbbrCode: 0x5
+ Values:
+ - Value: 0x00000025
+ - Value: 12
+ - AbbrCode: 0x6
+ Values:
+ - Value: 0x00000018
+ - Value: 0x0F
+ BlockData: [ 0x22, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
+ - AbbrCode: 0x6
+ Values:
+ - Value: 0x0000003a
+ - Value: 0x0F
+ BlockData: [ 0x22, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
+ - AbbrCode: 0x0
+ - AbbrCode: 0x0
+)";
+ Expected<StringMap<std::unique_ptr<MemoryBuffer>>> Sections =
+ DWARFYAML::emitDebugSections(StringRef(yamldata),
+ /*IsLittleEndian=*/true,
+ /*Is64BitAddrSize=*/true);
+ ASSERT_THAT_EXPECTED(Sections, Succeeded());
+ std::unique_ptr<DWARFContext> Ctx =
+ DWARFContext::create(*Sections, 4, /*isLittleEndian=*/true);
+
+ // FIXME: support _BitInt's with block forms. Currently they are just omitted.
+ // Will be necessary once -gsimple-template-names emit template value
+ // parameters with bit-width larger than 64.
+ testAppendAndTerminateTemplateParameters(
+ Ctx->getDIEForOffset(0x50),
+ "<(_BitInt(2))-1, (unsigned _BitInt(2))12, , >");
+}
} // end anonymous namespace
|
485a2df to
a241f5d
Compare
dwblaikie
left a comment
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 changes to simple-template-names.cpp should, in some part of this series, end up in the cross-project-tests version of the test, so we test the full roundtrip.
Thanks so much for taking an interest in/improving simplified template names!
Np! Was trying to see if we can make it a Darwin default (might be hard if most tools rely on the non-simple template names, but haven't really checked) |
I adjusted the |
a241f5d to
d90cecc
Compare
🐧 Linux x64 Test Results
|
I'd probably skip the unit test, personally... but I won't insist. |
dwblaikie
left a comment
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.
SGTM - as separate changes to LLVM, Clang, and cross-project-tests.
d90cecc to
d55daec
Compare
As of recent, LLVM includes the bit-size as a `DW_AT_bit_size` (and as part of `DW_AT_name`) of `_BitInt`s in DWARF. This allows us to mark `_BitInt`s as "reconstitutable" when compiling with `-gsimple-template-names`. However, before doing so we need to make sure the `DWARFTypePrinter` can reconstruct template parameter values that have `_BitInt` type. This patch adds support for printing `DW_TAG_template_value_parameter`s that have `_BitInt` type. Since `-gsimple-template-names` only omits template parameters that are `<= 64` bit wide, we don't support `_BitInt`s larger than 64 bits.
…simple-template-names Depends on: * llvm#168382 As of recent, LLVM includes the bit-size as a `DW_AT_bit_size` (and as part of `DW_AT_name`) of `_BitInt`s in DWARF. This allows us to mark `_BitInt`s as "reconstitutable" when compiling with `-gsimple-template-names`. We still only omits template parameters that are `<= 64` bit wide. So support `_BitInt`s larger than 64 bits is not part of this patch.
d55daec to
1fba5ac
Compare
Depends on:
As of recent, LLVM includes the bit-size as a
DW_AT_bit_size(and as part ofDW_AT_name) of_BitInts in DWARF. This allows us to mark_BitInts as "reconstitutable" when compiling with-gsimple-template-names. We still only omit template parameters that are<= 64bit wide. So support_BitInts larger than 64 bits is not part of this patch.