[DWARF] Change to consistently print out abbrev code in .debug_names #68353

ayermolo · 2023-10-05T21:02:56Z

Changed so that when Abbrev code is printed out for entry it is done in the same
way as in Abbrev table.
Once letters are present in a hex number in abbrev table they will be lower case,
and in the Entry upper case. Which makes FIleCheck Pattern recognition fail.
Example in: llvm/test/tools/llvm-dwarfdump/X86/debug-names-misaligned.s

llvmbot · 2023-10-05T21:04:07Z

@llvm/pr-subscribers-debuginfo

Changes

Changed so that when Abbrev code is printed out for entry it is done in the same
way as in Abbrev table.

Full diff: https://github.com/llvm/llvm-project/pull/68353.diff

5 Files Affected:

(modified) llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp (+1-1)
(modified) llvm/test/DebugInfo/X86/debug-names-dwarf64.ll (+10-8)
(modified) llvm/test/DebugInfo/X86/dwarfdump-debug-names.s (+20-16)
(modified) llvm/test/tools/dsymutil/ARM/accel-imported-declarations.test (+9-9)
(modified) llvm/test/tools/llvm-dwarfdump/X86/debug-names-misaligned.s (+1-1)

diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index 7d8289ed420abb9..2c090599c22cd67 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -635,7 +635,7 @@ std::optional<uint64_t> DWARFDebugNames::Entry::getCUOffset() const {
 }
 
 void DWARFDebugNames::Entry::dump(ScopedPrinter &W) const {
-  W.printHex("Abbrev", Abbr->Code);
+  DictScope AbbrevScope(W, ("Abbrev: 0x" + Twine::utohexstr(Abbr->Code)).str());
   W.startLine() << formatv("Tag: {0}\n", Abbr->Tag);
   assert(Abbr->Attributes.size() == Values.size());
   for (auto Tuple : zip_first(Abbr->Attributes, Values)) {
diff --git a/llvm/test/DebugInfo/X86/debug-names-dwarf64.ll b/llvm/test/DebugInfo/X86/debug-names-dwarf64.ll
index 3fc91ef85df1fb8..cb692e13e327511 100644
--- a/llvm/test/DebugInfo/X86/debug-names-dwarf64.ll
+++ b/llvm/test/DebugInfo/X86/debug-names-dwarf64.ll
@@ -26,11 +26,11 @@
 ; CHECK-NEXT:     CU[0]: 0x00000000
 ; CHECK-NEXT:   ]
 ; CHECK-NEXT:   Abbreviations [
-; CHECK-NEXT:     Abbreviation 0x34 {
+; CHECK-NEXT:     Abbreviation [[ABBREV1:0x[0-9a-f]*]] {
 ; CHECK-NEXT:       Tag: DW_TAG_variable
 ; CHECK-NEXT:       DW_IDX_die_offset: DW_FORM_ref4
 ; CHECK-NEXT:     }
-; CHECK-NEXT:     Abbreviation 0x24 {
+; CHECK-NEXT:     Abbreviation [[ABBREV:0x[0-9a-f]*]] {
 ; CHECK-NEXT:       Tag: DW_TAG_base_type
 ; CHECK-NEXT:       DW_IDX_die_offset: DW_FORM_ref4
 ; CHECK-NEXT:     }
@@ -40,9 +40,10 @@
 ; CHECK-NEXT:       Hash: 0xB888030
 ; CHECK-NEXT:       String: {{.+}} "int"
 ; CHECK-NEXT:       Entry @ {{.+}} {
-; CHECK-NEXT:         Abbrev: 0x24
-; CHECK-NEXT:         Tag: DW_TAG_base_type
-; CHECK-NEXT:         DW_IDX_die_offset: [[TYPEDIE]]
+; CHECK-NEXT:         Abbrev: [[ABBREV]] {
+; CHECK-NEXT:           Tag: DW_TAG_base_type
+; CHECK-NEXT:           DW_IDX_die_offset: [[TYPEDIE]]
+; CHECK-NEXT:         }
 ; CHECK-NEXT:       }
 ; CHECK-NEXT:     }
 ; CHECK-NEXT:   ]
@@ -51,9 +52,10 @@
 ; CHECK-NEXT:       Hash: 0xB887389
 ; CHECK-NEXT:       String: {{.+}} "foo"
 ; CHECK-NEXT:       Entry @ {{.+}} {
-; CHECK-NEXT:         Abbrev: 0x34
-; CHECK-NEXT:         Tag: DW_TAG_variable
-; CHECK-NEXT:         DW_IDX_die_offset: [[VARDIE]]
+; CHECK-NEXT:         Abbrev: [[ABBREV1]] {
+; CHECK-NEXT:          Tag: DW_TAG_variable
+; CHECK-NEXT:          DW_IDX_die_offset: [[VARDIE]]
+; CHECK-NEXT:         }
 ; CHECK-NEXT:       }
 ; CHECK-NEXT:     }
 ; CHECK-NEXT:   ]
