Skip to content

Commit

Permalink
[CHERI-RISC-V] Restrict CFromPtr usage to ISAv8 compatibility
Browse files Browse the repository at this point in the history
We emit `x == 0 ? null : CSetOffset(auth, x)` when we see the intrinsic in
non-ISAv8 compilation mode instead. When expanding `inttoptr` in hybrid
mode, we do not include the base of DDC as part of the expansion (since
DDC relocation is no longer part of ISAv9). Adding the base would result
in a capability that points to a different address when dereferenced.
Therefore, inttoptr is expanded to `x == 0 ? null : CSetAddr(DDC, x)`.
It would be nice if we could just use `CSetAddr(DDC, x)` unconditionally
for non-constant arguments, but that could result in semantic differences
(tagged vs untagged) for integers that happen to be zero at run time.
  • Loading branch information
arichardson committed Jul 7, 2023
1 parent 2a20c31 commit 0c0ec2f
Show file tree
Hide file tree
Showing 4 changed files with 252 additions and 8 deletions.
55 changes: 54 additions & 1 deletion llvm/lib/Target/RISCV/RISCVISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -514,8 +514,10 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
setOperationAction(ISD::GlobalTLSAddress, CLenVT, Custom);
setOperationAction(ISD::ADDRSPACECAST, CLenVT, Custom);
setOperationAction(ISD::ADDRSPACECAST, XLenVT, Custom);
if (!Subtarget.hasCheriISAv8Semantics())
if (!Subtarget.hasCheriISAv8Semantics()) {
setOperationAction(ISD::PTRTOINT, XLenVT, Custom);
setOperationAction(ISD::INTTOPTR, CLenVT, Custom);
}
}

// TODO: On M-mode only targets, the cycle[h] CSR may not be present.
Expand Down Expand Up @@ -2861,6 +2863,19 @@ static SDValue lowerCTLZ_CTTZ_ZERO_UNDEF(SDValue Op, SelectionDAG &DAG) {
return DAG.getNode(ISD::SUB, DL, VT, DAG.getConstant(Adjust, DL, VT), Trunc);
}

/// Emit a replacement for the (to-be) removed CFromPtr instruction.
static SDValue
emitCFromPtrReplacement(SelectionDAG &DAG, const SDLoc &DL, SDValue Base,
SDValue IntVal, EVT CLenVT,
unsigned Intrin = Intrinsic::cheri_cap_offset_set) {
SDValue AsCap =
DAG.getNode(ISD::INTRINSIC_WO_CHAIN, DL, CLenVT,
DAG.getConstant(Intrin, DL, MVT::i64), Base, IntVal);
return DAG.getSelectCC(DL, IntVal,
DAG.getConstant(0, DL, IntVal.getValueType()), AsCap,
DAG.getNullCapability(DL), ISD::SETNE);
}

