https://github.com/vossjannik updated https://github.com/llvm/llvm-project/pull/173846
>From b8a20746172387f9d3ba4109269ba28c39fb7510 Mon Sep 17 00:00:00 2001 From: Jannik Voss <[email protected]> Date: Tue, 23 Dec 2025 14:04:26 +0100 Subject: [PATCH] [Clang][Targets] Fix crash on ARM/AArch64 Windows EABI targets with virtual inheritance This patch fixes a regression introduced in beea2a941470 where targets using the Microsoft C++ ABI but not identified as "MSVC environments" (such as `arm-none-windows-eabi` or UEFI) were defaulting to Itanium record layout. This caused Clang to use the Itanium record layout builder for these targets while still using Microsoft ABI semantics elsewhere, leading to assertion failures and crashes when processing classes with virtual inheritance. The root cause was that `HasMicrosoftRecordLayout` was cached in the `TargetInfo` base constructor before derived classes had a chance to set `TheCXXABI` to Microsoft. This fix changes `hasMicrosoftRecordLayout()` to dynamically check `TheCXXABI.isMicrosoft()`, ensuring any target that sets Microsoft C++ ABI automatically gets Microsoft record layout without requiring explicit synchronization. It also renames the backing field to `ForceMicrosoftRecordLayout` to clarify it is now only used as an override for CUDA/HIP device compilation where the device ABI is Itanium but must match the host's Microsoft layout. This correctly handles layout for: - MicrosoftARMleTargetInfo - MicrosoftARM64TargetInfo - MicrosoftMipsTargetInfo - UEFIX86_64TargetInfo Added a regression test verifying correct record layout generation for `armv7a-none-windows-eabi`. Fixes #172991 --- clang/include/clang/Basic/TargetInfo.h | 12 ++++++++-- clang/lib/Basic/TargetInfo.cpp | 6 ++--- .../Layout/ms-arm-virtual-inheritance.cpp | 22 +++++++++++++++++++ 3 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 clang/test/Layout/ms-arm-virtual-inheritance.cpp diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 4ff77bb64cf1c..86feef3d80c75 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -291,7 +291,10 @@ class TargetInfo : public TransferrableTargetInfo, std::optional<llvm::Triple> DarwinTargetVariantTriple; - bool HasMicrosoftRecordLayout = false; + /// Override flag to force Microsoft record layout, used when device target + /// (e.g., CUDA/HIP) must match host's Microsoft layout despite using a + /// different C++ ABI. + bool ForceMicrosoftRecordLayout = false; // TargetInfo Constructor. Default initializes all fields. TargetInfo(const llvm::Triple &T); @@ -1905,7 +1908,12 @@ class TargetInfo : public TransferrableTargetInfo, virtual void setAuxTarget(const TargetInfo *Aux) {} - bool hasMicrosoftRecordLayout() const { return HasMicrosoftRecordLayout; } + /// Returns true if Microsoft record layout should be used. + /// This is true when either the C++ ABI is Microsoft, or when forced + /// for device compilation to match a Microsoft host. + bool hasMicrosoftRecordLayout() const { + return ForceMicrosoftRecordLayout || TheCXXABI.isMicrosoft(); + } /// Whether target allows debuginfo types for decl only variables/functions. virtual bool allowDebugInfoForExternalRef() const { return false; } diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp index 794621c4b3e1f..28c3131029707 100644 --- a/clang/lib/Basic/TargetInfo.cpp +++ b/clang/lib/Basic/TargetInfo.cpp @@ -179,8 +179,6 @@ TargetInfo::TargetInfo(const llvm::Triple &T) : Triple(T) { ? TargetCXXABI::Microsoft : TargetCXXABI::GenericItanium); - HasMicrosoftRecordLayout = TheCXXABI.isMicrosoft(); - // Default to an empty address space map. AddrSpaceMap = &DefaultAddrSpaceMap; UseAddrSpaceMapMangling = false; @@ -559,8 +557,8 @@ void TargetInfo::adjust(DiagnosticsEngine &Diags, LangOptions &Opts, AddrSpaceMap = &FakeAddrSpaceMap; // Check if it's CUDA device compilation; ensure layout consistency with host. - if (Opts.CUDA && Opts.CUDAIsDevice && Aux && !HasMicrosoftRecordLayout) - HasMicrosoftRecordLayout = Aux->getCXXABI().isMicrosoft(); + if (Opts.CUDA && Opts.CUDAIsDevice && Aux && !ForceMicrosoftRecordLayout) + ForceMicrosoftRecordLayout = Aux->getCXXABI().isMicrosoft(); } bool TargetInfo::initFeatureMap( diff --git a/clang/test/Layout/ms-arm-virtual-inheritance.cpp b/clang/test/Layout/ms-arm-virtual-inheritance.cpp new file mode 100644 index 0000000000000..32fc9986acb08 --- /dev/null +++ b/clang/test/Layout/ms-arm-virtual-inheritance.cpp @@ -0,0 +1,22 @@ +// RUN: %clang_cc1 -emit-llvm-only -triple armv7a-none-windows-eabi -fdump-record-layouts %s 2>/dev/null | FileCheck %s +// RUN: %clang_cc1 -emit-llvm-only -triple thumbv7-none-windows-eabihf -fdump-record-layouts %s 2>/dev/null | FileCheck %s + +// Verify that ARM Windows with eabi environment correctly uses Microsoft +// record layout for classes with virtual inheritance. +// This is a regression test for a crash caused by incorrect record layout +// selection when TheCXXABI is Microsoft but HasMicrosoftRecordLayout was false. + +class A {}; +class B : virtual A {}; +B b; + +// CHECK: *** Dumping AST Record Layout +// CHECK: *** Dumping AST Record Layout +// CHECK: 0 | class B +// CHECK-NEXT: 0 | (B vbtable pointer) +// CHECK-NEXT: 4 | class A (virtual base) (empty) +// CHECK-NEXT: | [sizeof=4, align=4, +// CHECK-NEXT: | nvsize=4, nvalign=4] + +// Microsoft record layout should NOT print dsize= (only Itanium layout does) +// CHECK-NOT: dsize= _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