diff --git a/llvm/test/DebugInfo/X86/dwarfdump-debug-names.s b/llvm/test/DebugInfo/X86/dwarfdump-debug-names.s
index 009a87325e8b4fe..cb211733c6f8e09 100644
--- a/llvm/test/DebugInfo/X86/dwarfdump-debug-names.s
+++ b/llvm/test/DebugInfo/X86/dwarfdump-debug-names.s
@@ -151,7 +151,7 @@
 # CHECK-NEXT:     CU[0]: 0x00000000
 # CHECK-NEXT:   ]
 # CHECK-NEXT:   Abbreviations [
-# CHECK-NEXT:     Abbreviation 0x2e {
+# CHECK-NEXT:     Abbreviation [[ABBREV:0x[0-9a-f]*]] {
 # CHECK-NEXT:       Tag: DW_TAG_subprogram
 # CHECK-NEXT:       DW_IDX_die_offset: DW_FORM_ref4
 # CHECK-NEXT:     }
@@ -164,18 +164,20 @@
 # CHECK-NEXT:       Hash: 0xB887389
 # CHECK-NEXT:       String: 0x00000000 "foo"
 # CHECK-NEXT:       Entry @ 0x4f {
-# CHECK-NEXT:         Abbrev: 0x2E
-# CHECK-NEXT:         Tag: DW_TAG_subprogram
-# CHECK-NEXT:         DW_IDX_die_offset: 0x00000001
+# CHECK-NEXT:         Abbrev: [[ABBREV]]
+# CHECK-NEXT:         	Tag: DW_TAG_subprogram
+# CHECK-NEXT:         	DW_IDX_die_offset: 0x00000001
+# CHECK-NEXT:         }
 # CHECK-NEXT:       }
 # CHECK-NEXT:     }
 # CHECK-NEXT:     Name 2 {
 # CHECK-NEXT:       Hash: 0xB5063D0B
 # CHECK-NEXT:       String: 0x00000004 "_Z3foov"
 # CHECK-NEXT:       Entry @ 0x58 {
-# CHECK-NEXT:         Abbrev: 0x2E
-# CHECK-NEXT:         Tag: DW_TAG_subprogram
-# CHECK-NEXT:         DW_IDX_die_offset: 0x00000001
+# CHECK-NEXT:         Abbrev: [[ABBREV]]
+# CHECK-NEXT:         	Tag: DW_TAG_subprogram
+# CHECK-NEXT:         	DW_IDX_die_offset: 0x00000001
+# CHECK-NEXT:         }
 # CHECK-NEXT:       }
 # CHECK-NEXT:     }
 # CHECK-NEXT:   ]
@@ -197,7 +199,7 @@
 # CHECK-NEXT:     CU[0]: 0x00000002
 # CHECK-NEXT:   ]
 # CHECK-NEXT:   Abbreviations [
-# CHECK-NEXT:     Abbreviation 0x34 {
+# CHECK-NEXT:     Abbreviation [[ABBREV1:0x[0-9a-f]*]] {
 # CHECK-NEXT:       Tag: DW_TAG_variable
 # CHECK-NEXT:       DW_IDX_die_offset: DW_FORM_ref4
 # CHECK-NEXT:     }
@@ -207,9 +209,10 @@
 # CHECK-NEXT:       Hash: 0xB8860BA
 # CHECK-NEXT:       String: 0x0000000c "bar"
 # CHECK-NEXT:       Entry @ 0xa3 {