/// The CToPtr instruction is deprecated and will be removed. This function
/// emits the currently defined semantics of `gettag(x) ? x.addr - base : 0`.
static SDValue emitCToPtrReplacement(SelectionDAG &DAG, const SDLoc &DL,
Expand Down Expand Up @@ -3037,6 +3052,37 @@ SDValue RISCVTargetLowering::LowerOperation(SDValue Op,
unsigned NewOp = ToCap ? ISD::INTTOPTR : ISD::PTRTOINT;
return DAG.getNode(NewOp, DL, Op.getValueType(), Op0);
}
case ISD::INTTOPTR: {
SDValue Op0 = Op.getOperand(0);
if (Op.getValueType().isFatPointer() && !Subtarget.hasCheriISAv8Semantics() &&
!RISCVABI::isCheriPureCapABI(Subtarget.getTargetABI())) {
if (isNullConstant(Op0)) {
// Do not custom lower (inttoptr 0) here as that is the canonical
// representation of capability NULL, and expanding it here disables
// matches for this specific pattern.
return Op;
}
SDLoc DL(Op);
EVT CLenVT = Op.getValueType();
auto GetDDC = DAG.getConstant(Intrinsic::cheri_ddc_get, DL, MVT::i64);
auto DDC = DAG.getNode(ISD::INTRINSIC_WO_CHAIN, DL, CapType, GetDDC);
// NB: Since DDC relocation is not included as part of ISAv9, we
// emit a SetAddress operation instead of the CFromPtr SetOffset. If we
// were to use SetOffset, the resulting capability could point to a
// different memory location when dereferenced compared to an integer
// based DDC-constrained access.
// It would be nice if we could just use SetAddr on DDC, but for
// consistency between constant inttoptr and non-constant inttoptr
// we emit a copy of NULL if the value is zero and otherwise use a SetAddr
// on DDC and use a select to choose the correct value. This avoids
// getting different values for inttoptr with a constant zero argument and
// inttoptr with a variable that happens to be zero (the latter should not
// result in a tagged value).
return emitCFromPtrReplacement(DAG, DL, DDC, Op0, CLenVT,
Intrinsic::cheri_cap_address_set);
}
return Op;
}
case ISD::PTRTOINT: {
SDValue Op0 = Op.getOperand(0);
if (Op0.getValueType().isFatPointer() &&
Expand Down Expand Up @@ -4682,6 +4728,13 @@ SDValue RISCVTargetLowering::LowerINTRINSIC_WO_CHAIN(SDValue Op,
switch (IntNo) {
default:
break; // Don't custom lower most intrinsics.
case Intrinsic::cheri_cap_from_pointer:
// Expand CFromPtr if the dedicated instruction has been removed.
if (!Subtarget.hasCheriISAv8Semantics()) {
return emitCFromPtrReplacement(DAG, DL, Op.getOperand(1),
Op.getOperand(2), Op.getValueType());
}
break;
case Intrinsic::cheri_cap_to_pointer:
// Expand CToPtr if the dedicated instruction has been removed.
if (!Subtarget.hasCheriISAv8Semantics()) {
Expand Down
14 changes: 7 additions & 7 deletions llvm/lib/Target/RISCV/RISCVInstrInfoXCheri.td
Original file line number Diff line number Diff line change
Expand Up @@ -370,10 +370,10 @@ def : InstAlias<"csetboundsimm $cd, $cs1, $imm",
// Pointer-Arithmetic Instructions
//===----------------------------------------------------------------------===//

let Predicates = [HasCheriISAv8], mayTrap = 1 in
let Predicates = [HasCheriISAv8], mayTrap = 1 in {
def CToPtr : Cheri_rr<0x12, "ctoptr", GPR, GPCRC0IsDDC>;
let Predicates = [HasCheri], mayTrap = 1 in
def CFromPtr : Cheri_rr<0x13, "cfromptr", GPCR, GPR, GPCRC0IsDDC>;
}
let Predicates = [HasCheri] in {
let hasSideEffects = 0, mayLoad = 0, mayStore = 0, isCodeGenOnly = 0,
isAsmParserOnly = 1 in
Expand Down Expand Up @@ -1211,19 +1211,19 @@ def : Pat<(ptradd CapFI128:$rs1, simm12:$imm12),

/// Pointer-Arithmetic Instructions

// int_cheri_cap_to_pointer is expanded in C++ code for ISAv9, so we don't
// need replacement patterns here.
// int_cheri_cap_to_pointer and int_cheri_cap_from_pointer are expanded in C++
// code for ISAv9, so we don't need replacement patterns here.
// TODO: these intrinsics should probably be removed at some point.
let Predicates = [HasCheriISAv8] in {
def : Pat<(int_cheri_cap_to_pointer GPCR:$rs1, GPCRC0IsDDC:$rs2),
(CToPtr GPCR:$rs1, GPCRC0IsDDC:$rs2)>;
def : Pat<(int_cheri_cap_to_pointer (CLenVT (int_cheri_ddc_get)), GPCR:$rs1),
(CToPtr $rs1, DDC)>;
}
def : Pat<(int_cheri_cap_from_pointer GPCRC0IsDDC:$rs1, GPR:$rs2),
(CFromPtr GPCRC0IsDDC:$rs1, GPR:$rs2)>;
def : Pat<(int_cheri_cap_from_pointer (CLenVT (int_cheri_ddc_get)), GPR:$rs2),
(CFromPtr DDC, $rs2)>;
} // let Predicates = [HasCheriISAv8]
def : Pat<(int_cheri_cap_diff GPCR:$cs1, GPCR:$cs2),
(SUB (EXTRACT_SUBREG GPCR:$cs1, sub_cap_addr),
(EXTRACT_SUBREG GPCR:$cs2, sub_cap_addr))>;
Expand All @@ -1234,10 +1234,10 @@ def : Pat<(inttoptr simm12:$imm12), (CIncOffsetImm C0, simm12:$imm12)>;
def : Pat<(ptrtoint GPCR:$rs1), (PseudoCGetAddr GPCR:$rs1)>;
}

let Predicates = [NotPureCapABI] in
let Predicates = [NotPureCapABI, HasCheriISAv8] in {
def : Pat<(inttoptr GPR:$rs2), (CFromPtr DDC, GPR:$rs2)>;
let Predicates = [NotPureCapABI, HasCheriISAv8] in
def : Pat<(ptrtoint GPCR:$rs1), (CToPtr GPCR:$rs1, DDC)>;
}

/// Null Capability Patterns

Expand Down
66 changes: 66 additions & 0 deletions llvm/test/CodeGen/RISCV/cheri/isav9-cap-from-ptr.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
;; The CFromPtr instruction is not part of ISAv9, check that we emit `x == 0 ? null : csetoffset(auth, x)` instead.
; RUN: %riscv64_cheri_purecap_llc < %s | FileCheck %s --check-prefix=ISAV8
; RUN: %riscv64_cheri_purecap_llc -mattr=+xcheri-no-v8-compat < %s | FileCheck %s --check-prefix=ISAV9
;; Also emit a .o file since verifyInstructionPredicates() is only called for binary output.
; RUN: %riscv64_cheri_purecap_llc -mattr=+xcheri-no-v8-compat -filetype=obj -o /dev/null < %s

;; (int_cheri_cap_from_ptr auth, x) -> x == 0 ? null : csetoffset(auth, x)
define internal i8 addrspace(200)* @cap_from_ptr(i8 addrspace(200)* addrspace(200)* %ptr, i8 addrspace(200)* %cap, i64 %offset) nounwind {
; ISAV8-LABEL: cap_from_ptr:
; ISAV8: # %bb.0: # %entry
; ISAV8-NEXT: cfromptr ca1, ca1, a2
; ISAV8-NEXT: csc ca1, 0(ca0)
; ISAV8-NEXT: cmove ca0, ca1
; ISAV8-NEXT: cret
;
; ISAV9-LABEL: cap_from_ptr:
; ISAV9: # %bb.0: # %entry
; ISAV9-NEXT: bnez a2, .LBB0_2
; ISAV9-NEXT: # %bb.1: # %entry
; ISAV9-NEXT: cmove ca1, cnull
; ISAV9-NEXT: j .LBB0_3
; ISAV9-NEXT: .LBB0_2:
; ISAV9-NEXT: csetoffset ca1, ca1, a2
; ISAV9-NEXT: .LBB0_3: # %entry
; ISAV9-NEXT: csc ca1, 0(ca0)
; ISAV9-NEXT: cmove ca0, ca1
; ISAV9-NEXT: cret
entry:
%new = call i8 addrspace(200)* @llvm.cheri.cap.from.pointer.i64(i8 addrspace(200)* %cap, i64 %offset)
store i8 addrspace(200)* %new, i8 addrspace(200)* addrspace(200)* %ptr, align 16
ret i8 addrspace(200)* %new
}

;; (int_cheri_cap_from_ptr ddc, y) -> x == 0 ? null : csetoffset(ddc, x)
;; NB: This is not the same as (inttoptr x), since the explicit intrinsic retains the offsetting semantics.
define internal i8 addrspace(200)* @cap_from_ptr_ddc(i8 addrspace(200)* addrspace(200)* %ptr, i64 %offset) nounwind {
; ISAV8-LABEL: cap_from_ptr_ddc:
; ISAV8: # %bb.0: # %entry
; ISAV8-NEXT: cfromptr ca1, ddc, a1
; ISAV8-NEXT: csc ca1, 0(ca0)
; ISAV8-NEXT: cmove ca0, ca1
; ISAV8-NEXT: cret
;
; ISAV9-LABEL: cap_from_ptr_ddc:
; ISAV9: # %bb.0: # %entry
; ISAV9-NEXT: cspecialr ca2, ddc
; ISAV9-NEXT: bnez a1, .LBB1_2
; ISAV9-NEXT: # %bb.1: # %entry
; ISAV9-NEXT: cmove ca1, cnull
; ISAV9-NEXT: j .LBB1_3
; ISAV9-NEXT: .LBB1_2:
; ISAV9-NEXT: csetoffset ca1, ca2, a1
; ISAV9-NEXT: .LBB1_3: # %entry
; ISAV9-NEXT: csc ca1, 0(ca0)
; ISAV9-NEXT: cmove ca0, ca1
; ISAV9-NEXT: cret
entry:
%ddc = call i8 addrspace(200)* @llvm.cheri.ddc.get()
%new = call i8 addrspace(200)* @llvm.cheri.cap.from.pointer.i64(i8 addrspace(200)* %ddc, i64 %offset)
store i8 addrspace(200)* %new, i8 addrspace(200)* addrspace(200)* %ptr, align 16
ret i8 addrspace(200)* %new
}

declare i8 addrspace(200)* @llvm.cheri.cap.from.pointer.i64(i8 addrspace(200)*, i64)
declare i8 addrspace(200)* @llvm.cheri.ddc.get()
125 changes: 125 additions & 0 deletions llvm/test/CodeGen/RISCV/cheri/isav9-inttoptr.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
;; Check that we don't emit CFromPtr when lowering inttoptr for ISAv9.
; RUN: %riscv64_cheri_purecap_llc -mattr=+xcheri-no-v8-compat < %s | FileCheck %s --check-prefix=PURECAP
; RUN: %riscv64_cheri_llc -mattr=+xcheri-no-v8-compat < %s | FileCheck %s --check-prefix=HYBRID
;; Also emit a .o file since verifyInstructionPredicates() is only called for binary output.
; RUN: %riscv64_cheri_purecap_llc -mattr=+xcheri-no-v8-compat -filetype=obj -o /dev/null < %s
; RUN: %riscv64_cheri_llc -mattr=+xcheri-no-v8-compat -filetype=obj -o /dev/null < %s

define internal i8 addrspace(200)* @inttoptr(i64 %ptr) addrspace(200) nounwind {
; PURECAP-LABEL: inttoptr:
; PURECAP: # %bb.0:
; PURECAP-NEXT: cincoffset ca0, cnull, a0
; PURECAP-NEXT: cret
;
; HYBRID-LABEL: inttoptr:
; HYBRID: # %bb.0:
; HYBRID-NEXT: cspecialr ca1, ddc
; HYBRID-NEXT: bnez a0, .LBB0_2
; HYBRID-NEXT: # %bb.1:
; HYBRID-NEXT: cmove ca0, cnull
; HYBRID-NEXT: ret
; HYBRID-NEXT: .LBB0_2:
; HYBRID-NEXT: csetaddr ca0, ca1, a0
; HYBRID-NEXT: ret
%ret = inttoptr i64 %ptr to i8 addrspace(200)*
ret i8 addrspace(200)* %ret
}

define internal i8 addrspace(200)* @inttoptr_plus_const(i64 %ptr) addrspace(200) nounwind {
; PURECAP-LABEL: inttoptr_plus_const:
; PURECAP: # %bb.0:
; PURECAP-NEXT: cincoffset ca0, cnull, a0
; PURECAP-NEXT: cincoffset ca0, ca0, 2
; PURECAP-NEXT: cret
;
; HYBRID-LABEL: inttoptr_plus_const:
; HYBRID: # %bb.0:
; HYBRID-NEXT: cspecialr ca1, ddc
; HYBRID-NEXT: bnez a0, .LBB1_2
; HYBRID-NEXT: # %bb.1:
; HYBRID-NEXT: cincoffset ca0, cnull, 2
; HYBRID-NEXT: ret
; HYBRID-NEXT: .LBB1_2:
; HYBRID-NEXT: csetaddr ca0, ca1, a0
; HYBRID-NEXT: cincoffset ca0, ca0, 2
; HYBRID-NEXT: ret
%cap = inttoptr i64 %ptr to i8 addrspace(200)*
%ret = getelementptr i8, i8 addrspace(200)* %cap, i64 2
ret i8 addrspace(200)* %ret
}

;; CFromPtr has been removed from CHERI ISAv9, but we want to retain the
;; semantics that inttoptr of a variable that happens to be zero will result in
;; an untagged value. This means we can't just expand inttoptr to CSetAddr on
;; DDC since using that with address zero could result in a valid capability.
;; Therefore we have to insert a conditional branch and emit
;; `x.addr == 0 ? null : CSetAddr DDC, x` instead. This is a rather long code
;; sequence due to lack of conditional moves, but it should happen rarely.

define internal i8 addrspace(200)* @inttoptr_plus_var(i64 %ptr, i64 %add) addrspace(200) nounwind {
; PURECAP-LABEL: inttoptr_plus_var:
; PURECAP: # %bb.0:
; PURECAP-NEXT: cincoffset ca0, cnull, a0
; PURECAP-NEXT: cincoffset ca0, ca0, a1
; PURECAP-NEXT: cret
;
; HYBRID-LABEL: inttoptr_plus_var:
; HYBRID: # %bb.0:
; HYBRID-NEXT: cspecialr ca2, ddc
; HYBRID-NEXT: bnez a0, .LBB2_2
; HYBRID-NEXT: # %bb.1:
; HYBRID-NEXT: cincoffset ca0, cnull, a1
; HYBRID-NEXT: ret
; HYBRID-NEXT: .LBB2_2:
; HYBRID-NEXT: csetaddr ca0, ca2, a0
; HYBRID-NEXT: cincoffset ca0, ca0, a1
; HYBRID-NEXT: ret
%cap = inttoptr i64 %ptr to i8 addrspace(200)*
%ret = getelementptr i8, i8 addrspace(200)* %cap, i64 %add
ret i8 addrspace(200)* %ret
}

define internal i8 addrspace(200)* @inttoptr_null() addrspace(200) nounwind {
; PURECAP-LABEL: inttoptr_null:
; PURECAP: # %bb.0:
; PURECAP-NEXT: cmove ca0, cnull
; PURECAP-NEXT: cret
;
; HYBRID-LABEL: inttoptr_null:
; HYBRID: # %bb.0:
; HYBRID-NEXT: cmove ca0, cnull
; HYBRID-NEXT: ret
%ret = inttoptr i64 0 to i8 addrspace(200)*
ret i8 addrspace(200)* %ret
}

define internal i8 addrspace(200)* @inttoptr_null_plus_const() addrspace(200) nounwind {
; PURECAP-LABEL: inttoptr_null_plus_const:
; PURECAP: # %bb.0:
; PURECAP-NEXT: cincoffset ca0, cnull, 2
; PURECAP-NEXT: cret
;
; HYBRID-LABEL: inttoptr_null_plus_const:
; HYBRID: # %bb.0:
; HYBRID-NEXT: cincoffset ca0, cnull, 2
; HYBRID-NEXT: ret
%null = inttoptr i64 0 to i8 addrspace(200)*
%ret = getelementptr i8, i8 addrspace(200)* %null, i64 2
ret i8 addrspace(200)* %ret
}

define internal i8 addrspace(200)* @inttoptr_null_plus_var(i64 %add) addrspace(200) nounwind {
; PURECAP-LABEL: inttoptr_null_plus_var:
; PURECAP: # %bb.0:
; PURECAP-NEXT: cincoffset ca0, cnull, a0
; PURECAP-NEXT: cret
;
; HYBRID-LABEL: inttoptr_null_plus_var:
; HYBRID: # %bb.0:
; HYBRID-NEXT: cincoffset ca0, cnull, a0
; HYBRID-NEXT: ret
%null = inttoptr i64 0 to i8 addrspace(200)*
%ret = getelementptr i8, i8 addrspace(200)* %null, i64 %add
ret i8 addrspace(200)* %ret
}

0 comments on commit 0c0ec2f

Please sign in to comment.