Skip to content

Commit 8896e09

Browse files
committed
[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.
1 parent 33cdae8 commit 8896e09

File tree

5 files changed

+62
-2
lines changed

5 files changed

+62
-2
lines changed

clang/lib/CodeGen/CodeGenModule.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,8 @@ createTargetCodeGenInfo(CodeGenModule &CGM) {
244244
else if (ABIStr.ends_with("d"))
245245
ABIFLen = 64;
246246
bool EABI = ABIStr.ends_with("e");
247+
EABI |= ABIStr == "cheriot";
248+
EABI |= ABIStr == "cheriot-baremetal";
247249
return createRISCVTargetCodeGenInfo(CGM, XLen, ABIFLen, EABI);
248250
}
249251

clang/lib/CodeGen/Targets/RISCV.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,10 @@ RValue RISCVABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
587587
// 2×XLEN-bit alignment and size at most 2×XLEN bits like `long long`,
588588
// `unsigned long long` and `double` to have 4-byte alignment. This
589589
// behavior may be changed when RV32E/ILP32E is ratified.
590-
if (EABI && XLen == 32)
590+
// NOTE: Cheriot does not use the GCC workaround behavior.
591+
StringRef TargetABI = getTarget().getABI();
592+
bool IsCheriot = TargetABI == "cheriot" || TargetABI == "cheriot-baremetal";
593+
if (EABI && XLen == 32 && !IsCheriot)
591594
TInfo.Align = std::min(TInfo.Align, CharUnits::fromQuantity(4));
592595

593596
bool IsSingleCapRecord = false;

clang/test/CodeGen/cheri/cheriot-variadic.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,22 @@ typedef struct {
1414

1515
extern int onward(void *, int, char *);
1616

17+
// CHECK-LABEL: @foo
1718
int foo(va_list ap) {
1819
// Make sure that we don't see a memcpy in address space zero!
1920
// CHECK-NOT: p0i8
2021
bar_t x = va_arg(ap, bar_t);
2122
return onward(x.a, x.b, x.c);
2223
}
24+
25+
// CHECK-LABEL: @bar
26+
double bar(const char* c, ...) {
27+
// Make sure that a double is dynamically aligned up to 8 bytes.
28+
// CHECK: ptrmask
29+
va_list ap;
30+
va_start((ap), c);
31+
int i1 = va_arg(ap, int);
32+
double f = va_arg(ap, double);
33+
va_end(ap);
34+
return f;
35+
}

llvm/lib/Target/RISCV/RISCVCallingConv.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,8 +302,12 @@ static bool CC_RISCVAssign2XLen(unsigned XLen, CCState &State, bool IsPureCapVar
302302
// Both halves must be passed on the stack, with proper alignment.
303303
// TODO: To be compatible with GCC's behaviors, we force them to have 4-byte
304304
// alignment. This behavior may be changed when RV32E/ILP32E is ratified.
305+
// NOTE: Cheriot does not use the GCC workaround behavior.
305306
Align StackAlign(XLenInBytes);
306-
if (!EABI || XLen != 32)
307+
RISCVABI::ABI ABI = STI.getTargetABI();
308+
bool IsCheriot = (ABI == RISCVABI::ABI_CHERIOT) ||
309+
(ABI == RISCVABI::ABI_CHERIOT_BAREMETAL);
310+
if (!EABI || XLen != 32 || IsCheriot)
307311
StackAlign = std::max(StackAlign, ArgFlags1.getNonZeroOrigAlign());
308312
State.addLoc(
309313
CCValAssign::getMem(VA1.getValNo(), VA1.getValVT(),
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
; RUN: llc --filetype=asm --mcpu=cheriot --mtriple=riscv32cheriot-unknown-cheriotrtos -target-abi cheriot -mattr=+xcheri,+cap-mode,+xcheriot %s -o - | FileCheck %s
2+
3+
; Verify that varargs doubles are aligned to 8 bytes on the stack.
4+
5+
target datalayout = "e-m:e-p:32:32-i64:64-n32-S128-pf200:64:64:64:32-A200-P200-G200"
6+
target triple = "riscv32cheriot-unknown-cheriotrtos"
7+
8+
declare hidden noundef double @_Z7va_testPKcz(ptr addrspace(200) nocapture readnone %c, ...) local_unnamed_addr addrspace(200)
9+
10+
define hidden noundef double @_Z6helperv() local_unnamed_addr addrspace(200) {
11+
; CHECK-LABEL: _Z6helperv:
12+
; CHECK-NOT: csw [.*], 4(csp)
13+
; CHECK-DAG: lui [[R0:.*]], 261939
14+
; CHECK-DAG: lui [[R1:.*]], 209715
15+
; CHECK-DAG: addi [[R0]], [[R0]], 819
16+
; CHECK-DAG: addi [[R1]], [[R1]], 819
17+
; CHECK-DAG: csw [[R0]], 12(csp)
18+
; CHECK-DAG: csw [[R1]], 8(csp)
19+
; CHECK-NOT: csw [.*], 4(csp)
20+
entry:
21+
%call = tail call noundef double (ptr addrspace(200), ...) @_Z7va_testPKcz(ptr addrspace(200) nonnull poison, i32 noundef 0, double noundef 1.200000e+00)
22+
ret double %call
23+
}
24+
25+
!llvm.linker.options = !{}
26+
!llvm.module.flags = !{!0, !1, !2, !4}
27+
!llvm.ident = !{!5}
28+
29+
!0 = !{i32 1, !"wchar_size", i32 2}
30+
!1 = !{i32 1, !"target-abi", !"cheriot"}
31+
!2 = !{i32 6, !"riscv-isa", !3}
32+
!3 = !{!"rv32e2p0_m2p0_c2p0_zmmul1p0_xcheri0p0_xcheriot1p0"}
33+
!4 = !{i32 8, !"SmallDataLimit", i32 0}
34+
!5 = !{!"clang version 20.1.3 ([email protected]:resistor/llvm-project-1.git 592752fee8b25c925a65fb40eeb8a496f1b0ee2c)"}
35+
!6 = !{!7, !7, i64 0}
36+
!7 = !{!"double", !8, i64 0}
37+
!8 = !{!"omnipotent char", !9, i64 0}
38+
!9 = !{!"Simple C++ TBAA"}

0 commit comments

Comments
 (0)