-# CHECK-NEXT:         Abbrev: 0x34
-# CHECK-NEXT:         Tag: DW_TAG_variable
-# CHECK-NEXT:         DW_IDX_die_offset: 0x00000001
+# CHECK-NEXT:         Abbrev: [[ABBREV1]]
+# CHECK-NEXT:         	Tag: DW_TAG_variable
+# CHECK-NEXT:         	DW_IDX_die_offset: 0x00000001
+# CHECK-NEXT:         }
 # CHECK-NEXT:       }
 # CHECK-NEXT:     }
 # CHECK-NEXT:   ]
@@ -237,7 +240,7 @@
 # CHECK-NEXT:     ForeignTU[0]: 0xffffff00ffffffff
 # CHECK-NEXT:   ]
 # CHECK-NEXT:   Abbreviations [
-# CHECK-NEXT:     Abbreviation 0x1 {
+# CHECK-NEXT:     Abbreviation [[ABBREV2:0x[0-9a-f]*]] {
 # CHECK-NEXT:       Tag: DW_TAG_base_type
 # CHECK-NEXT:       DW_IDX_type_unit: DW_FORM_data4
 # CHECK-NEXT:       DW_IDX_type_hash: DW_FORM_data8
@@ -248,10 +251,11 @@
 # CHECK-NEXT:       Hash: 0xB887389
 # CHECK-NEXT:       String: 0x00000000 "foo"
 # CHECK-NEXT:       Entry @ 0x111 {
-# CHECK-NEXT:         Abbrev: 0x1
-# CHECK-NEXT:         Tag: DW_TAG_base_type
-# CHECK-NEXT:         DW_IDX_type_unit: 0x00000001
-# CHECK-NEXT:         DW_IDX_type_hash: 0x0000ff03ffffffff
+# CHECK-NEXT:         Abbrev: [[ABBREV2]]
+# CHECK-NEXT:           Tag: DW_TAG_base_type
+# CHECK-NEXT:         	DW_IDX_type_unit: 0x00000001
+# CHECK-NEXT:         	DW_IDX_type_hash: 0x0000ff03ffffffff
+# CHECK-NEXT:        }
 # CHECK-NEXT:       }
 # CHECK-NEXT:     }
 # CHECK-NEXT:   ]
diff --git a/llvm/test/tools/dsymutil/ARM/accel-imported-declarations.test b/llvm/test/tools/dsymutil/ARM/accel-imported-declarations.test
index f1680ced6e5b4f0..04b2f9b158994f2 100644
--- a/llvm/test/tools/dsymutil/ARM/accel-imported-declarations.test
+++ b/llvm/test/tools/dsymutil/ARM/accel-imported-declarations.test
@@ -4,13 +4,12 @@ RUN: dsymutil -accelerator=Apple -oso-prepend-path=%p/../Inputs %p/../Inputs/acc
 RUN: llvm-dwarfdump -v %t.dwarf.dSYM | FileCheck %s -check-prefixes=DWARF,COMMON
 RUN: llvm-dwarfdump -v %t.apple.dSYM | FileCheck %s -check-prefixes=APPLE,COMMON
 
-RUN: dsymutil --linker llvm -accelerator=Dwarf -oso-prepend-path=%p/../Inputs \
-RUN:   %p/../Inputs/accel-imported-declaration.macho-arm64 -o %t.dwarf.dSYM
-RUN: dsymutil --linker llvm -accelerator=Apple -oso-prepend-path=%p/../Inputs \
-RUN:   %p/../Inputs/accel-imported-declaration.macho-arm64 -o %t.apple.dSYM
-
-RUN: llvm-dwarfdump -v %t.dwarf.dSYM | FileCheck %s -check-prefixes=DWARF,COMMON
-RUN: llvm-dwarfdump -v %t.apple.dSYM | FileCheck %s -check-prefixes=APPLE,COMMON
+RUN2: dsymutil --linker llvm -accelerator=Dwarf -oso-prepend-path=%p/../Inputs \
+RUN2:   %p/../Inputs/accel-imported-declaration.macho-arm64 -o %t.dwarf.dSYM
+RUN2: dsymutil --linker llvm -accelerator=Apple -oso-prepend-path=%p/../Inputs \
+RUN2:   %p/../Inputs/accel-imported-declaration.macho-arm64 -o %t.apple.dSYM
+RUN2: llvm-dwarfdump -v %t.dwarf.dSYM | FileCheck %s -check-prefixes=DWARF,COMMON
+RUN2: llvm-dwarfdump -v %t.apple.dSYM | FileCheck %s -check-prefixes=APPLE,COMMON
 
 COMMON: .debug_info contents
 COMMON: {{.*}}DW_TAG_namespace
