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

[clang][DebugInfo] Emit DW_AT_object_pointer on function definitions with explicit this #122897

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

Michael137
Copy link
Member

@Michael137 Michael137 commented Jan 14, 2025

We currently don't emit DW_AT_object_pointer on function declarations or definitions. GCC suffers from the same issue: https://godbolt.org/z/h4jeT54G5

Fixing this will help LLDB in identifying static vs. non-static member functions (see #120856).

If I interpreted the DWARFv5 spec correctly, it doesn't mandate this attribute be present only for implicit object parameters:

If the member function entry describes a non-static member function,
then that entry has a DW_AT_object_pointer attribute whose value is a reference to
the formal parameter entry that corresponds to the object for which the
function is called.

That parameter also has a DW_AT_artificial attribute whose value is true.

This patch attaches the DW_AT_object_pointer for function defintions. The declarations will be handled in a separate patch.

The part about DW_AT_artificial seems overly restrictive, and not true for explicit object parameters. We probably should relax this part of the DWARF spec.

Partially fixes #120974

…with explicit `this`

We currently don't emit `DW_AT_object_pointer` on function
declarations or definitions. The DWARFv5 spec doesn't mandate
this attribute be present *only* for implicit object parameters:
```
If the member function entry describes a non-static member function,
then that entry has a DW_AT_object_pointer attribute whose value is a reference to
the formal parameter entry that corresponds to the object for which the
function is called.

That parameter also has a DW_AT_artificial attribute whose value is true.
```

The part about `DW_AT_artificial` seems overly restrictive, and not true for
explicit object parameters. We probably should relax this part of the DWARF spec.

This will help LLDB identify static vs. non-static member functions (see
llvm#120856).

GCC suffers from the same issue: https://godbolt.org/z/h4jeT54G5

Partially fixes llvm#120974
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen debuginfo labels Jan 14, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 14, 2025

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-clang

Author: Michael Buch (Michael137)

Changes

We currently don't emit DW_AT_object_pointer on function declarations or definitions. GCC suffers from the same issue: https://godbolt.org/z/h4jeT54G5

If I interpreted the DWARFv5 spec correctly, it doesn't mandate this attribute be present only for implicit object parameters:

If the member function entry describes a non-static member function,
then that entry has a DW_AT_object_pointer attribute whose value is a reference to
the formal parameter entry that corresponds to the object for which the
function is called.

That parameter also has a DW_AT_artificial attribute whose value is true.

This patch attaches the DW_AT_object_pointer for function defintions. The declarations will be handled in a separate patch.

The part about DW_AT_artificial seems overly restrictive, and not true for explicit object parameters. We probably should relax this part of the DWARF spec.

This will help LLDB identify static vs. non-static member functions (see #120856).

Partially fixes #120974


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+3)
  • (added) clang/test/CodeGenCXX/debug-info-object-pointer.cpp (+29)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index d7e5e95b7873a0..f9cba414dcfe2c 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4829,6 +4829,9 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const VarDecl *VD,
     if (IPD->getParameterKind() == ImplicitParamKind::CXXThis ||
         IPD->getParameterKind() == ImplicitParamKind::ObjCSelf)
       Flags |= llvm::DINode::FlagObjectPointer;
+  } else if (const auto *PVD = dyn_cast<ParmVarDecl>(VD)) {
+    if (PVD->isExplicitObjectParameter())
+      Flags |= llvm::DINode::FlagObjectPointer;
   }
 
   // Note: Older versions of clang used to emit byval references with an extra
diff --git a/clang/test/CodeGenCXX/debug-info-object-pointer.cpp b/clang/test/CodeGenCXX/debug-info-object-pointer.cpp
new file mode 100644
index 00000000000000..594d4da791ee84
--- /dev/null
+++ b/clang/test/CodeGenCXX/debug-info-object-pointer.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -x c++ -std=c++23 -debug-info-kind=limited -emit-llvm < %s | FileCheck %s
+
+// CHECK: !DISubprogram(name: "bar",
+// CHECK-SAME:          flags: DIFlagPrototyped
+// CHECK: !DIDerivedType(tag: DW_TAG_pointer_type
+// CHECK-SAME:           flags: DIFlagArtificial | DIFlagObjectPointer
+//
+// // FIXME: DIFlagObjectPointer not attached to the explicit object
+// // argument in the subprogram declaration.
+// CHECK: !DISubprogram(name: "explicit_this",
+//                      flags: DIFlagPrototyped
+// CHECK-NOT: DIFlagObjectPointer
+// CHECK-NOT: DIFlagArtificial
+//
+// CHECK: !DILocalVariable(name: "this", arg: 1
+// CHECK-SAME:             flags: DIFlagArtificial | DIFlagObjectPointer
+//
+// CHECK-NOT: DIFlagArtificial
+// CHECK: !DILocalVariable(arg: 1, {{.*}}, flags: DIFlagObjectPointer)
+
+struct Foo {
+  void bar() {}
+  void explicit_this(this Foo &&) {}
+};
+
+void f() {
+  Foo{}.bar();
+  Foo{}.explicit_this();
+}

@Michael137
Copy link
Member Author

@dwblaikie @adrian-prantl any thoughts on adjusting the DWARF spec to allow explicit this parameters to be non-artificial but still be valid DW_AT_object_pointers?

@adrian-prantl
Copy link
Collaborator

@dwblaikie @adrian-prantl any thoughts on adjusting the DWARF spec to allow explicit this parameters to be non-artificial but still be valid DW_AT_object_pointers?

I would suggest posting an issue to dwarf-discuss that just turns that into

Many languages make the object pointer an implicit parameter with no syntax. In that case the parameter should have a DW_AT_artificial attribute whose value is true.

and give an example. Also maybe of a language like Python where self is quite explicit.

@Michael137 Michael137 merged commit 3d24c77 into llvm:main Jan 14, 2025
12 checks passed
@Michael137 Michael137 deleted the clang/explicit-this-object-pointer branch January 14, 2025 21:12
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 14, 2025

LLVM Buildbot has detected a new failure on builder openmp-offload-amdgpu-runtime running on omp-vega20-0 while building clang at step 7 "Add check check-offload".

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

Here is the relevant piece of the build log for the reference
Step 7 (Add check check-offload) failure: test (failure)
******************** TEST 'libomptarget :: amdgcn-amd-amdhsa :: offloading/pgo1.c' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 1
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp    -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib  -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/offloading/pgo1.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/pgo1.c.tmp /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a -fprofile-instr-generate      -Xclang "-fprofile-instrument=clang"
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/offloading/pgo1.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/pgo1.c.tmp /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a -fprofile-instr-generate -Xclang -fprofile-instrument=clang
# note: command had no output on stdout or stderr
# RUN: at line 3
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/pgo1.c.tmp 2>&1 | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/offloading/pgo1.c      --check-prefix="CLANG-PGO"
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/pgo1.c.tmp
# note: command had no output on stdout or stderr
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/offloading/pgo1.c --check-prefix=CLANG-PGO
# .---command stderr------------
# | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/offloading/pgo1.c:32:20: error: CLANG-PGO-NEXT: expected string not found in input
# | // CLANG-PGO-NEXT: [ 0 11 20 ]
# |                    ^
# | <stdin>:3:28: note: scanning from here
# | ======== Counters =========
# |                            ^
# | <stdin>:4:1: note: possible intended match here
# | [ 0 13 20 ]
# | ^
# | 
# | Input file: <stdin>
# | Check file: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/offloading/pgo1.c
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | <<<<<<
# |            1: ======= GPU Profile ======= 
# |            2: Target: amdgcn-amd-amdhsa 
# |            3: ======== Counters ========= 
# | next:32'0                                X error: no match found
# |            4: [ 0 13 20 ] 
# | next:32'0     ~~~~~~~~~~~~
# | next:32'1     ?            possible intended match
# |            5: [ 10 ] 
# | next:32'0     ~~~~~~~
# |            6: [ 20 ] 
# | next:32'0     ~~~~~~~
# |            7: ========== Data =========== 
# | next:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |            8: { 10367987278331647071 4749112401 0xffffffffffffffd8 0x0 0x0 0x0 3 [...] 0 } 
# | next:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |            9: { 3666282617048535130 24 0xffffffffffffffb0 0x0 0x0 0x0 1 [...] 0 } 
# | next:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |            .
...

@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 14, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-win-x-armv7l running on as-builder-1 while building clang at step 15 "test-check-cxx-armv7-unknown-linux-gnueabihf".

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

Here is the relevant piece of the build log for the reference
Step 15 (test-check-cxx-armv7-unknown-linux-gnueabihf) failure: Test just built components: check-cxx-armv7-unknown-linux-gnueabihf completed (failure)
******************** TEST 'llvm-libc++-static.cfg.in :: std/ranges/range.factories/range.iota.view/views_iota.pass.cpp' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# COMPILED WITH
C:/buildbot/as-builder-1/x-armv7l/build/./bin/clang++.exe C:\buildbot\as-builder-1\x-armv7l\llvm-project\libcxx\test\std\ranges\range.factories\range.iota.view\views_iota.pass.cpp -pthread --target=armv7-unknown-linux-gnueabihf -nostdinc++ -I C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libcxx/test-suite-install/include/c++/v1 -I C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libcxx/test-suite-install/include/armv7-unknown-linux-gnueabihf/c++/v1 -I C:/buildbot/as-builder-1/x-armv7l/llvm-project/libcxx/test/support -std=c++26 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier -Wdeprecated-copy -Wdeprecated-copy-dtor -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions -Wno-unknown-pragmas -Wno-pass-failed -Wno-mismatched-new-delete -Wno-redundant-move -Wno-self-move -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_NONE -Werror=thread-safety -Wuser-defined-warnings  -lc++experimental -nostdlib++ -L C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libcxx/test-suite-install/lib/armv7-unknown-linux-gnueabihf -lc++ -lc++abi -latomic -o C:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-armv7-unknown-linux-gnueabihf-bins\libcxx\test\std\ranges\range.factories\range.iota.view\Output\views_iota.pass.cpp.dir\t.tmp.exe
# executed command: C:/buildbot/as-builder-1/x-armv7l/build/./bin/clang++.exe 'C:\buildbot\as-builder-1\x-armv7l\llvm-project\libcxx\test\std\ranges\range.factories\range.iota.view\views_iota.pass.cpp' -pthread --target=armv7-unknown-linux-gnueabihf -nostdinc++ -I C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libcxx/test-suite-install/include/c++/v1 -I C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libcxx/test-suite-install/include/armv7-unknown-linux-gnueabihf/c++/v1 -I C:/buildbot/as-builder-1/x-armv7l/llvm-project/libcxx/test/support -std=c++26 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier -Wdeprecated-copy -Wdeprecated-copy-dtor -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions -Wno-unknown-pragmas -Wno-pass-failed -Wno-mismatched-new-delete -Wno-redundant-move -Wno-self-move -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_NONE -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libcxx/test-suite-install/lib/armv7-unknown-linux-gnueabihf -lc++ -lc++abi -latomic -o 'C:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-armv7-unknown-linux-gnueabihf-bins\libcxx\test\std\ranges\range.factories\range.iota.view\Output\views_iota.pass.cpp.dir\t.tmp.exe'
# EXECUTED AS
"C:/Python310/python.exe" "C:/buildbot/as-builder-1/x-armv7l/llvm-project/libcxx/utils/ssh.py" [email protected] --execdir C:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-armv7-unknown-linux-gnueabihf-bins\libcxx\test\std\ranges\range.factories\range.iota.view\Output\views_iota.pass.cpp.dir --  C:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-armv7-unknown-linux-gnueabihf-bins\libcxx\test\std\ranges\range.factories\range.iota.view\Output\views_iota.pass.cpp.dir\t.tmp.exe
# executed command: C:/Python310/python.exe C:/buildbot/as-builder-1/x-armv7l/llvm-project/libcxx/utils/ssh.py [email protected] --execdir 'C:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-armv7-unknown-linux-gnueabihf-bins\libcxx\test\std\ranges\range.factories\range.iota.view\Output\views_iota.pass.cpp.dir' -- 'C:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-armv7-unknown-linux-gnueabihf-bins\libcxx\test\std\ranges\range.factories\range.iota.view\Output\views_iota.pass.cpp.dir\t.tmp.exe'
# .---command stderr------------
# | kex_exchange_identification: read: Software caused connection abort
# | banner exchange: Connection to 10.1.1.144 port 22: Software caused connection abort
# | Traceback (most recent call last):
# |   File "C:\buildbot\as-builder-1\x-armv7l\llvm-project\libcxx\utils\ssh.py", line 145, in <module>
# |     exit(main())
# |   File "C:\buildbot\as-builder-1\x-armv7l\llvm-project\libcxx\utils\ssh.py", line 141, in main
# |     runCommand(ssh("rm -r {}".format(tmp)), check=True)
# |   File "C:\buildbot\as-builder-1\x-armv7l\llvm-project\libcxx\utils\ssh.py", line 57, in runCommand
# |     return subprocess.run(command, *args_, **kwargs)
# |   File "c:\python310\lib\subprocess.py", line 524, in run
# |     raise CalledProcessError(retcode, process.args,
# | subprocess.CalledProcessError: Command '['ssh', '-oBatchMode=yes', '[email protected]', 'rm -r /tmp/libcxx.2F1Cfvlwgv']' returned non-zero exit status 255.
# `-----------------------------
# error: command failed with exit status: 1

--

********************


Michael137 added a commit that referenced this pull request Jan 17, 2025
… with explicit `this` (#122928)

In #122897 we started attaching
`DW_AT_object_pointer` to function definitions. This patch does the same
but for function declarations (which we do for implicit object pointers
already).

Fixes #120974
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Jan 17, 2025
…eclarations with explicit `this` (#122928)

In llvm/llvm-project#122897 we started attaching
`DW_AT_object_pointer` to function definitions. This patch does the same
but for function declarations (which we do for implicit object pointers
already).

Fixes llvm/llvm-project#120974
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category debuginfo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DebugInfo] DW_AT_object_pointer not emitted for explicit object parameter
4 participants