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

[llvm][DebugInfo] Add DW_AT_type to DW_TAG_enumeration_type in non-strict DWARF v2 mode #98335

Merged
merged 4 commits into from
Jul 12, 2024

Conversation

DavidSpickett
Copy link
Collaborator

During testing of #96202 we found that when clang set to DWARF v2 was used to build the test file, lldb could not tell that the unsigned enum type was in fact unsigned. So it defaulted to signed and printed the wrong value.

The reason for this is that DWARFv2 does not include DW_AT_type in DW_TAG_enumeration_type. This was added in DWARF v3:
"The enumeration type entry may also have a DW_AT_type attribute which refers to the underlying data type used to implement the enumeration.

In C or C++, the underlying type will be the appropriate integral type determined by the compiler from the properties of the enumeration literal values."

I noticed that gcc does emit this attribute for DWARF v2 but not when strict DWARF is requested (more details in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16063#c7).

This patch changes to clang to do the same. This will improve the experience of anyone using tools that can understand the attribute but for whatever reason are stuck building binaries containing v2 only.

You can see a current clang/gcc comparison here: https://godbolt.org/z/eG9Kc9WGf

https://reviews.llvm.org/D42734 added the original code that emitted this for >= v3 only.

…rict DWARF v2 mode

During testing of llvm#96202 we found that
when clang set to DWARF v2 was used to build the test file, lldb could not tell
that the unsigned enum type was in fact unsigned. So it defaulted to signed and
printed the wrong value.

The reason for this is that DWARFv2 does not include DW_AT_type in DW_TAG_enumeration_type.
This was added in DWARF v3:
"The enumeration type entry may also have a DW_AT_type attribute which refers to the
underlying data type used to implement the enumeration.

In C or C++, the underlying type will be the appropriate integral type determined by the
compiler from the properties of the enumeration literal values."

I noticed that gcc does emit this attribute for DWARF v2 but not when strict
DWARF is requested (more details in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16063#c7).

This patch changes to clang to do the same. This will improve the experience of anyone
using tools that can understand the attribute but for whatever reason are stuck
building binaries containing v2 only.

You can see a current clang/gcc comparison here: https://godbolt.org/z/eG9Kc9WGf

https://reviews.llvm.org/D42734 added the original code that emitted this
for >= v3 only.
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 10, 2024

@llvm/pr-subscribers-debuginfo

Author: David Spickett (DavidSpickett)

Changes

During testing of #96202 we found that when clang set to DWARF v2 was used to build the test file, lldb could not tell that the unsigned enum type was in fact unsigned. So it defaulted to signed and printed the wrong value.

The reason for this is that DWARFv2 does not include DW_AT_type in DW_TAG_enumeration_type. This was added in DWARF v3:
"The enumeration type entry may also have a DW_AT_type attribute which refers to the underlying data type used to implement the enumeration.

In C or C++, the underlying type will be the appropriate integral type determined by the compiler from the properties of the enumeration literal values."

I noticed that gcc does emit this attribute for DWARF v2 but not when strict DWARF is requested (more details in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16063#c7).

This patch changes to clang to do the same. This will improve the experience of anyone using tools that can understand the attribute but for whatever reason are stuck building binaries containing v2 only.

You can see a current clang/gcc comparison here: https://godbolt.org/z/eG9Kc9WGf

https://reviews.llvm.org/D42734 added the original code that emitted this for >= v3 only.


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

3 Files Affected:

  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+1-1)
  • (modified) llvm/test/DebugInfo/Generic/debug-info-enum.ll (+26-20)
  • (modified) llvm/test/DebugInfo/X86/dbg-rust-valid-enum-as-scope.ll (+3)
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index 6c04fa1c67a95..e76b0fe2081c0 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1585,7 +1585,7 @@ void DwarfUnit::constructEnumTypeDIE(DIE &Buffer, const DICompositeType *CTy) {
   const DIType *DTy = CTy->getBaseType();
   bool IsUnsigned = DTy && DD->isUnsignedDIType(DTy);
   if (DTy) {
-    if (DD->getDwarfVersion() >= 3)
+    if (!Asm->TM.Options.DebugStrictDwarf || DD->getDwarfVersion() >= 3)
       addType(Buffer, DTy);
     if (DD->getDwarfVersion() >= 4 && (CTy->getFlags() & DINode::FlagEnumClass))
       addFlag(Buffer, dwarf::DW_AT_enum_class);
diff --git a/llvm/test/DebugInfo/Generic/debug-info-enum.ll b/llvm/test/DebugInfo/Generic/debug-info-enum.ll
index 85e22931a5fc1..9af465d59b7d9 100644
--- a/llvm/test/DebugInfo/Generic/debug-info-enum.ll
+++ b/llvm/test/DebugInfo/Generic/debug-info-enum.ll
@@ -2,11 +2,17 @@
 ; * test value representation for each possible underlying integer type
 ; * test the integer type is as expected
 ; * test the DW_AT_enum_class attribute is present (resp. absent) as expected.
+; * test that DW_AT_type is present for v3 and greater, and v2 when strict DWARF
+;   is not enabled.
 
 ; RUN: llc -debugger-tune=gdb -dwarf-version=4 -filetype=obj -o %t.o < %s
-; RUN: llvm-dwarfdump -debug-info %t.o | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-DW4
+; RUN: llvm-dwarfdump -debug-info %t.o | FileCheck %s --check-prefixes=CHECK,CHECK-DW4,CHECK-TYPE
+; RUN: llc -debugger-tune=gdb -dwarf-version=3 -filetype=obj -o %t.o < %s
+; RUN: llvm-dwarfdump -debug-info %t.o | FileCheck %s --check-prefixes=CHECK,CHECK-TYPE
 ; RUN: llc -debugger-tune=gdb -dwarf-version=2 -filetype=obj -o %t.o < %s
-; RUN: llvm-dwarfdump -debug-info %t.o | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-DW2
+; RUN: llvm-dwarfdump -debug-info %t.o | FileCheck %s --check-prefixes=CHECK,CHECK-TYPE
+; RUN: llc -debugger-tune=gdb -dwarf-version=2 -strict-dwarf=true -filetype=obj -o %t.o < %s
+; RUN: llvm-dwarfdump -debug-info %t.o | FileCheck %s --check-prefixes=CHECK,CHECK-DW2-STRICT
 
 @x0 = global i8 0, align 1, !dbg !0
 @x1 = global i8 0, align 1, !dbg !46
@@ -34,8 +40,8 @@
 !8 = !DIEnumerator(name: "A0", value: -128)
 !9 = !DIEnumerator(name: "B0", value: 127)
 ; CHECK:         DW_TAG_enumeration_type
-; CHECK-DW2-NOT:   DW_AT_type
-; CHECK-DW4:       DW_AT_type{{.*}}"signed char"
+; CHECK-DW2-STRICT-NOT: DW_AT_type
+; CHECK-TYPE:      DW_AT_type{{.*}}"signed char"
 ; CHECK-DW4:       DW_AT_enum_class        (true)
 ; CHECK:           DW_AT_name      ("E0")
 ; CHECK:         DW_TAG_enumerator
@@ -51,8 +57,8 @@
 !12 = !{!13}
 !13 = !DIEnumerator(name: "A1", value: 255, isUnsigned: true)
 ; CHECK:         DW_TAG_enumeration_type
-; CHECK-DW2-NOT:   DW_AT_type
-; CHECK-DW4:       DW_AT_type{{.*}}"unsigned char"
+; CHECK-DW2-STRICT-NOT: DW_AT_type
+; CHECK-TYPE:      DW_AT_type{{.*}}"unsigned char"
 ; CHECK-DW4:       DW_AT_enum_class        (true)
 ; CHECK:           DW_AT_name      ("E1")
 ; CHECK:         DW_TAG_enumerator
@@ -66,8 +72,8 @@
 !17 = !DIEnumerator(name: "A2", value: -32768)
 !18 = !DIEnumerator(name: "B2", value: 32767)
 ; CHECK:         DW_TAG_enumeration_type
-; CHECK-DW2-NOT:   DW_AT_type
-; CHECK-DW4:       DW_AT_type{{.*}} "short"
+; CHECK-DW2-STRICT-NOT:   DW_AT_type
+; CHECK-TYPE:      DW_AT_type{{.*}} "short"
 ; CHECK-DW4:       DW_AT_enum_class        (true)
 ; CHECK:           DW_AT_name      ("E2")
 ; CHECK:         DW_TAG_enumerator
@@ -83,8 +89,8 @@
 !21 = !{!22}
 !22 = !DIEnumerator(name: "A3", value: 65535, isUnsigned: true)
 ; CHECK:         DW_TAG_enumeration_type
-; CHECK-DW2-NOT:   DW_AT_type
-; CHECK-DW4:       DW_AT_type{{.*}}"unsigned short"
+; CHECK-DW2-STRICT-NOT: DW_AT_type
+; CHECK-TYPE       DW_AT_type{{.*}}"unsigned short"
 ; CHECK-DW4:       DW_AT_enum_class        (true)
 ; CHECK:           DW_AT_name      ("E3")
 ; CHECK:         DW_TAG_enumerator
@@ -98,8 +104,8 @@
 !26 = !DIEnumerator(name: "A4", value: -2147483648)
 !27 = !DIEnumerator(name: "B4", value: 2147483647)
 ; CHECK:         DW_TAG_enumeration_type
-; CHECK-DW2-NOT:   DW_AT_type
-; CHECK-DW4:       DW_AT_type{{.*}}"int"
+; CHECK-DW2-STRICT-NOT: DW_AT_type
+; CHECK-TYPE:      DW_AT_type{{.*}}"int"
 ; CHECK-DW4:       DW_AT_enum_class        (true)
 ; CHECK:           DW_AT_name      ("E4")
 ; CHECK:         DW_TAG_enumerator
@@ -115,8 +121,8 @@
 !30 = !{!31}
 !31 = !DIEnumerator(name: "A5", value: 4294967295, isUnsigned: true)
 ; CHECK:         DW_TAG_enumeration_type
-; CHECK-DW2-NOT:   DW_AT_type
-; CHECK-DW4:       DW_AT_type{{.*}}"unsigned int"
+; CHECK-DW2-STRICT-NOT: DW_AT_type
+; CHECK-TYPE:      DW_AT_type{{.*}}"unsigned int"
 ; CHECK-DW4:       DW_AT_enum_class        (true)
 ; CHECK:           DW_AT_name      ("E5")
 ; CHECK:         DW_TAG_enumerator
@@ -130,8 +136,8 @@
 !35 = !DIEnumerator(name: "A6", value: -9223372036854775808)
 !36 = !DIEnumerator(name: "B6", value: 9223372036854775807)
 ; CHECK:         DW_TAG_enumeration_type
-; CHECK-DW2-NOT:   DW_AT_type
-; CHECK-DW4:       DW_AT_type{{.*}}"long long int"
+; CHECK-DW2-STRICT-NOT:   DW_AT_type
+; CHECK-TYPE:      DW_AT_type{{.*}}"long long int"
 ; CHECK-DW4:       DW_AT_enum_class        (true)
 ; CHECK:           DW_AT_name      ("E6")
 ; CHECK:         DW_TAG_enumerator
@@ -147,8 +153,8 @@
 !39 = !{!40}
 !40 = !DIEnumerator(name: "A7", value: 18446744073709551615, isUnsigned: true)
 ; CHECK:         DW_TAG_enumeration_type
-; CHECK-DW2-NOT:   DW_AT_type
-; CHECK-DW4:       DW_AT_type{{.*}}"long long unsigned int"
+; CHECK-DW2-STRICT-NOT:   DW_AT_type
+; CHECK-TYPE:      DW_AT_type{{.*}}"long long unsigned int"
 ; CHECK-DW4:       DW_AT_enum_class        (true)
 ; CHECK:           DW_AT_name      ("E7")
 ; CHECK:         DW_TAG_enumerator
@@ -163,8 +169,8 @@
 !43 = !DIEnumerator(name: "A8", value: -128)
 !44 = !DIEnumerator(name: "B8", value: 127)
 ; CHECK:         DW_TAG_enumeration_type
-; CHECK-DW2-NOT:   DW_AT_type
-; CHECK-DW4:       DW_AT_type{{.*}}"int"
+; CHECK-DW2-STRICT-NOT:   DW_AT_type
+; CHECK-TYPE:      DW_AT_type{{.*}}"int"
 ; CHECK-NOT:       DW_AT_enum_class
 ; CHECK:           DW_AT_name      ("E8")
 
diff --git a/llvm/test/DebugInfo/X86/dbg-rust-valid-enum-as-scope.ll b/llvm/test/DebugInfo/X86/dbg-rust-valid-enum-as-scope.ll
index f44ca04e0fd96..a6f24081d63ee 100644
--- a/llvm/test/DebugInfo/X86/dbg-rust-valid-enum-as-scope.ll
+++ b/llvm/test/DebugInfo/X86/dbg-rust-valid-enum-as-scope.ll
@@ -3,6 +3,7 @@
 ; CHECK:   DW_AT_language	(DW_LANG_Rust)
 ; CHECK:   DW_TAG_namespace
 ; CHECK:     DW_TAG_enumeration_type
+; CHECK:     DW_AT_type (0x{{[0-9]+}} "u8")
 ; CHECK:     DW_AT_name	("E")
 ; CHECK:       DW_TAG_enumerator
 ; CHECK:       DW_TAG_enumerator
@@ -12,6 +13,8 @@
 ; CHECK:         NULL
 ; CHECK:       NULL
 ; CHECK:     NULL
+; CHECK:   DW_TAG_base_type
+; CHECK:     DW_AT_name ("u8")
 ; CHECK:   DW_TAG_pointer_type
 ; CHECK:   NULL
 

@DavidSpickett
Copy link
Collaborator Author

This will improve the experience of anyone using tools that can understand the attribute but for whatever reason are stuck building binaries containing v2 only.

How many of these folks exist, I've no idea. Not many I expect, but it was easy enough to implement so I figured why not.

@DavidSpickett
Copy link
Collaborator Author

@Michael137 I'll update the lldb test case if/when this lands. Unless your matrix is using strict-dwarf that is.

Copy link
Member

@Michael137 Michael137 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing!

Given GCC does this too and aligns with how the spec changed, this makes sense to me

@Michael137
Copy link
Member

@Michael137 I'll update the lldb test case if/when this lands. Unless your matrix is using strict-dwarf that is.

Can confirm the bot isn't using strict-dwarf

Copy link
Collaborator

@pogo59 pogo59 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This will improve the experience of anyone using tools that can understand the attribute but for whatever reason are stuck building binaries containing v2 only.

I know NVTPX is stuck on v2, but I'd hope this is not a problem for that target.

@DavidSpickett DavidSpickett merged commit ca715de into llvm:main Jul 12, 2024
8 checks passed
@DavidSpickett DavidSpickett deleted the attype branch July 12, 2024 08:42
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 12, 2024

LLVM Buildbot has detected a new failure on builder sanitizer-aarch64-linux-fuzzer running on sanitizer-buildbot11 while building llvm at step 2 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/159/builds/1954

Here is the relevant piece of the build log for the reference:

Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure)
...
+ JOBS=48
+ RunFuzzerTest libxml2-v2.9.2
+ echo @@@BUILD_STEP test libxml2-v2.9.2 fuzzer@@@
@@@BUILD_STEP test libxml2-v2.9.2 fuzzer@@@
+ ln -sf /b/sanitizer-aarch64-linux-fuzzer/build/llvm/lib/Fuzzer .
+ export FUZZING_ENGINE=fsanitize_fuzzer
+ FUZZING_ENGINE=fsanitize_fuzzer
++ pwd
+ /b/sanitizer-aarch64-linux-fuzzer/build/fuzzer-test-suite/build-and-test.sh libxml2-v2.9.2
Cloning into 'SRC'...
command timed out: 1200 seconds without output running [b'python', b'../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1285.290476
Step 9 (test libxml2-v2.9.2 fuzzer) failure: test libxml2-v2.9.2 fuzzer (failure)
@@@BUILD_STEP test libxml2-v2.9.2 fuzzer@@@
+ ln -sf /b/sanitizer-aarch64-linux-fuzzer/build/llvm/lib/Fuzzer .
+ export FUZZING_ENGINE=fsanitize_fuzzer
+ FUZZING_ENGINE=fsanitize_fuzzer
++ pwd
+ /b/sanitizer-aarch64-linux-fuzzer/build/fuzzer-test-suite/build-and-test.sh libxml2-v2.9.2
Cloning into 'SRC'...

command timed out: 1200 seconds without output running [b'python', b'../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1285.290476

@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 12, 2024

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building llvm at step 10 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/1711

Here is the relevant piece of the build log for the reference:

Step 10 (Add check check-offload) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
...
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug47654.cpp (776 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug49779.cpp (777 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/test_libc.cpp (778 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/wtime.c (779 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu :: offloading/bug49021.cpp (780 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu :: offloading/std_complex_arithmetic.cpp (781 of 786)
PASS: libomptarget :: amdgcn-amd-amdhsa :: offloading/bug49021.cpp (782 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/complex_reduction.cpp (783 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug49021.cpp (784 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp (785 of 786)
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1227.651405

@DavidSpickett
Copy link
Collaborator Author

Timeouts resolved themselves on the next runs.

DavidSpickett added a commit that referenced this pull request Jul 12, 2024
Since #98335 clang adds DW_AT_type, unless strict DWARF is requested.
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
…rict DWARF v2 mode (llvm#98335)

During testing of llvm#96202 we
found that when clang set to DWARF v2 was used to build the test file,
lldb could not tell that the unsigned enum type was in fact unsigned. So
it defaulted to signed and printed the wrong value.

The reason for this is that DWARFv2 does not include DW_AT_type in
DW_TAG_enumeration_type. This was added in DWARF v3:
"The enumeration type entry may also have a DW_AT_type attribute which
refers to the underlying data type used to implement the enumeration.

In C or C++, the underlying type will be the appropriate integral type
determined by the compiler from the properties of the enumeration
literal values."

I noticed that gcc does emit this attribute for DWARF v2 but not when
strict DWARF is requested (more details in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16063#c7).

This patch changes to clang to do the same. This will improve the
experience of anyone using tools that can understand the attribute but
for whatever reason are stuck building binaries containing v2 only.

You can see a current clang/gcc comparison here:
https://godbolt.org/z/eG9Kc9WGf

https://reviews.llvm.org/D42734 added the original code that emitted
this for >= v3 only.
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
Since llvm#98335 clang adds DW_AT_type, unless strict DWARF is requested.
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

5 participants