llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-arm Author: Jannik Voss (vossjannik) <details> <summary>Changes</summary> 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 `HasMicrosoftRecordLayout` to false. 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 fix explicitly sets `HasMicrosoftRecordLayout = true` in the constructors for: - MicrosoftARMleTargetInfo - MicrosoftARM64TargetInfo - MicrosoftMipsTargetInfo - UEFIX86_64TargetInfo Added a regression test verifying correct record layout generation for `armv7a-none-windows-eabi`. --- Full diff: https://github.com/llvm/llvm-project/pull/173843.diff 5 Files Affected: - (modified) clang/lib/Basic/Targets/AArch64.cpp (+1) - (modified) clang/lib/Basic/Targets/ARM.cpp (+1) - (modified) clang/lib/Basic/Targets/Mips.cpp (+1) - (modified) clang/lib/Basic/Targets/X86.h (+1) - (added) clang/test/Layout/ms-arm-virtual-inheritance.cpp (+22) ``````````diff diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp index ecd441be364c2..184856cf59e5c 100644 --- a/clang/lib/Basic/Targets/AArch64.cpp +++ b/clang/lib/Basic/Targets/AArch64.cpp @@ -1701,6 +1701,7 @@ MicrosoftARM64TargetInfo::MicrosoftARM64TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : WindowsARM64TargetInfo(Triple, Opts) { TheCXXABI.set(TargetCXXABI::Microsoft); + HasMicrosoftRecordLayout = true; } void MicrosoftARM64TargetInfo::getTargetDefines(const LangOptions &Opts, diff --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp index f21e9ebbc903a..bf73a3bce6f14 100644 --- a/clang/lib/Basic/Targets/ARM.cpp +++ b/clang/lib/Basic/Targets/ARM.cpp @@ -1482,6 +1482,7 @@ MicrosoftARMleTargetInfo::MicrosoftARMleTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : WindowsARMTargetInfo(Triple, Opts) { TheCXXABI.set(TargetCXXABI::Microsoft); + HasMicrosoftRecordLayout = true; } void MicrosoftARMleTargetInfo::getTargetDefines(const LangOptions &Opts, diff --git a/clang/lib/Basic/Targets/Mips.cpp b/clang/lib/Basic/Targets/Mips.cpp index a999d1410d254..4aeb75bcf9b19 100644 --- a/clang/lib/Basic/Targets/Mips.cpp +++ b/clang/lib/Basic/Targets/Mips.cpp @@ -353,6 +353,7 @@ MicrosoftMipsTargetInfo::MicrosoftMipsTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : WindowsMipsTargetInfo(Triple, Opts) { TheCXXABI.set(TargetCXXABI::Microsoft); + HasMicrosoftRecordLayout = true; } void MicrosoftMipsTargetInfo::getTargetDefines(const LangOptions &Opts, diff --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h index 922e32906cd04..0614f83041f21 100644 --- a/clang/lib/Basic/Targets/X86.h +++ b/clang/lib/Basic/Targets/X86.h @@ -845,6 +845,7 @@ class LLVM_LIBRARY_VISIBILITY UEFIX86_64TargetInfo // intend to offer flexibility by supporting choices that are not default in // Windows target in the future. this->TheCXXABI.set(TargetCXXABI::Microsoft); + this->HasMicrosoftRecordLayout = true; LongWidth = LongAlign = 32; DoubleAlign = LongLongAlign = 64; LongDoubleWidth = LongDoubleAlign = 64; 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= `````````` </details> https://github.com/llvm/llvm-project/pull/173843 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
