From 284dba29bb0d0c10f1032f019274c8667bf1da0e Mon Sep 17 00:00:00 2001 From: Owen Anderson Date: Thu, 5 Jun 2025 14:30:25 +1000 Subject: [PATCH] [CHERIoT] Fix divergence between frontend and backend as to whether CHERIoT counts as EABI. It was always intended to be treated as such, but when upstream added support for EABI, we missed updating the check in the frontend to handle CHERIoT as well. This manifests as a bug only in one very specific scenario: clang maintains compatibility with non-conforming versions of GCC that do not properly align doubles on the stack when targeting EABI, aligning them instead to 4 bytes. CHERIoT does not specifically care whether or not we are using the GCC-compatible version, but if the frontend and backend disagree on which version we are using, then the caller and callee will end up with incompatible stack layouts. This is because the caller layout is constructed by the backend, while the callee layout is constructed by the frontend. --- clang/lib/CodeGen/CodeGenModule.cpp | 2 + clang/lib/CodeGen/Targets/RISCV.cpp | 5 ++- clang/test/CodeGen/cheri/cheriot-variadic.c | 13 +++++++ llvm/lib/Target/RISCV/RISCVCallingConv.cpp | 6 ++- .../cheri/cheriot-variadic-double-align.ll | 38 +++++++++++++++++++ 5 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 llvm/test/CodeGen/RISCV/cheri/cheriot-variadic-double-align.ll diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 95e4a64115f65..cd708d22aee0e 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -244,6 +244,8 @@ createTargetCodeGenInfo(CodeGenModule &CGM) { else if (ABIStr.ends_with("d")) ABIFLen = 64; bool EABI = ABIStr.ends_with("e"); + EABI |= ABIStr == "cheriot"; + EABI |= ABIStr == "cheriot-baremetal"; return createRISCVTargetCodeGenInfo(CGM, XLen, ABIFLen, EABI); } diff --git a/clang/lib/CodeGen/Targets/RISCV.cpp b/clang/lib/CodeGen/Targets/RISCV.cpp index 9e14fd312e656..32218989aefae 100644 --- a/clang/lib/CodeGen/Targets/RISCV.cpp +++ b/clang/lib/CodeGen/Targets/RISCV.cpp @@ -587,7 +587,10 @@ RValue RISCVABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr, // 2×XLEN-bit alignment and size at most 2×XLEN bits like `long long`, // `unsigned long long` and `double` to have 4-byte alignment. This // behavior may be changed when RV32E/ILP32E is ratified. - if (EABI && XLen == 32) + // NOTE: Cheriot does not use the GCC workaround behavior. + StringRef TargetABI = getTarget().getABI(); + bool IsCheriot = TargetABI == "cheriot" || TargetABI == "cheriot-baremetal"; + if (EABI && XLen == 32 && !IsCheriot) TInfo.Align = std::min(TInfo.Align, CharUnits::fromQuantity(4)); bool IsSingleCapRecord = false; diff --git a/clang/test/CodeGen/cheri/cheriot-variadic.c b/clang/test/CodeGen/cheri/cheriot-variadic.c index 67c35db27337f..dd8d72a139361 100644 --- a/clang/test/CodeGen/cheri/cheriot-variadic.c +++ b/clang/test/CodeGen/cheri/cheriot-variadic.c @@ -14,9 +14,22 @@ typedef struct { extern int onward(void *, int, char *); +// CHECK-LABEL: @foo int foo(va_list ap) { // Make sure that we don't see a memcpy in address space zero! // CHECK-NOT: p0i8 bar_t x = va_arg(ap, bar_t); return onward(x.a, x.b, x.c); } + +// CHECK-LABEL: @bar +double bar(const char* c, ...) { +// Make sure that a double is dynamically aligned up to 8 bytes. +// CHECK: ptrmask + va_list ap; + va_start((ap), c); + int i1 = va_arg(ap, int); + double f = va_arg(ap, double); + va_end(ap); + return f; +} diff --git a/llvm/lib/Target/RISCV/RISCVCallingConv.cpp b/llvm/lib/Target/RISCV/RISCVCallingConv.cpp index c87370fc3f905..9c919907a6387 100644 --- a/llvm/lib/Target/RISCV/RISCVCallingConv.cpp +++ b/llvm/lib/Target/RISCV/RISCVCallingConv.cpp @@ -302,8 +302,12 @@ static bool CC_RISCVAssign2XLen(unsigned XLen, CCState &State, bool IsPureCapVar // Both halves must be passed on the stack, with proper alignment. // TODO: To be compatible with GCC's behaviors, we force them to have 4-byte // alignment. This behavior may be changed when RV32E/ILP32E is ratified. + // NOTE: Cheriot does not use the GCC workaround behavior. Align StackAlign(XLenInBytes); - if (!EABI || XLen != 32) + RISCVABI::ABI ABI = STI.getTargetABI(); + bool IsCheriot = (ABI == RISCVABI::ABI_CHERIOT) || + (ABI == RISCVABI::ABI_CHERIOT_BAREMETAL); + if (!EABI || XLen != 32 || IsCheriot) StackAlign = std::max(StackAlign, ArgFlags1.getNonZeroOrigAlign()); State.addLoc( CCValAssign::getMem(VA1.getValNo(), VA1.getValVT(), diff --git a/llvm/test/CodeGen/RISCV/cheri/cheriot-variadic-double-align.ll b/llvm/test/CodeGen/RISCV/cheri/cheriot-variadic-double-align.ll new file mode 100644 index 0000000000000..67708f3915265 --- /dev/null +++ b/llvm/test/CodeGen/RISCV/cheri/cheriot-variadic-double-align.ll @@ -0,0 +1,38 @@ +; RUN: llc --filetype=asm --mcpu=cheriot --mtriple=riscv32cheriot-unknown-cheriotrtos -target-abi cheriot -mattr=+xcheri,+cap-mode,+xcheriot %s -o - | FileCheck %s + +; Verify that varargs doubles are aligned to 8 bytes on the stack. + +target datalayout = "e-m:e-p:32:32-i64:64-n32-S128-pf200:64:64:64:32-A200-P200-G200" +target triple = "riscv32cheriot-unknown-cheriotrtos" + +declare hidden noundef double @_Z7va_testPKcz(ptr addrspace(200) nocapture readnone %c, ...) local_unnamed_addr addrspace(200) + +define hidden noundef double @_Z6helperv() local_unnamed_addr addrspace(200) { +; CHECK-LABEL: _Z6helperv: +; CHECK-NOT: csw [.*], 4(csp) +; CHECK-DAG: lui [[R0:.*]], 261939 +; CHECK-DAG: lui [[R1:.*]], 209715 +; CHECK-DAG: addi [[R0]], [[R0]], 819 +; CHECK-DAG: addi [[R1]], [[R1]], 819 +; CHECK-DAG: csw [[R0]], 12(csp) +; CHECK-DAG: csw [[R1]], 8(csp) +; CHECK-NOT: csw [.*], 4(csp) +entry: + %call = tail call noundef double (ptr addrspace(200), ...) @_Z7va_testPKcz(ptr addrspace(200) nonnull poison, i32 noundef 0, double noundef 1.200000e+00) + ret double %call +} + +!llvm.linker.options = !{} +!llvm.module.flags = !{!0, !1, !2, !4} +!llvm.ident = !{!5} + +!0 = !{i32 1, !"wchar_size", i32 2} +!1 = !{i32 1, !"target-abi", !"cheriot"} +!2 = !{i32 6, !"riscv-isa", !3} +!3 = !{!"rv32e2p0_m2p0_c2p0_zmmul1p0_xcheri0p0_xcheriot1p0"} +!4 = !{i32 8, !"SmallDataLimit", i32 0} +!5 = !{!"clang version 20.1.3 (git@github.com:resistor/llvm-project-1.git 592752fee8b25c925a65fb40eeb8a496f1b0ee2c)"} +!6 = !{!7, !7, i64 0} +!7 = !{!"double", !8, i64 0} +!8 = !{!"omnipotent char", !9, i64 0} +!9 = !{!"Simple C++ TBAA"}