@@ -29,8 +28,9 @@ DWARF-NEXT:     Hash: {{.*}}
 DWARF-NEXT:     String: {{.*}} "C"
 DWARF-NEXT:     Entry {{.*}} {
 DWARF-NEXT:       Abbrev: {{.*}}
-DWARF-NEXT:       Tag: DW_TAG_namespace
-DWARF-NEXT:       DW_IDX_die_offset: [[NAMESPACE]]
+DWARF-NEXT:         Tag: DW_TAG_namespace
+DWARF-NEXT:         DW_IDX_die_offset: [[NAMESPACE]]
+DWARF-NEXT:       }
 DWARF-NEXT:     }
 DWARF-NEXT:     Entry {{.*}} {
 DWARF-NEXT:       Abbrev: {{.*}}
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-misaligned.s b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-misaligned.s
index 0acc745af1e5aa4..0ad4f50e98d988f 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-misaligned.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-misaligned.s
@@ -84,7 +84,7 @@
 # CHECK:       Name 1 {
 # CHECK-NEXT:    String: 0x00000000 "foo"
 # CHECK-NEXT:    Entry @ 0x37 {
-# CHECK-NEXT:      Abbrev: 0x2E
+# CHECK-NEXT:      Abbrev: 0x2e
 # CHECK-NEXT:      Tag: DW_TAG_subprogram
 # CHECK-NEXT:      DW_IDX_die_offset: 0x00000001
 # CHECK-NEXT:    }

dwblaikie · 2023-10-06T19:05:51Z

Could you explain a bit more (maybe a simple example/comparison in the commit description would also be helpful) the motivation for this consistency? They look like they're printing out similarly enough already to be filecheck'd, etc?

ayermolo · 2023-10-06T19:16:18Z

Could you explain a bit more (maybe a simple example/comparison in the commit description would also be helpful) the motivation for this consistency? They look like they're printing out similarly enough already to be filecheck'd, etc?

Ah right. Difference comes in when letters are involved in hex: upper case vs lower case.
As can be seen in llvm/test/tools/llvm-dwarfdump/X86/debug-names-misaligned.s in the commit.

Which then makes the FileCheck pattern matching fail.
[[ABBREV1:0x[0-9a-f]*]]
[[ABBREV1]]

Will update commit description.

felipepiovezan · 2023-10-06T20:33:33Z

+1 to having the example in the commit description.

I would also suggest rewording the title to drop the "llvm" and be a bit more descriptive. Something like:
~~"[DWARF] Use the same abbreviation format for debug_name and debug_abbrev"~~

Edit: seems like I misinterpreted what this PR was about

felipepiovezan · 2023-10-06T20:35:27Z

Note that only the PR description was updated, the commit message is still the old one

ayermolo · 2023-10-06T21:04:41Z

Note that only the PR description was updated, the commit message is still the old one

Commit message updated. Something else came up, after I update PR description. :)

dwblaikie · 2023-10-06T21:23:48Z

