[clang][DebugInfo] Disable VTable debug info (#130255) on COFF platforms #150938

kikairoya · 2025-07-28T13:05:55Z

On COFF platform, d1b0cbf generates a debug info linked with VTable regardless definition is present or not. If that VTable ends up implicitly dllimported from another DLL, ld.bfd produces a runtime pseudo relocation for it (LLD doesn't, since d17db60). If the debug section is stripped, the runtime pseudo relocation points to memory space outside of the module, causing an access violation.
For the release branch, we simply disable VTable debug info on COFF platform to avoid this problem.

llvmbot · 2025-07-28T13:06:26Z

@llvm/pr-subscribers-clang-modules
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-debuginfo

Author: Tomohiro Kashiwada (kikairoya)

Changes

On COFF platform, d1b0cbf generates a debug info linked with VTable even if that is dllimport-ed. That causes an access violation while performing runtime pseudo-relocation if the debug section is stripped.
For the release branch, we simply disable VTable debug info on COFF platform to avoid this problem.

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

2 Files Affected:

(modified) clang/lib/CodeGen/CGDebugInfo.cpp (+2-1)
(modified) clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp (+8-3)

diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 0dde045453e3a..ec28bd259e8e1 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2645,7 +2645,8 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) {
 // existing information in the DWARF. The type is assumed to be 'void *'.
 void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable,
                                    const CXXRecordDecl *RD) {
-  if (!CGM.getTarget().getCXXABI().isItaniumFamily())
+  if (!CGM.getTarget().getCXXABI().isItaniumFamily() ||
+      CGM.getTarget().getTriple().isOSBinFormatCOFF())
     return;
 
   ASTContext &Context = CGM.getContext();
diff --git a/clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp b/clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp
index 249586f5991f1..b24ece1598327 100644
--- a/clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp
+++ b/clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp
@@ -1,5 +1,3 @@
-// REQUIRES: target={{x86_64.*-linux.*}}
-
 // Simple inheritance case:
 // For CBase and CDerived we check:
 // - Generation of their vtables (including attributes).
@@ -30,13 +28,20 @@ int main() {
   return 0;
 }
 
-// RUN: %clang --target=x86_64-linux -Xclang -disable-O0-optnone -Xclang -disable-llvm-passes -emit-llvm -S -g %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -mrelocation-model pic -pic-is-pie -debug-info-kind=limited -dwarf-version=5 -disable-O0-optnone -disable-llvm-passes %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -mrelocation-model pic -pic-is-pie -debug-info-kind=limited -dwarf-version=5 -disable-O0-optnone -disable-llvm-passes %s -o - | FileCheck %s --check-prefix=COFF
 
 // CHECK: $_ZTVN3NSP5CBaseE = comdat any
 // CHECK: $_ZTV8CDerived = comdat any
 
 // CHECK: @_ZTVN3NSP5CBaseE = linkonce_odr {{dso_local|hidden}} unnamed_addr constant {{.*}}, comdat, align 8, !dbg [[BASE_VTABLE_VAR:![0-9]*]]
 // CHECK: @_ZTV8CDerived = linkonce_odr {{dso_local|hidden}} unnamed_addr constant {{.*}}, comdat, align 8, !dbg [[DERIVED_VTABLE_VAR:![0-9]*]]
+// COFF: @_ZTVN3NSP5CBaseE = linkonce_odr {{dso_local|hidden}} unnamed_addr constant {{.*}}, comdat, align 8
+// COFF-NOT: !dbg
+// COFF-SAME: {{$}}
+// COFF: @_ZTV8CDerived = linkonce_odr {{dso_local|hidden}} unnamed_addr constant {{.*}}, comdat, align 8
+// COFF-NOT: !dbg
+// COFF-SAME: {{$}}
 
 // CHECK: [[BASE_VTABLE_VAR]] = !DIGlobalVariableExpression(var: [[BASE_VTABLE:![0-9]*]], expr: !DIExpression())
 // CHECK-NEXT: [[BASE_VTABLE]] = distinct !DIGlobalVariable(name: "_vtable$", linkageName: "_ZTVN3NSP5CBaseE"

llvmbot · 2025-07-28T13:06:26Z

@llvm/pr-subscribers-clang

Author: Tomohiro Kashiwada (kikairoya)

Changes

On COFF platform, d1b0cbf generates a debug info linked with VTable even if that is dllimport-ed. That causes an access violation while performing runtime pseudo-relocation if the debug section is stripped.
For the release branch, we simply disable VTable debug info on COFF platform to avoid this problem.

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

2 Files Affected:

(modified) clang/lib/CodeGen/CGDebugInfo.cpp (+2-1)
(modified) clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp (+8-3)

diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 0dde045453e3a..ec28bd259e8e1 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2645,7 +2645,8 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) {
 // existing information in the DWARF. The type is assumed to be 'void *'.
 void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable,
                                    const CXXRecordDecl *RD) {
-  if (!CGM.getTarget().getCXXABI().isItaniumFamily())
+  if (!CGM.getTarget().getCXXABI().isItaniumFamily() ||
+      CGM.getTarget().getTriple().isOSBinFormatCOFF())
     return;
 
   ASTContext &Context = CGM.getContext();
diff --git a/clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp b/clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp
index 249586f5991f1..b24ece1598327 100644
--- a/clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp
+++ b/clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp
@@ -1,5 +1,3 @@
-// REQUIRES: target={{x86_64.*-linux.*}}
-
 // Simple inheritance case:
 // For CBase and CDerived we check:
 // - Generation of their vtables (including attributes).
@@ -30,13 +28,20 @@ int main() {
   return 0;
 }
 
-// RUN: %clang --target=x86_64-linux -Xclang -disable-O0-optnone -Xclang -disable-llvm-passes -emit-llvm -S -g %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -mrelocation-model pic -pic-is-pie -debug-info-kind=limited -dwarf-version=5 -disable-O0-optnone -disable-llvm-passes %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -mrelocation-model pic -pic-is-pie -debug-info-kind=limited -dwarf-version=5 -disable-O0-optnone -disable-llvm-passes %s -o - | FileCheck %s --check-prefix=COFF
 
 // CHECK: $_ZTVN3NSP5CBaseE = comdat any
 // CHECK: $_ZTV8CDerived = comdat any
 
 // CHECK: @_ZTVN3NSP5CBaseE = linkonce_odr {{dso_local|hidden}} unnamed_addr constant {{.*}}, comdat, align 8, !dbg [[BASE_VTABLE_VAR:![0-9]*]]
 // CHECK: @_ZTV8CDerived = linkonce_odr {{dso_local|hidden}} unnamed_addr constant {{.*}}, comdat, align 8, !dbg [[DERIVED_VTABLE_VAR:![0-9]*]]
+// COFF: @_ZTVN3NSP5CBaseE = linkonce_odr {{dso_local|hidden}} unnamed_addr constant {{.*}}, comdat, align 8
+// COFF-NOT: !dbg
+// COFF-SAME: {{$}}
+// COFF: @_ZTV8CDerived = linkonce_odr {{dso_local|hidden}} unnamed_addr constant {{.*}}, comdat, align 8
+// COFF-NOT: !dbg
+// COFF-SAME: {{$}}
 
 // CHECK: [[BASE_VTABLE_VAR]] = !DIGlobalVariableExpression(var: [[BASE_VTABLE:![0-9]*]], expr: !DIExpression())
 // CHECK-NEXT: [[BASE_VTABLE]] = distinct !DIGlobalVariable(name: "_vtable$", linkageName: "_ZTVN3NSP5CBaseE"

mstorsjo

LGTM, thanks - this looks reasonable to me, but I'd prefer to have an explicit ack from @jmorse still before merging.

Minor nit, we could adjust the commit message (PR description), saying "even if that may be dllimported", instead of "is dllimported". If it has an explicit dllimport, iirc this issue doesn't hit at all - it's the "maybe dllimported" case which is problematic here.

Longer term, as mentioned in #149639, we could consider just emitting this debug info in the object files that actually define the vtable, which could allow removing this restriction (to produce such debug info on COFF targets too).

mstorsjo · 2025-07-28T13:33:54Z

clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp

@@ -1,5 +1,3 @@
-// REQUIRES: target={{x86_64.*-linux.*}}


FWIW, I've verified that this test does indeed run correctly even if the x86 target isn't enabled in the build of llvm/clang - so removing the REQUIRES line should be fine here.

(Not sure if this is related to changing from %clang to %clang_cc1, or if the original test also actually didn't require the x86 target to be available.)

kikairoya · 2025-07-28T14:13:14Z

Former is obvious, but latter uses %itanium_abi_triple then output relies on default target. Is it okay to add UNSUPPORTED: target={{.*(windows|mingw|cyg).*}}, or should I separate by target?

I've diverged with '%if' directive.

kikairoya · 2025-07-28T14:15:46Z

Minor nit, we could adjust the commit message (PR description), saying "even if that may be dllimported", instead of "is dllimported". If it has an explicit dllimport, iirc this issue doesn't hit at all - it's the "maybe dllimported" case which is problematic here.

Updated. I hope I have written precisely.

mstorsjo · 2025-07-28T14:22:52Z

Minor nit, we could adjust the commit message (PR description), saying "even if that may be dllimported", instead of "is dllimported". If it has an explicit dllimport, iirc this issue doesn't hit at all - it's the "maybe dllimported" case which is problematic here.

Updated. I hope I have written precisely.

Thanks! I took the liberty to further reword it a bit more, to clarify the scope - let me know if you disagree with the wording.

kikairoya · 2025-07-28T14:37:09Z

Thanks! I took the liberty to further reword it a bit more, to clarify the scope - let me know if you disagree with the wording.

Thank you for your follow-up. I really appreciate it.

mstorsjo · 2025-07-29T08:36:52Z

This now seems to pass tests just fine, on both linux and mingw targets. Can @jmorse ack the fix (and the test modifications), or is it enough with the comment in #130255 (comment) saying this is what we should do?

jmorse

LGTM with a minor test question to confirm my understanding.

jmorse · 2025-07-29T12:23:19Z

clang/test/Modules/ExtDebugInfo.cpp

+
+// There is a full definition of the type available in the module.
+// CHECKCOFF: !DICompositeType(tag: DW_TAG_structure_type, name: "Virtual",
+// CHECKCOFF-SAME:             DIFlagFwdDecl
+// CHECKCOFF-SAME:             identifier: "_ZTS7Virtual")


If I'm reading this correctly, the DICompositeType is identical to the ELF DICompositeType earlier, but presumably it's in a different location in the output because it's no longer attached to a global variable?

If that's true, then this test coverage is somewhat indirectly covering that behaviour -- but I think it's fine because the vtable-debug-info-inheritance-simple.cpp is precisely checking the behaviour.

To be honest, I'm not quite sure why this change happens, as this diff is basically just a revert.
But your assumption sounds reasonable to me.

As you said, vtable-debug-info-inheritance-simple.cpp asserts that MinGW doesn't emit the debug info.

tru · 2025-08-01T07:02:01Z

Hm this is unfortunate. The script messed up because of the merge commit - I will have to figure that out. Meanwhile I merged the squashed version to the release branch as 8c3ef23

llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. debuginfo labels Jul 28, 2025

kikairoya mentioned this pull request Jul 28, 2025

clang 21.1.0-rc1 creates broken executables on Cygwin/MinGW when linked with GNU ld.bfd #149639

Open

mstorsjo requested review from mstorsjo, jmorse, jeremyd2019 and CarlosAlbertoEnciso July 28, 2025 13:11

mstorsjo approved these changes Jul 28, 2025

View reviewed changes

This comment was marked as resolved.

Sign in to view

llvmbot added the clang:modules C++20 modules and Clang Header Modules label Jul 28, 2025

tru added this to the LLVM 21.x Release milestone Jul 29, 2025

github-project-automation bot added this to LLVM Release Status Jul 29, 2025

github-project-automation bot moved this to Needs Triage in LLVM Release Status Jul 29, 2025

tru moved this from Needs Triage to Needs Review in LLVM Release Status Jul 29, 2025

jmorse approved these changes Jul 29, 2025

View reviewed changes

github-project-automation bot moved this from Needs Review to Needs Merge in LLVM Release Status Jul 29, 2025

tru force-pushed the disable-vtable-debuginfo-coff branch from cd69c5e to f6a0643 Compare August 1, 2025 06:57

tru closed this Aug 1, 2025

tru force-pushed the disable-vtable-debuginfo-coff branch from f6a0643 to 4ca9a4b Compare August 1, 2025 06:57

tru moved this from Needs Merge to Done in LLVM Release Status Aug 1, 2025

mstorsjo mentioned this pull request Aug 4, 2025

[clang][DebugInfo] Disable VTable debug info (#130255) on COFF platforms #151684

Merged

kikairoya deleted the disable-vtable-debuginfo-coff branch August 6, 2025 21:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang][DebugInfo] Disable VTable debug info (#130255) on COFF platforms #150938

[clang][DebugInfo] Disable VTable debug info (#130255) on COFF platforms #150938

Uh oh!

kikairoya commented Jul 28, 2025 •

edited by mstorsjo

Loading

Uh oh!

llvmbot commented Jul 28, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jul 28, 2025

Uh oh!

mstorsjo left a comment

Uh oh!

mstorsjo Jul 28, 2025

Uh oh!

This comment was marked as resolved.

kikairoya commented Jul 28, 2025

Uh oh!

kikairoya commented Jul 28, 2025

Uh oh!

mstorsjo commented Jul 28, 2025

Uh oh!

kikairoya commented Jul 28, 2025

Uh oh!

mstorsjo commented Jul 29, 2025

Uh oh!

jmorse left a comment

Uh oh!

jmorse Jul 29, 2025

Uh oh!

kikairoya Jul 29, 2025

Uh oh!

tru commented Aug 1, 2025

Uh oh!

Uh oh!

[clang][DebugInfo] Disable VTable debug info (#130255) on COFF platforms #150938

[clang][DebugInfo] Disable VTable debug info (#130255) on COFF platforms #150938

Uh oh!

Conversation

kikairoya commented Jul 28, 2025 • edited by mstorsjo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 28, 2025

Uh oh!

mstorsjo left a comment

Choose a reason for hiding this comment

Uh oh!

mstorsjo Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

kikairoya commented Jul 28, 2025

Uh oh!

kikairoya commented Jul 28, 2025

Uh oh!

mstorsjo commented Jul 28, 2025

Uh oh!

kikairoya commented Jul 28, 2025

Uh oh!

mstorsjo commented Jul 29, 2025

Uh oh!

jmorse left a comment

Choose a reason for hiding this comment

Uh oh!

jmorse Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

kikairoya Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

tru commented Aug 1, 2025

Uh oh!

Uh oh!

kikairoya commented Jul 28, 2025 •

edited by mstorsjo

Loading

llvmbot commented Jul 28, 2025 •

edited

Loading