[DWARF] Fix DWARTTypePrinter unable to print qualified name for DW_TAG_typedef DIE #117239
Conversation
|
@llvm/pr-subscribers-debuginfo Author: Zequan Wu (ZequanWu) ChangesFix a bug introduced in #117071. Ideally the DWARTTypePrinter test should go to Full diff: https://github.com/llvm/llvm-project/pull/117239.diff 2 Files Affected:
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
index ae63e286cc1551..3a03ed283a98d4 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
@@ -23,6 +23,26 @@ using namespace lldb_private;
using namespace lldb_private::plugin::dwarf;
using namespace lldb_private::dwarf;
+namespace {
+void Test_appendAndTerminateTemplateParameters(const DWARFDIE &die,
+ const std::string &expected) {
+ std::string template_name;
+ llvm::raw_string_ostream template_name_os(template_name);
+ llvm::DWARFTypePrinter<DWARFDIE> template_name_printer(template_name_os);
+ template_name_printer.appendAndTerminateTemplateParameters(die);
+ EXPECT_THAT(template_name, expected);
+}
+
+void Test_appendQualifiedName(const DWARFDIE &die,
+ const std::string &expected) {
+ std::string qualified_name;
+ llvm::raw_string_ostream template_name_os(qualified_name);
+ llvm::DWARFTypePrinter<DWARFDIE> template_name_printer(template_name_os);
+ template_name_printer.appendQualifiedName(die);
+ EXPECT_THAT(qualified_name, expected);
+}
+} // namespace
+
TEST(DWARFDIETest, ChildIteration) {
// Tests DWARFDIE::child_iterator.
@@ -466,6 +486,14 @@ TEST(DWARFDIETest, TestDWARFTypePrinter) {
Attributes:
- Attribute: DW_AT_type
Form: DW_FORM_ref4
+ - Code: 0x8
+ Tag: DW_TAG_typedef
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref4
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
debug_info:
- Version: 4
AddrSize: 8
@@ -494,6 +522,10 @@ TEST(DWARFDIETest, TestDWARFTypePrinter) {
- AbbrCode: 0x7
Values:
- Value: 0x0000000c # update
+ - AbbrCode: 0x8
+ Values:
+ - Value: 0x0000000c
+ - CStr: my_int
- AbbrCode: 0x0
- AbbrCode: 0x0)";
YAMLModuleTester t(yamldata);
@@ -505,17 +537,7 @@ TEST(DWARFDIETest, TestDWARFTypePrinter) {
unit->Dump(&debug_os);
ASSERT_TRUE(unit);
- DWARFDIE t1_die = unit->GetDIE(0x11);
- std::string template_name;
- llvm::raw_string_ostream template_name_os(template_name);
- llvm::DWARFTypePrinter<DWARFDIE> template_name_printer(template_name_os);
- template_name_printer.appendAndTerminateTemplateParameters(t1_die);
- EXPECT_THAT(template_name, "<t3<int> >");
-
- DWARFDIE t2_die = unit->GetDIE(0x1a);
- std::string qualified_name;
- llvm::raw_string_ostream qualified_name_os(qualified_name);
- llvm::DWARFTypePrinter<DWARFDIE> qualified_name_printer(qualified_name_os);
- qualified_name_printer.appendQualifiedName(t2_die);
- EXPECT_THAT(qualified_name, "t1<t3<int> >::t2");
+ Test_appendAndTerminateTemplateParameters(unit->GetDIE(0x11), "<t3<int> >");
+ Test_appendQualifiedName(unit->GetDIE(0x1a), "t1<t3<int> >::t2");
+ Test_appendQualifiedName(unit->GetDIE(0x28), "t3<int>::my_int");
}
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
index 962462b8278259..3c936b93865045 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
@@ -70,6 +70,7 @@ template <typename DieType> struct DWARFTypePrinter {
case dwarf::DW_TAG_union_type:
case dwarf::DW_TAG_namespace:
case dwarf::DW_TAG_enumeration_type:
+ case dwarf::DW_TAG_typedef:
return true;
default:
break;
|
|
@llvm/pr-subscribers-lldb Author: Zequan Wu (ZequanWu) ChangesFix a bug introduced in #117071. Ideally the DWARTTypePrinter test should go to Full diff: https://github.com/llvm/llvm-project/pull/117239.diff 2 Files Affected:
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
index ae63e286cc1551..3a03ed283a98d4 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
@@ -23,6 +23,26 @@ using namespace lldb_private;
using namespace lldb_private::plugin::dwarf;
using namespace lldb_private::dwarf;
+namespace {
+void Test_appendAndTerminateTemplateParameters(const DWARFDIE &die,
+ const std::string &expected) {
+ std::string template_name;
+ llvm::raw_string_ostream template_name_os(template_name);
+ llvm::DWARFTypePrinter<DWARFDIE> template_name_printer(template_name_os);
+ template_name_printer.appendAndTerminateTemplateParameters(die);
+ EXPECT_THAT(template_name, expected);
+}
+
+void Test_appendQualifiedName(const DWARFDIE &die,
+ const std::string &expected) {
+ std::string qualified_name;
+ llvm::raw_string_ostream template_name_os(qualified_name);
+ llvm::DWARFTypePrinter<DWARFDIE> template_name_printer(template_name_os);
+ template_name_printer.appendQualifiedName(die);
+ EXPECT_THAT(qualified_name, expected);
+}
+} // namespace
+
TEST(DWARFDIETest, ChildIteration) {
// Tests DWARFDIE::child_iterator.
@@ -466,6 +486,14 @@ TEST(DWARFDIETest, TestDWARFTypePrinter) {
Attributes:
- Attribute: DW_AT_type
Form: DW_FORM_ref4
+ - Code: 0x8
+ Tag: DW_TAG_typedef
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref4
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
debug_info:
- Version: 4
AddrSize: 8
@@ -494,6 +522,10 @@ TEST(DWARFDIETest, TestDWARFTypePrinter) {
- AbbrCode: 0x7
Values:
- Value: 0x0000000c # update
+ - AbbrCode: 0x8
+ Values:
+ - Value: 0x0000000c
+ - CStr: my_int
- AbbrCode: 0x0
- AbbrCode: 0x0)";
YAMLModuleTester t(yamldata);
@@ -505,17 +537,7 @@ TEST(DWARFDIETest, TestDWARFTypePrinter) {
unit->Dump(&debug_os);
ASSERT_TRUE(unit);
- DWARFDIE t1_die = unit->GetDIE(0x11);
- std::string template_name;
- llvm::raw_string_ostream template_name_os(template_name);
- llvm::DWARFTypePrinter<DWARFDIE> template_name_printer(template_name_os);
- template_name_printer.appendAndTerminateTemplateParameters(t1_die);
- EXPECT_THAT(template_name, "<t3<int> >");
-
- DWARFDIE t2_die = unit->GetDIE(0x1a);
- std::string qualified_name;
- llvm::raw_string_ostream qualified_name_os(qualified_name);
- llvm::DWARFTypePrinter<DWARFDIE> qualified_name_printer(qualified_name_os);
- qualified_name_printer.appendQualifiedName(t2_die);
- EXPECT_THAT(qualified_name, "t1<t3<int> >::t2");
+ Test_appendAndTerminateTemplateParameters(unit->GetDIE(0x11), "<t3<int> >");
+ Test_appendQualifiedName(unit->GetDIE(0x1a), "t1<t3<int> >::t2");
+ Test_appendQualifiedName(unit->GetDIE(0x28), "t3<int>::my_int");
}
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
index 962462b8278259..3c936b93865045 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
@@ -70,6 +70,7 @@ template <typename DieType> struct DWARFTypePrinter {
case dwarf::DW_TAG_union_type:
case dwarf::DW_TAG_namespace:
case dwarf::DW_TAG_enumeration_type:
+ case dwarf::DW_TAG_typedef:
return true;
default:
break;
|
|
Rather than a unit test, perhaps this could be tested with llvm-dwarfdump? (I think that's how I tested the DWARFTypePrinter during its initial development) It's a feature improvement to llvm-dwarfdump - being able to print out names better. I guess most of what I did was more specifically about simplified-template-names roundtripping, which this new typedef case isn't relevant to (typedefs aren't canonical names, don't themselves get simplified/have to be rebuilt). But the testing was in llvm/test/tools/llvm-dwarfdump/X86/simplified-template-names.s (and there's similar testing in the cross_project_tests and in clang - so adding the test coverage in those places too would be good, if this test file's one that could be used for this patch). I guess maybe this testing all only does "verify" testing, which means it won't print the typedef qualified name you want to test. In that case, we have some testing for type printing prior to the DWARFTypePrinter work... Oh, also - LLVM fixes should be tested in LLVM, not in lldb... Probably the most common way to test this would be to add something in llvm/test/DebugInfo/X86/dwarfdump-*.ll Oh, this test might be where we test most of this in the past: llvm/test/tools/llvm-dwarfdump/X86/prettyprint_types.s |
|
Just noticed that this breaks |
|
FYI I have temporary reverted f06c187 to keep things rolling |
@JDevlieghere can probably speak more to that, but sounds very golden-file-y & can probably be updated. I take it it's this part of the test? Where those names go from "const string" to "const std::__1::string" (and from "string" to "std::__1::string")? or something like that. |
Yes. In some cases, it's getting super verbose, e.g:
Because DWARTTypePrinter is used by both llvm-dwarfump and lldb (after a reland), how about just test DWARTTypePrinter in |
Oh yeah, still good though.
Perhaps - if it's easy to write/legible. Though I don't think we should feel bad about somewhat "indirectly" testing libDebugInfoDWARF via llvm-dwarfdump tests, rather than restricting ourselves to API tests for bugs in libDebugInfoDWARF. So whichever's more legible/maintainable. |
ZequanWu
force-pushed
the
fix-dwarf-type-printer
branch
from
5ecdcda to
f006f03
Compare
|
Moved the unit tests for DWARFTypePrinter from lldb to |
Fix a bug introduced in #117071.
Ideally the DWARTTypePrinter test should go to
llvm/unittests/DebugInfo/DWARF/DWARTTypePrinterTest.cpp.