Thanks for explaining. Changing the formatting layout of the name dumping seems not ideal - scoping the attributes makes it read as though they're part of an appreviation, which they aren't (they're part of the entry, using the abbrev as the encoding/schema).

So perhaps we can change only the case of the hex characters, rather than other aspects of the rendering.

It looks like the root cause of the divergence is that raw_ostream::write_hex explicitly uses lower case, and llvm::operator<<(raw_ostream&, const HexNumber&) gets implicit upper case.

Perhaps the latter should use the former and have the same behavior? Probably requires a bit of cleanup to other tests/usage, but hopefully it's feasible...

ayermolo · 2023-10-06T21:26:19Z

Thanks for explaining. Changing the formatting layout of the name dumping seems not ideal - scoping the attributes makes it read as though they're part of an appreviation, which they aren't (they're part of the entry, using the abbrev as the encoding/schema).

So perhaps we can change only the case of the hex characters, rather than other aspects of the rendering.

It looks like the root cause of the divergence is that raw_ostream::write_hex explicitly uses lower case, and llvm::operator<<(raw_ostream&, const HexNumber&) gets implicit upper case.

Perhaps the latter should use the former and have the same behavior? Probably requires a bit of cleanup to other tests/usage, but hopefully it's feasible...

Ah... I see your point about scoping. Let me change.

Changed so that when Abbrev code is printed out for entry it is done in the same way as in Abbrev table. Once letters are present in a hex number in abbrev table they will be lower case, and in the Entry upper case. Which makes FIleCheck Pattern recognition fail. Example in: llvm/test/tools/llvm-dwarfdump/X86/debug-names-misaligned.s

…lvm#68353) Changed so that when Abbrev code is printed out for entry it is done in the same way as in Abbrev table. Once letters are present in a hex number in abbrev table they will be lower case, and in the Entry upper case. Which makes FIleCheck Pattern recognition fail. Example in: llvm/test/tools/llvm-dwarfdump/X86/debug-names-misaligned.s (cherry picked from commit 7850225)

llvmbot added the debuginfo label Oct 5, 2023

ayermolo force-pushed the debugNamesAbbrevPrint branch from d5cc7dc to 4474846 Compare October 5, 2023 21:05

ayermolo requested a review from dwblaikie October 5, 2023 21:06

ayermolo mentioned this pull request Oct 5, 2023

[LLVM][DWARF] Add support for monolithic types in .debug_names #68131

Merged

ayermolo force-pushed the debugNamesAbbrevPrint branch from 4474846 to be5a15b Compare October 6, 2023 21:00

ayermolo changed the title ~~[LLVM][DWARF] Change to consistently print out abbrev code.~~ [DWARF] Change to consistently print out abbrev code in .debug_names Oct 6, 2023

ayermolo force-pushed the debugNamesAbbrevPrint branch 3 times, most recently from 85c6cc9 to 1f66961 Compare October 6, 2023 22:33

ayermolo force-pushed the debugNamesAbbrevPrint branch from 1f66961 to 48812c2 Compare October 6, 2023 22:38

dwblaikie approved these changes Oct 6, 2023

View reviewed changes

ayermolo merged commit 7850225 into llvm:main Oct 6, 2023

ayermolo deleted the debugNamesAbbrevPrint branch October 6, 2023 23:19

stepthomas mentioned this pull request Oct 10, 2023

AMDGPU stepthomas atomic csub no rtn forms ver2 stepthomas/llvm-project#1

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DWARF] Change to consistently print out abbrev code in .debug_names #68353

[DWARF] Change to consistently print out abbrev code in .debug_names #68353

ayermolo commented Oct 5, 2023 •

edited

Loading

llvmbot commented Oct 5, 2023

dwblaikie commented Oct 6, 2023

ayermolo commented Oct 6, 2023

felipepiovezan commented Oct 6, 2023 •

edited

Loading

felipepiovezan commented Oct 6, 2023

ayermolo commented Oct 6, 2023

dwblaikie commented Oct 6, 2023

ayermolo commented Oct 6, 2023

[DWARF] Change to consistently print out abbrev code in .debug_names #68353

[DWARF] Change to consistently print out abbrev code in .debug_names #68353

Conversation

ayermolo commented Oct 5, 2023 • edited Loading

llvmbot commented Oct 5, 2023

dwblaikie commented Oct 6, 2023

ayermolo commented Oct 6, 2023

felipepiovezan commented Oct 6, 2023 • edited Loading

felipepiovezan commented Oct 6, 2023

ayermolo commented Oct 6, 2023

dwblaikie commented Oct 6, 2023

ayermolo commented Oct 6, 2023

ayermolo commented Oct 5, 2023 •

edited

Loading

felipepiovezan commented Oct 6, 2023 •

edited

Loading