Skip to content
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

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

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

ayermolo
Copy link
Contributor

@ayermolo ayermolo commented Oct 5, 2023

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
Copy link
Collaborator

llvmbot commented Oct 5, 2023

@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
Copy link
Collaborator

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
Copy link
Contributor Author

ayermolo commented Oct 6, 2023

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
Copy link
Contributor

felipepiovezan commented Oct 6, 2023

+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
Copy link
Contributor

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

@ayermolo
Copy link
Contributor Author

ayermolo commented Oct 6, 2023

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. :)

@ayermolo 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
@dwblaikie
Copy link
Collaborator

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
Copy link
Contributor Author

ayermolo commented Oct 6, 2023

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.

@ayermolo ayermolo force-pushed the debugNamesAbbrevPrint branch 3 times, most recently from 85c6cc9 to 1f66961 Compare October 6, 2023 22:33
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
@ayermolo ayermolo merged commit 7850225 into llvm:main Oct 6, 2023
3 checks passed
@ayermolo ayermolo deleted the debugNamesAbbrevPrint branch October 6, 2023 23:19
felipepiovezan pushed a commit to felipepiovezan/llvm-project that referenced this pull request Nov 13, 2023
…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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants