-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[AArch64][PAC] Sign block addresses used in indirectbr. #97647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
5195708
cdf8b86
c21435b
46274c3
6cf9845
ecac22c
b91e109
f9cf7d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,6 +85,7 @@ | |
#include "llvm/Support/InstructionCost.h" | ||
#include "llvm/Support/KnownBits.h" | ||
#include "llvm/Support/MathExtras.h" | ||
#include "llvm/Support/SipHash.h" | ||
#include "llvm/Support/raw_ostream.h" | ||
#include "llvm/Target/TargetMachine.h" | ||
#include "llvm/Target/TargetOptions.h" | ||
|
@@ -509,6 +510,7 @@ AArch64TargetLowering::AArch64TargetLowering(const TargetMachine &TM, | |
setOperationAction(ISD::SELECT_CC, MVT::f64, Custom); | ||
setOperationAction(ISD::BR_JT, MVT::Other, Custom); | ||
setOperationAction(ISD::JumpTable, MVT::i64, Custom); | ||
setOperationAction(ISD::BRIND, MVT::Other, Custom); | ||
setOperationAction(ISD::SETCCCARRY, MVT::i64, Custom); | ||
|
||
setOperationAction(ISD::PtrAuthGlobalAddress, MVT::i64, Custom); | ||
|
@@ -6694,6 +6696,8 @@ SDValue AArch64TargetLowering::LowerOperation(SDValue Op, | |
return LowerJumpTable(Op, DAG); | ||
case ISD::BR_JT: | ||
return LowerBR_JT(Op, DAG); | ||
case ISD::BRIND: | ||
return LowerBRIND(Op, DAG); | ||
case ISD::ConstantPool: | ||
return LowerConstantPool(Op, DAG); | ||
case ISD::BlockAddress: | ||
|
@@ -10685,6 +10689,26 @@ SDValue AArch64TargetLowering::LowerBR_JT(SDValue Op, | |
return DAG.getNode(ISD::BRIND, DL, MVT::Other, JTInfo, SDValue(Dest, 0)); | ||
} | ||
|
||
SDValue AArch64TargetLowering::LowerBRIND(SDValue Op, SelectionDAG &DAG) const { | ||
MachineFunction &MF = DAG.getMachineFunction(); | ||
ahmedbougacha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
std::optional<uint16_t> BADisc = | ||
Subtarget->getPtrAuthBlockAddressDiscriminator(MF.getFunction()); | ||
if (!BADisc) | ||
return SDValue(); | ||
|
||
SDLoc DL(Op); | ||
SDValue Chain = Op.getOperand(0); | ||
SDValue Dest = Op.getOperand(1); | ||
|
||
SDValue Disc = DAG.getTargetConstant(*BADisc, DL, MVT::i64); | ||
SDValue Key = DAG.getTargetConstant(AArch64PACKey::IA, DL, MVT::i32); | ||
SDValue AddrDisc = DAG.getRegister(AArch64::XZR, MVT::i64); | ||
|
||
SDNode *BrA = DAG.getMachineNode(AArch64::BRA, DL, MVT::Other, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a problematic piece of code. If jump table hardening is not enabled, then jump tables will be codegenerated using The code above might check if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, that explains it. I'd rather have the BR_JT survive as a BR_JT (or really, an AArch64 BR_JT that's analogous to ISD::BRIND, so that we can still do what we do on JumpTableDests), but that's a more serious change for the whole backend; let's do it separately. For now checking the target seems iffy but probably workable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, having |
||
{Dest, Key, Disc, AddrDisc, Chain}); | ||
return SDValue(BrA, 0); | ||
} | ||
|
||
SDValue AArch64TargetLowering::LowerConstantPool(SDValue Op, | ||
SelectionDAG &DAG) const { | ||
ConstantPoolSDNode *CP = cast<ConstantPoolSDNode>(Op); | ||
|
@@ -10704,15 +10728,36 @@ SDValue AArch64TargetLowering::LowerConstantPool(SDValue Op, | |
|
||
SDValue AArch64TargetLowering::LowerBlockAddress(SDValue Op, | ||
SelectionDAG &DAG) const { | ||
BlockAddressSDNode *BA = cast<BlockAddressSDNode>(Op); | ||
BlockAddressSDNode *BAN = cast<BlockAddressSDNode>(Op); | ||
const BlockAddress *BA = BAN->getBlockAddress(); | ||
|
||
if (std::optional<uint16_t> BADisc = | ||
Subtarget->getPtrAuthBlockAddressDiscriminator(*BA->getFunction())) { | ||
SDLoc DL(Op); | ||
|
||
// This isn't cheap, but BRIND is rare. | ||
SDValue TargetBA = DAG.getTargetBlockAddress(BA, BAN->getValueType(0)); | ||
|
||
SDValue Disc = DAG.getTargetConstant(*BADisc, DL, MVT::i64); | ||
|
||
SDValue Key = DAG.getTargetConstant(AArch64PACKey::IA, DL, MVT::i32); | ||
SDValue AddrDisc = DAG.getRegister(AArch64::XZR, MVT::i64); | ||
|
||
SDNode *MOV = | ||
DAG.getMachineNode(AArch64::MOVaddrPAC, DL, {MVT::Other, MVT::Glue}, | ||
{TargetBA, Key, AddrDisc, Disc}); | ||
return DAG.getCopyFromReg(SDValue(MOV, 0), DL, AArch64::X16, MVT::i64, | ||
SDValue(MOV, 1)); | ||
} | ||
|
||
CodeModel::Model CM = getTargetMachine().getCodeModel(); | ||
if (CM == CodeModel::Large && !Subtarget->isTargetMachO()) { | ||
if (!getTargetMachine().isPositionIndependent()) | ||
return getAddrLarge(BA, DAG); | ||
return getAddrLarge(BAN, DAG); | ||
} else if (CM == CodeModel::Tiny) { | ||
return getAddrTiny(BA, DAG); | ||
return getAddrTiny(BAN, DAG); | ||
} | ||
return getAddr(BA, DAG); | ||
return getAddr(BAN, DAG); | ||
} | ||
|
||
SDValue AArch64TargetLowering::LowerDarwin_VASTART(SDValue Op, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: there is no empty line between previous
Args.addOptInFlag(...)
invocations, so probably this new line should also be deleted.Feel free to ignore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whitespaces help differentiate the different classes of fptrauth features (here frontendy vs backendy); TBH we should probably have more, not less