[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Ahmed Bougacha via cfe-commits 

https://github.com/ahmedbougacha closed 
https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Eli Friedman via cfe-commits 


@@ -10909,6 +10909,14 @@ QualType Sema::CheckAdditionOperands(ExprResult &LHS, 
ExprResult &RHS,
   if (isObjCPointer && checkArithmeticOnObjCPointer(*this, Loc, PExp))
 return QualType();
 
+  // Arithmetic on label addresses is normally allowed, except when we add
+  // a ptrauth signature to the addresses.
+  if (isa(PExp) && getLangOpts().PointerAuthIndirectGotos) {

efriedma-quic wrote:

I don't think this catches all the relevant cases: there are various ways you 
could "hide" an address-of-label from this check.  Parentheses, a conditional 
operator, a constexpr variable, etc.

Maybe the check should be in IntExprEvaluator::VisitBinaryOperator.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Anton Korobeynikov via cfe-commits 

https://github.com/asl approved this pull request.


https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Ahmed Bougacha via cfe-commits 

https://github.com/ahmedbougacha updated 
https://github.com/llvm/llvm-project/pull/97647

>From 519570896c82887a5dd878fcc16f884857d4fce3 Mon Sep 17 00:00:00 2001
From: Ahmed Bougacha 
Date: Tue, 12 Mar 2024 14:40:17 -0700
Subject: [PATCH 1/5] [AArch64][PAC] Sign block addresses used in indirectbr.

Enabled in clang using:
  -fptrauth-indirect-gotos

and at the IR level using function attribute:
  "ptrauth-indirect-gotos"

Signing uses IA and a per-function integer discriminator.
The discriminator isn't ABI-visible, and is currently:
  ptrauth_string_discriminator(" blockaddress")

A sufficiently sophisticated frontend could benefit from
per-indirectbr discrimination, which would need additional
machinery, such as allowing "ptrauth" bundles on indirectbr.
For our purposes, the simple scheme above is sufficient.
---
 clang/include/clang/Basic/Features.def|   1 +
 clang/include/clang/Basic/LangOptions.def |   1 +
 .../include/clang/Basic/PointerAuthOptions.h  |   3 +
 clang/include/clang/Driver/Options.td |   2 +
 clang/lib/CodeGen/CodeGenFunction.cpp |   2 +
 clang/lib/Driver/ToolChains/Clang.cpp |   3 +
 clang/lib/Frontend/CompilerInvocation.cpp |   6 +-
 .../CodeGen/ptrauth-function-attributes.c |   5 +
 llvm/docs/PointerAuth.md  |  24 
 llvm/include/llvm/CodeGen/AsmPrinter.h|   3 +
 llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp|   6 +-
 llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp |  35 +-
 llvm/lib/Target/AArch64/AArch64FastISel.cpp   |   4 +
 .../Target/AArch64/AArch64ISelLowering.cpp|  53 -
 llvm/lib/Target/AArch64/AArch64ISelLowering.h |   1 +
 llvm/lib/Target/AArch64/AArch64InstrInfo.td   |  18 +++
 llvm/lib/Target/AArch64/AArch64Subtarget.cpp  |  11 ++
 llvm/lib/Target/AArch64/AArch64Subtarget.h|   9 ++
 .../GISel/AArch64InstructionSelector.cpp  |  26 +
 .../CodeGen/AArch64/ptrauth-indirectbr.ll | 106 ++
 20 files changed, 308 insertions(+), 11 deletions(-)
 create mode 100644 llvm/test/CodeGen/AArch64/ptrauth-indirectbr.ll

diff --git a/clang/include/clang/Basic/Features.def 
b/clang/include/clang/Basic/Features.def
index 53f410d3cb4bd..cf800afe08557 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -108,6 +108,7 @@ FEATURE(ptrauth_calls, LangOpts.PointerAuthCalls)
 FEATURE(ptrauth_returns, LangOpts.PointerAuthReturns)
 FEATURE(ptrauth_vtable_pointer_address_discrimination, 
LangOpts.PointerAuthVTPtrAddressDiscrimination)
 FEATURE(ptrauth_vtable_pointer_type_discrimination, 
LangOpts.PointerAuthVTPtrTypeDiscrimination)
+FEATURE(ptrauth_indirect_gotos, LangOpts.PointerAuthIndirectGotos)
 FEATURE(ptrauth_member_function_pointer_type_discrimination, 
LangOpts.PointerAuthCalls)
 FEATURE(ptrauth_init_fini, LangOpts.PointerAuthInitFini)
 EXTENSION(swiftcc,
diff --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 491759e2fcdbb..bdf77a5b35208 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -165,6 +165,7 @@ LANGOPT(ExperimentalLibrary, 1, 0, "enable unstable and 
experimental library fea
 LANGOPT(PointerAuthIntrinsics, 1, 0, "pointer authentication intrinsics")
 LANGOPT(PointerAuthCalls  , 1, 0, "function pointer authentication")
 LANGOPT(PointerAuthReturns, 1, 0, "return pointer authentication")
+LANGOPT(PointerAuthIndirectGotos, 1, 0, "indirect gotos pointer 
authentication")
 LANGOPT(PointerAuthAuthTraps, 1, 0, "pointer authentication failure traps")
 LANGOPT(PointerAuthVTPtrAddressDiscrimination, 1, 0, "incorporate address 
discrimination in authenticated vtable pointers")
 LANGOPT(PointerAuthVTPtrTypeDiscrimination, 1, 0, "incorporate type 
discrimination in authenticated vtable pointers")
diff --git a/clang/include/clang/Basic/PointerAuthOptions.h 
b/clang/include/clang/Basic/PointerAuthOptions.h
index 197d63642ca6d..2711639dbe299 100644
--- a/clang/include/clang/Basic/PointerAuthOptions.h
+++ b/clang/include/clang/Basic/PointerAuthOptions.h
@@ -154,6 +154,9 @@ class PointerAuthSchema {
 };
 
 struct PointerAuthOptions {
+  /// Do indirect goto label addresses need to be authenticated?
+  bool IndirectGotos = false;
+
   /// The ABI for C function pointers.
   PointerAuthSchema FunctionPointers;
 
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 58ca6f2bea9e4..791b7261ddbda 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4228,6 +4228,8 @@ defm ptrauth_vtable_pointer_address_discrimination :
 defm ptrauth_vtable_pointer_type_discrimination :
   OptInCC1FFlag<"ptrauth-vtable-pointer-type-discrimination", "Enable type 
discrimination of vtable pointers">;
 defm ptrauth_init_fini : OptInCC1FFlag<"ptrauth-init-fini", "Enable signing of 
function pointers in init/fini arrays">;
+defm ptrauth_indirect_gotos : OptInCC1FFlag<"ptraut

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Anton Korobeynikov via cfe-commits 


@@ -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();
+  std::optional 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,

asl wrote:

yeah, having `BR_JT` nodes is certainly more correct way, but seems to be more 
invasive. Worth a `TODO` added, yes.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Ahmed Bougacha via cfe-commits 


@@ -1789,6 +1789,9 @@ void Clang::AddAArch64TargetArgs(const ArgList &Args,
   options::OPT_fno_ptrauth_vtable_pointer_type_discrimination);
   Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_init_fini,
 options::OPT_fno_ptrauth_init_fini);
+

ahmedbougacha wrote:

The whitespaces help differentiate the different classes of fptrauth features 
(here frontendy vs backendy);  TBH we should probably have more, not less

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Ahmed Bougacha via cfe-commits 


@@ -412,6 +412,15 @@ class AArch64Subtarget final : public 
AArch64GenSubtargetInfo {
   /// Choose a method of checking LR before performing a tail call.
   AArch64PAuth::AuthCheckMethod getAuthenticatedLRCheckMethod() const;
 
+  /// Compute the integer discriminator for a given BlockAddress constant, if
+  /// blockaddress signing is enabled (using function attribute
+  /// "ptrauth-indirect-gotos").

ahmedbougacha wrote:

I added `IfNeeded` to make the function name more explicit?

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Ahmed Bougacha via cfe-commits 


@@ -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();
+  std::optional 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,

ahmedbougacha wrote:

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

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Ahmed Bougacha via cfe-commits 


@@ -0,0 +1,106 @@
+; RUN: llc -mtriple arm64e-apple-darwin \

ahmedbougacha wrote:

It's not too bad; switched

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Ahmed Bougacha via cfe-commits 

https://github.com/ahmedbougacha updated 
https://github.com/llvm/llvm-project/pull/97647

>From 519570896c82887a5dd878fcc16f884857d4fce3 Mon Sep 17 00:00:00 2001
From: Ahmed Bougacha 
Date: Tue, 12 Mar 2024 14:40:17 -0700
Subject: [PATCH 1/3] [AArch64][PAC] Sign block addresses used in indirectbr.

Enabled in clang using:
  -fptrauth-indirect-gotos

and at the IR level using function attribute:
  "ptrauth-indirect-gotos"

Signing uses IA and a per-function integer discriminator.
The discriminator isn't ABI-visible, and is currently:
  ptrauth_string_discriminator(" blockaddress")

A sufficiently sophisticated frontend could benefit from
per-indirectbr discrimination, which would need additional
machinery, such as allowing "ptrauth" bundles on indirectbr.
For our purposes, the simple scheme above is sufficient.
---
 clang/include/clang/Basic/Features.def|   1 +
 clang/include/clang/Basic/LangOptions.def |   1 +
 .../include/clang/Basic/PointerAuthOptions.h  |   3 +
 clang/include/clang/Driver/Options.td |   2 +
 clang/lib/CodeGen/CodeGenFunction.cpp |   2 +
 clang/lib/Driver/ToolChains/Clang.cpp |   3 +
 clang/lib/Frontend/CompilerInvocation.cpp |   6 +-
 .../CodeGen/ptrauth-function-attributes.c |   5 +
 llvm/docs/PointerAuth.md  |  24 
 llvm/include/llvm/CodeGen/AsmPrinter.h|   3 +
 llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp|   6 +-
 llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp |  35 +-
 llvm/lib/Target/AArch64/AArch64FastISel.cpp   |   4 +
 .../Target/AArch64/AArch64ISelLowering.cpp|  53 -
 llvm/lib/Target/AArch64/AArch64ISelLowering.h |   1 +
 llvm/lib/Target/AArch64/AArch64InstrInfo.td   |  18 +++
 llvm/lib/Target/AArch64/AArch64Subtarget.cpp  |  11 ++
 llvm/lib/Target/AArch64/AArch64Subtarget.h|   9 ++
 .../GISel/AArch64InstructionSelector.cpp  |  26 +
 .../CodeGen/AArch64/ptrauth-indirectbr.ll | 106 ++
 20 files changed, 308 insertions(+), 11 deletions(-)
 create mode 100644 llvm/test/CodeGen/AArch64/ptrauth-indirectbr.ll

diff --git a/clang/include/clang/Basic/Features.def 
b/clang/include/clang/Basic/Features.def
index 53f410d3cb4bd..cf800afe08557 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -108,6 +108,7 @@ FEATURE(ptrauth_calls, LangOpts.PointerAuthCalls)
 FEATURE(ptrauth_returns, LangOpts.PointerAuthReturns)
 FEATURE(ptrauth_vtable_pointer_address_discrimination, 
LangOpts.PointerAuthVTPtrAddressDiscrimination)
 FEATURE(ptrauth_vtable_pointer_type_discrimination, 
LangOpts.PointerAuthVTPtrTypeDiscrimination)
+FEATURE(ptrauth_indirect_gotos, LangOpts.PointerAuthIndirectGotos)
 FEATURE(ptrauth_member_function_pointer_type_discrimination, 
LangOpts.PointerAuthCalls)
 FEATURE(ptrauth_init_fini, LangOpts.PointerAuthInitFini)
 EXTENSION(swiftcc,
diff --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 491759e2fcdbb..bdf77a5b35208 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -165,6 +165,7 @@ LANGOPT(ExperimentalLibrary, 1, 0, "enable unstable and 
experimental library fea
 LANGOPT(PointerAuthIntrinsics, 1, 0, "pointer authentication intrinsics")
 LANGOPT(PointerAuthCalls  , 1, 0, "function pointer authentication")
 LANGOPT(PointerAuthReturns, 1, 0, "return pointer authentication")
+LANGOPT(PointerAuthIndirectGotos, 1, 0, "indirect gotos pointer 
authentication")
 LANGOPT(PointerAuthAuthTraps, 1, 0, "pointer authentication failure traps")
 LANGOPT(PointerAuthVTPtrAddressDiscrimination, 1, 0, "incorporate address 
discrimination in authenticated vtable pointers")
 LANGOPT(PointerAuthVTPtrTypeDiscrimination, 1, 0, "incorporate type 
discrimination in authenticated vtable pointers")
diff --git a/clang/include/clang/Basic/PointerAuthOptions.h 
b/clang/include/clang/Basic/PointerAuthOptions.h
index 197d63642ca6d..2711639dbe299 100644
--- a/clang/include/clang/Basic/PointerAuthOptions.h
+++ b/clang/include/clang/Basic/PointerAuthOptions.h
@@ -154,6 +154,9 @@ class PointerAuthSchema {
 };
 
 struct PointerAuthOptions {
+  /// Do indirect goto label addresses need to be authenticated?
+  bool IndirectGotos = false;
+
   /// The ABI for C function pointers.
   PointerAuthSchema FunctionPointers;
 
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 58ca6f2bea9e4..791b7261ddbda 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4228,6 +4228,8 @@ defm ptrauth_vtable_pointer_address_discrimination :
 defm ptrauth_vtable_pointer_type_discrimination :
   OptInCC1FFlag<"ptrauth-vtable-pointer-type-discrimination", "Enable type 
discrimination of vtable pointers">;
 defm ptrauth_init_fini : OptInCC1FFlag<"ptrauth-init-fini", "Enable signing of 
function pointers in init/fini arrays">;
+defm ptrauth_indirect_gotos : OptInCC1FFlag<"ptraut

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Anton Korobeynikov via cfe-commits 


@@ -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();
+  std::optional 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,

asl wrote:

This is a problematic piece of code. If jump table hardening is not enabled, 
then jump tables will be codegenerated using `BRIND` (see the code few lines 
above – directly at the end of `LowerBR_JT`) and the code here does not 
distinguish between indirect branches that has "register" destination and 
indirect branches as a result of jump table lowering...

The code above might check if `Dest` is `AArch64::JumpTableDest32` and do 
nothing in such case.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Anton Korobeynikov via cfe-commits 

https://github.com/asl requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-22 Thread Anton Korobeynikov via cfe-commits 

asl wrote:

FWIW, we are seeing authentication fails when running musl with pauth enabled 
and this PR. Working on reproducer.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-17 Thread Jon Roelofs via cfe-commits 


@@ -0,0 +1,106 @@
+; RUN: llc -mtriple arm64e-apple-darwin \

jroelofs wrote:

does update_test_checks do horrible things to the CHECK lines in this?

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-17 Thread Ahmed Bougacha via cfe-commits 

ahmedbougacha wrote:

> The difference should be not be signed IMO as stated above.

We discussed this in the sync-up, but for the record I don't think we can get 
away with not signing and nothing else.  It defeats the purpose of signing 
these in the first place:  an arbitrary write becomes a branch to an arbitrary 
destination.

One alternative would be to sign the difference, though we would want to have a 
specific way for people to write it to make it predictable (single expression, 
64-bit result;  maybe explicitly as a builtin).  Doesn't help existing code, 
but there's already a high adoption cost for all of these ptrauth changes, this 
seems almost trivial in comparison ;)

A radically different solution could be to lower the blockaddresses without 
signing, then turn the goto into a checked switch.  That allows substitution, 
but that's already the case for signed blockaddresses.
That would easily work with real block addresses, but the switch would look 
nasty.  If we used some dense numbering of the address-taken blocks, and 
lowered `blockaddress()` into that number, then the switch becomes a simple 
jump table, and the only check needed is a range check.  So we'd pretty much 
end up with the "hardened" jump-table from #97666.
Either way this could still be relatively expensive, and pushes the cost from 
address materialization to dispatch, which seems like the worst tradeoff for 
anyone using indirect goto..  So I'm not sure I would want to adopt this for 
MachO, but I'm happy to help and review if folks want to implement this.
Thankfully this isn't ABI visible (beyond the usual problems with code looking 
at the values) so either way we can replace this entirely in the future.

In the meantime, for this PR, I'll look into more actionable errors for the 
unencodable constant expressions, and the single-expression difference, when we 
can diagnose it.  But at the end of the day they'll still be errors.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Eli Friedman via cfe-commits 

efriedma-quic wrote:

The issue is just that clang expects the following to compile.  And with the 
current version of the patch, I think we end up crashing in the backend.

```
@f.x = internal global i32 trunc (i64 sub (i64 ptrtoint (ptr blockaddress(@f, 
%A) to i64), i64 ptrtoint (ptr blockaddress(@f, %B) to i64)) to i32), align 4
```

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Anton Korobeynikov via cfe-commits 

asl wrote:

> Under the proposed ABI, `&&A-&&B` is actually "sign(A)-sign(B)". Which is a 
> constant, but not one which can be represented as a relocation (as far as I 
> know).

Right. The difference should be not be signed IMO as stated above.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Eli Friedman via cfe-commits 

efriedma-quic wrote:

Under the proposed ABI, `&&A-&&B` is actually "sign(A)-sign(B)".  Which is a 
constant, but not one which can be represented as a relocation (as far as I 
know).

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Anton Korobeynikov via cfe-commits 

asl wrote:

> Please make sure you have a testcase for computing the difference between two 
> blockaddresses (`void g(int*); int f() { static int x = &&A-&&B; 
> A:g(&x);B:return x; }`). Not sure how you should handle that case.

Will you please clarify what is the problem here? `&&A-&&B` will be resolved by 
a linker (either static or dynamic), so there will be no way to substitute one 
of addresses in this expression. If this is materialized separately, as in:
```
  void* tmp = &&A;
  tmp -= &&B;
  static int x = tmp;
```
in some constructor routine, then both `&&A` an `&&B` would be separately 
authenticated.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 

kovdan01 wrote:

@ahmedbougacha I've updated my review: I've misinterpreted logic a bit 
previously, and thought that a couple of codepaths are not covered by tests. 
It's not true, everything is OK, but please address @efriedma-quic 's concern 
described above so this can be approved and merged.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 

https://github.com/kovdan01 edited 
https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 

https://github.com/kovdan01 deleted 
https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 

https://github.com/kovdan01 deleted 
https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 

https://github.com/kovdan01 edited 
https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 


@@ -1789,6 +1789,9 @@ void Clang::AddAArch64TargetArgs(const ArgList &Args,
   options::OPT_fno_ptrauth_vtable_pointer_type_discrimination);
   Args.addOptInFlag(CmdArgs, options::OPT_fptrauth_init_fini,
 options::OPT_fno_ptrauth_init_fini);
+

kovdan01 wrote:

Nit: there is no empty line between previous `Args.addOptInFlag(...)` 
invocations, so probably this new line should also be deleted.

Feel free to ignore

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 


@@ -0,0 +1,106 @@
+; RUN: llc -mtriple arm64e-apple-darwin \
+; RUN:   -asm-verbose=false -aarch64-enable-collect-loh=false \
+; RUN:   -o - %s | FileCheck %s
+
+; RUN: llc -mtriple arm64e-apple-darwin \
+; RUN:   -global-isel -global-isel-abort=1 -verify-machineinstrs \
+; RUN:   -asm-verbose=false -aarch64-enable-collect-loh=false \
+; RUN:   -o - %s | FileCheck %s
+
+; The discriminator is the same for all blockaddresses in the function.

kovdan01 wrote:

Nit: it looks like that comments which are actually comments and not run/check 
lines use `;;` as prefix instead of `;` in newly added tests. Same applies 
below.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 


@@ -3461,6 +3470,23 @@ bool AArch64InstructionSelector::select(MachineInstr &I) 
{
 return true;
   }
   case TargetOpcode::G_BLOCK_ADDR: {
+Function *BAFn = I.getOperand(1).getBlockAddress()->getFunction();
+if (std::optional BADisc =
+STI.getPtrAuthBlockAddressDiscriminator(*BAFn)) {
+  MIB.buildInstr(TargetOpcode::IMPLICIT_DEF, {AArch64::X16}, {});

kovdan01 wrote:

Is there a test which covers this code path?

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 


@@ -2516,6 +2516,10 @@ bool AArch64FastISel::selectIndirectBr(const Instruction 
*I) {
   if (AddrReg == 0)
 return false;
 
+  // Authenticated indirectbr is not implemented yet.

kovdan01 wrote:

It looks like that a test with FastISel ensuring that we fall back to 
SelectionDAG ISel is missing. I'm OK with adding missing tests later as a 
separate patch.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 


@@ -1866,6 +1876,20 @@ void AArch64AsmPrinter::LowerMOVaddrPAC(const 
MachineInstr &MI) {
   assert(STI->getInstrInfo()->getInstSizeInBytes(MI) >= InstsEmitted * 4);
 }
 
+const MCExpr *
+AArch64AsmPrinter::lowerBlockAddressConstant(const BlockAddress &BA) {
+  const MCExpr *BAE = AsmPrinter::lowerBlockAddressConstant(BA);
+  const Function &Fn = *BA.getFunction();
+
+  if (std::optional BADisc =
+  STI->getPtrAuthBlockAddressDiscriminator(Fn))
+return AArch64AuthMCExpr::create(BAE, *BADisc, AArch64PACKey::IA,
+ /* HasAddressDiversity= */ false,

kovdan01 wrote:

Nit: it looks like that across llvm it's more common not to leave spaces inside 
comment in such case 
https://llvm.org/docs/CodingStandards.html#comment-formatting

```suggestion
 /*HasAddressDiversity=*/ false,
```

Feel free to ignore

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 


@@ -10704,15 +10728,36 @@ SDValue 
AArch64TargetLowering::LowerConstantPool(SDValue Op,
 
 SDValue AArch64TargetLowering::LowerBlockAddress(SDValue Op,
SelectionDAG &DAG) const {
-  BlockAddressSDNode *BA = cast(Op);
+  BlockAddressSDNode *BAN = cast(Op);
+  const BlockAddress *BA = BAN->getBlockAddress();
+
+  if (std::optional BADisc =
+  Subtarget->getPtrAuthBlockAddressDiscriminator(*BA->getFunction())) {
+SDLoc DL(Op);

kovdan01 wrote:

Is there a test which covers this code path?

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 


@@ -412,6 +412,15 @@ class AArch64Subtarget final : public 
AArch64GenSubtargetInfo {
   /// Choose a method of checking LR before performing a tail call.
   AArch64PAuth::AuthCheckMethod getAuthenticatedLRCheckMethod() const;
 
+  /// Compute the integer discriminator for a given BlockAddress constant, if
+  /// blockaddress signing is enabled (using function attribute
+  /// "ptrauth-indirect-gotos").

kovdan01 wrote:

Nit: it might be worth explicitly saying that `std::nullopt` return value 
stands for disabled indirect gotos signing: someone might mistakenly think that 
`std::nullopt` might indicate absent discriminator (say, zero discr by default) 
with signing enabled.

Alternatively, you might consider adding a separate function for determining 
presence of "ptrauth-indirect-gotos" attribute and use just `uint16_t` as a 
return value for `getPtrAuthBlockAddressDiscriminator` (with an assertion 
inserted against presence of "ptrauth-indirect-gotos"). A drawback of such 
approach is that we might occasionally forget to check if indirect gotos are 
signed before calling get discr function, but a benefit is that function names 
will be more expressive IMHO - `getPtrAuthBlockAddressDiscriminator` will only 
get the discriminator and it'll not have a responsibility to check if the 
signing is enabled.

Feel free to ignore.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 

https://github.com/kovdan01 commented:

The changes mostly look reasonable, but there are several comments to be 
answered before the PR can be merged.

Regarding previous @efriedma-quic 's comment about computing the difference 
between two blockaddresses - I suppose it might be OK just to explicitly do not 
support this as for now and implement+test a proper error message (if it's too 
time-consuming to have full support right now).

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 


@@ -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();

kovdan01 wrote:

Nit
```suggestion
  const MachineFunction &MF = DAG.getMachineFunction();
```

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-08 Thread Daniil Kovalev via cfe-commits 

https://github.com/kovdan01 edited 
https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-03 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Ahmed Bougacha (ahmedbougacha)


Changes

Enabled in clang using:

-fptrauth-indirect-gotos

and at the IR level using function attribute:

"ptrauth-indirect-gotos"

Signing uses IA and a per-function integer discriminator. The discriminator 
isn't ABI-visible, and is currently:

ptrauth_string_discriminator(" blockaddress")

A sufficiently sophisticated frontend could benefit from per-indirectbr 
discrimination, which would need additional machinery, such as allowing 
"ptrauth" bundles on indirectbr. For our purposes, the simple scheme above is 
sufficient.

---

Patch is 28.91 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/97647.diff


20 Files Affected:

- (modified) clang/include/clang/Basic/Features.def (+1) 
- (modified) clang/include/clang/Basic/LangOptions.def (+1) 
- (modified) clang/include/clang/Basic/PointerAuthOptions.h (+3) 
- (modified) clang/include/clang/Driver/Options.td (+2) 
- (modified) clang/lib/CodeGen/CodeGenFunction.cpp (+2) 
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+3) 
- (modified) clang/lib/Frontend/CompilerInvocation.cpp (+5-1) 
- (modified) clang/test/CodeGen/ptrauth-function-attributes.c (+5) 
- (modified) llvm/docs/PointerAuth.md (+24) 
- (modified) llvm/include/llvm/CodeGen/AsmPrinter.h (+3) 
- (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+5-1) 
- (modified) llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp (+30-5) 
- (modified) llvm/lib/Target/AArch64/AArch64FastISel.cpp (+4) 
- (modified) llvm/lib/Target/AArch64/AArch64ISelLowering.cpp (+49-4) 
- (modified) llvm/lib/Target/AArch64/AArch64ISelLowering.h (+1) 
- (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.td (+18) 
- (modified) llvm/lib/Target/AArch64/AArch64Subtarget.cpp (+11) 
- (modified) llvm/lib/Target/AArch64/AArch64Subtarget.h (+9) 
- (modified) llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp (+26) 
- (added) llvm/test/CodeGen/AArch64/ptrauth-indirectbr.ll (+106) 


``diff
diff --git a/clang/include/clang/Basic/Features.def 
b/clang/include/clang/Basic/Features.def
index 53f410d3cb4bd..cf800afe08557 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -108,6 +108,7 @@ FEATURE(ptrauth_calls, LangOpts.PointerAuthCalls)
 FEATURE(ptrauth_returns, LangOpts.PointerAuthReturns)
 FEATURE(ptrauth_vtable_pointer_address_discrimination, 
LangOpts.PointerAuthVTPtrAddressDiscrimination)
 FEATURE(ptrauth_vtable_pointer_type_discrimination, 
LangOpts.PointerAuthVTPtrTypeDiscrimination)
+FEATURE(ptrauth_indirect_gotos, LangOpts.PointerAuthIndirectGotos)
 FEATURE(ptrauth_member_function_pointer_type_discrimination, 
LangOpts.PointerAuthCalls)
 FEATURE(ptrauth_init_fini, LangOpts.PointerAuthInitFini)
 EXTENSION(swiftcc,
diff --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 491759e2fcdbb..bdf77a5b35208 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -165,6 +165,7 @@ LANGOPT(ExperimentalLibrary, 1, 0, "enable unstable and 
experimental library fea
 LANGOPT(PointerAuthIntrinsics, 1, 0, "pointer authentication intrinsics")
 LANGOPT(PointerAuthCalls  , 1, 0, "function pointer authentication")
 LANGOPT(PointerAuthReturns, 1, 0, "return pointer authentication")
+LANGOPT(PointerAuthIndirectGotos, 1, 0, "indirect gotos pointer 
authentication")
 LANGOPT(PointerAuthAuthTraps, 1, 0, "pointer authentication failure traps")
 LANGOPT(PointerAuthVTPtrAddressDiscrimination, 1, 0, "incorporate address 
discrimination in authenticated vtable pointers")
 LANGOPT(PointerAuthVTPtrTypeDiscrimination, 1, 0, "incorporate type 
discrimination in authenticated vtable pointers")
diff --git a/clang/include/clang/Basic/PointerAuthOptions.h 
b/clang/include/clang/Basic/PointerAuthOptions.h
index 197d63642ca6d..2711639dbe299 100644
--- a/clang/include/clang/Basic/PointerAuthOptions.h
+++ b/clang/include/clang/Basic/PointerAuthOptions.h
@@ -154,6 +154,9 @@ class PointerAuthSchema {
 };
 
 struct PointerAuthOptions {
+  /// Do indirect goto label addresses need to be authenticated?
+  bool IndirectGotos = false;
+
   /// The ABI for C function pointers.
   PointerAuthSchema FunctionPointers;
 
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 58ca6f2bea9e4..791b7261ddbda 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4228,6 +4228,8 @@ defm ptrauth_vtable_pointer_address_discrimination :
 defm ptrauth_vtable_pointer_type_discrimination :
   OptInCC1FFlag<"ptrauth-vtable-pointer-type-discrimination", "Enable type 
discrimination of vtable pointers">;
 defm ptrauth_init_fini : OptInCC1FFlag<"ptrauth-init-fini", "Enable signing of 
function pointers in init/fini arrays">;
+defm ptrauth_indirect_gotos : OptInCC1FFlag<"ptrauth-indirect-gotos",
+  "En

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-03 Thread Ahmed Bougacha via cfe-commits 

https://github.com/ahmedbougacha ready_for_review 
https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-03 Thread Ahmed Bougacha via cfe-commits 

ahmedbougacha wrote:

> Please make sure you have a testcase for computing the difference between two 
> blockaddresses (void g(int*); int f() { static int x = &&A-&&B; 
> A:g(&x);B:return x; }). Not sure how you should handle that case.

Oh yeah, we can't handle that at all, I don't think!  The best we can do is 
probably to declare it disallowed and try to diagnose it in the frontend as 
best we can..  I don't think we ever looked into that, I'll give it a try

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-03 Thread Eli Friedman via cfe-commits 

efriedma-quic wrote:

Please make sure you have a testcase for computing the difference between two 
blockaddresses (`void g(int*); int f() { static int x = &&A-&&B; 
A:g(&x);B:return x; }`).  Not sure how you should handle that case.

https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-03 Thread Ahmed Bougacha via cfe-commits 

https://github.com/ahmedbougacha updated 
https://github.com/llvm/llvm-project/pull/97647

>From 519570896c82887a5dd878fcc16f884857d4fce3 Mon Sep 17 00:00:00 2001
From: Ahmed Bougacha 
Date: Tue, 12 Mar 2024 14:40:17 -0700
Subject: [PATCH] [AArch64][PAC] Sign block addresses used in indirectbr.

Enabled in clang using:
  -fptrauth-indirect-gotos

and at the IR level using function attribute:
  "ptrauth-indirect-gotos"

Signing uses IA and a per-function integer discriminator.
The discriminator isn't ABI-visible, and is currently:
  ptrauth_string_discriminator(" blockaddress")

A sufficiently sophisticated frontend could benefit from
per-indirectbr discrimination, which would need additional
machinery, such as allowing "ptrauth" bundles on indirectbr.
For our purposes, the simple scheme above is sufficient.
---
 clang/include/clang/Basic/Features.def|   1 +
 clang/include/clang/Basic/LangOptions.def |   1 +
 .../include/clang/Basic/PointerAuthOptions.h  |   3 +
 clang/include/clang/Driver/Options.td |   2 +
 clang/lib/CodeGen/CodeGenFunction.cpp |   2 +
 clang/lib/Driver/ToolChains/Clang.cpp |   3 +
 clang/lib/Frontend/CompilerInvocation.cpp |   6 +-
 .../CodeGen/ptrauth-function-attributes.c |   5 +
 llvm/docs/PointerAuth.md  |  24 
 llvm/include/llvm/CodeGen/AsmPrinter.h|   3 +
 llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp|   6 +-
 llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp |  35 +-
 llvm/lib/Target/AArch64/AArch64FastISel.cpp   |   4 +
 .../Target/AArch64/AArch64ISelLowering.cpp|  53 -
 llvm/lib/Target/AArch64/AArch64ISelLowering.h |   1 +
 llvm/lib/Target/AArch64/AArch64InstrInfo.td   |  18 +++
 llvm/lib/Target/AArch64/AArch64Subtarget.cpp  |  11 ++
 llvm/lib/Target/AArch64/AArch64Subtarget.h|   9 ++
 .../GISel/AArch64InstructionSelector.cpp  |  26 +
 .../CodeGen/AArch64/ptrauth-indirectbr.ll | 106 ++
 20 files changed, 308 insertions(+), 11 deletions(-)
 create mode 100644 llvm/test/CodeGen/AArch64/ptrauth-indirectbr.ll

diff --git a/clang/include/clang/Basic/Features.def 
b/clang/include/clang/Basic/Features.def
index 53f410d3cb4bd..cf800afe08557 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -108,6 +108,7 @@ FEATURE(ptrauth_calls, LangOpts.PointerAuthCalls)
 FEATURE(ptrauth_returns, LangOpts.PointerAuthReturns)
 FEATURE(ptrauth_vtable_pointer_address_discrimination, 
LangOpts.PointerAuthVTPtrAddressDiscrimination)
 FEATURE(ptrauth_vtable_pointer_type_discrimination, 
LangOpts.PointerAuthVTPtrTypeDiscrimination)
+FEATURE(ptrauth_indirect_gotos, LangOpts.PointerAuthIndirectGotos)
 FEATURE(ptrauth_member_function_pointer_type_discrimination, 
LangOpts.PointerAuthCalls)
 FEATURE(ptrauth_init_fini, LangOpts.PointerAuthInitFini)
 EXTENSION(swiftcc,
diff --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 491759e2fcdbb..bdf77a5b35208 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -165,6 +165,7 @@ LANGOPT(ExperimentalLibrary, 1, 0, "enable unstable and 
experimental library fea
 LANGOPT(PointerAuthIntrinsics, 1, 0, "pointer authentication intrinsics")
 LANGOPT(PointerAuthCalls  , 1, 0, "function pointer authentication")
 LANGOPT(PointerAuthReturns, 1, 0, "return pointer authentication")
+LANGOPT(PointerAuthIndirectGotos, 1, 0, "indirect gotos pointer 
authentication")
 LANGOPT(PointerAuthAuthTraps, 1, 0, "pointer authentication failure traps")
 LANGOPT(PointerAuthVTPtrAddressDiscrimination, 1, 0, "incorporate address 
discrimination in authenticated vtable pointers")
 LANGOPT(PointerAuthVTPtrTypeDiscrimination, 1, 0, "incorporate type 
discrimination in authenticated vtable pointers")
diff --git a/clang/include/clang/Basic/PointerAuthOptions.h 
b/clang/include/clang/Basic/PointerAuthOptions.h
index 197d63642ca6d..2711639dbe299 100644
--- a/clang/include/clang/Basic/PointerAuthOptions.h
+++ b/clang/include/clang/Basic/PointerAuthOptions.h
@@ -154,6 +154,9 @@ class PointerAuthSchema {
 };
 
 struct PointerAuthOptions {
+  /// Do indirect goto label addresses need to be authenticated?
+  bool IndirectGotos = false;
+
   /// The ABI for C function pointers.
   PointerAuthSchema FunctionPointers;
 
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 58ca6f2bea9e4..791b7261ddbda 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4228,6 +4228,8 @@ defm ptrauth_vtable_pointer_address_discrimination :
 defm ptrauth_vtable_pointer_type_discrimination :
   OptInCC1FFlag<"ptrauth-vtable-pointer-type-discrimination", "Enable type 
discrimination of vtable pointers">;
 defm ptrauth_init_fini : OptInCC1FFlag<"ptrauth-init-fini", "Enable signing of 
function pointers in init/fini arrays">;
+defm ptrauth_indirect_gotos : OptInCC1FFlag<"ptrauth-in

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-03 Thread via cfe-commits 

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 4a1fdeb04d10f5562687568ea8c494b3ef46c587 
f0d8af86161c6037e9e0d1fe800e5876dd090092 -- 
clang/include/clang/Basic/PointerAuthOptions.h 
clang/lib/CodeGen/CodeGenFunction.cpp clang/lib/Driver/ToolChains/Clang.cpp 
clang/lib/Frontend/CompilerInvocation.cpp 
clang/test/CodeGen/ptrauth-function-attributes.c 
llvm/include/llvm/CodeGen/AsmPrinter.h 
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp 
llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp 
llvm/lib/Target/AArch64/AArch64FastISel.cpp 
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp 
llvm/lib/Target/AArch64/AArch64ISelLowering.h 
llvm/lib/Target/AArch64/AArch64Subtarget.cpp 
llvm/lib/Target/AArch64/AArch64Subtarget.h 
llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
``





View the diff from clang-format here.


``diff
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp 
b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 8a373c3a46..8978b254f4 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -10689,8 +10689,7 @@ 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 {
+SDValue AArch64TargetLowering::LowerBRIND(SDValue Op, SelectionDAG &DAG) const 
{
   MachineFunction &MF = DAG.getMachineFunction();
   std::optional BADisc =
   Subtarget->getPtrAuthBlockAddressDiscriminator(MF.getFunction());
@@ -10745,8 +10744,8 @@ SDValue 
AArch64TargetLowering::LowerBlockAddress(SDValue Op,
 SDValue AddrDisc = DAG.getRegister(AArch64::XZR, MVT::i64);
 
 SDNode *MOV =
-  DAG.getMachineNode(AArch64::MOVaddrPAC, DL, {MVT::Other, MVT::Glue},
- {TargetBA, Key, AddrDisc, Disc});
+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));
   }

``




https://github.com/llvm/llvm-project/pull/97647
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [AArch64][PAC] Sign block addresses used in indirectbr. (PR #97647)

2024-07-03 Thread Ahmed Bougacha via cfe-commits 

https://github.com/ahmedbougacha created 
https://github.com/llvm/llvm-project/pull/97647

Enabled in clang using:

-fptrauth-indirect-gotos

and at the IR level using function attribute:

"ptrauth-indirect-gotos"

Signing uses IA and a per-function integer discriminator. The discriminator 
isn't ABI-visible, and is currently:

ptrauth_string_discriminator(" blockaddress")

A sufficiently sophisticated frontend could benefit from per-indirectbr 
discrimination, which would need additional machinery, such as allowing 
"ptrauth" bundles on indirectbr. For our purposes, the simple scheme above is 
sufficient.

>From f0d8af86161c6037e9e0d1fe800e5876dd090092 Mon Sep 17 00:00:00 2001
From: Ahmed Bougacha 
Date: Tue, 12 Mar 2024 14:40:17 -0700
Subject: [PATCH] [AArch64][PAC] Sign block addresses used in indirectbr.

Enabled in clang using:
  -fptrauth-indirect-gotos

and at the IR level using function attribute:
  "ptrauth-indirect-gotos"

Signing uses IA and a per-function integer discriminator.
The discriminator isn't ABI-visible, and is currently:
  ptrauth_string_discriminator(" blockaddress")

A sufficiently sophisticated frontend could benefit from
per-indirectbr discrimination, which would need additional
machinery, such as allowing "ptrauth" bundles on indirectbr.
For our purposes, the simple scheme above is sufficient.
---
 clang/include/clang/Basic/Features.def|   1 +
 clang/include/clang/Basic/LangOptions.def |   1 +
 .../include/clang/Basic/PointerAuthOptions.h  |   3 +
 clang/include/clang/Driver/Options.td |   2 +
 clang/lib/CodeGen/CodeGenFunction.cpp |   2 +
 clang/lib/Driver/ToolChains/Clang.cpp |   3 +
 clang/lib/Frontend/CompilerInvocation.cpp |   6 +-
 .../CodeGen/ptrauth-function-attributes.c |   5 +
 llvm/docs/PointerAuth.md  |  24 
 llvm/include/llvm/CodeGen/AsmPrinter.h|   3 +
 llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp|   6 +-
 llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp |  35 +-
 llvm/lib/Target/AArch64/AArch64FastISel.cpp   |   4 +
 .../Target/AArch64/AArch64ISelLowering.cpp|  54 -
 llvm/lib/Target/AArch64/AArch64ISelLowering.h |   1 +
 llvm/lib/Target/AArch64/AArch64InstrInfo.td   |  18 +++
 llvm/lib/Target/AArch64/AArch64Subtarget.cpp  |  11 ++
 llvm/lib/Target/AArch64/AArch64Subtarget.h|   9 ++
 .../GISel/AArch64InstructionSelector.cpp  |  26 +
 .../CodeGen/AArch64/ptrauth-indirectbr.ll | 106 ++
 20 files changed, 309 insertions(+), 11 deletions(-)
 create mode 100644 llvm/test/CodeGen/AArch64/ptrauth-indirectbr.ll

diff --git a/clang/include/clang/Basic/Features.def 
b/clang/include/clang/Basic/Features.def
index 53f410d3cb4bd..cf800afe08557 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -108,6 +108,7 @@ FEATURE(ptrauth_calls, LangOpts.PointerAuthCalls)
 FEATURE(ptrauth_returns, LangOpts.PointerAuthReturns)
 FEATURE(ptrauth_vtable_pointer_address_discrimination, 
LangOpts.PointerAuthVTPtrAddressDiscrimination)
 FEATURE(ptrauth_vtable_pointer_type_discrimination, 
LangOpts.PointerAuthVTPtrTypeDiscrimination)
+FEATURE(ptrauth_indirect_gotos, LangOpts.PointerAuthIndirectGotos)
 FEATURE(ptrauth_member_function_pointer_type_discrimination, 
LangOpts.PointerAuthCalls)
 FEATURE(ptrauth_init_fini, LangOpts.PointerAuthInitFini)
 EXTENSION(swiftcc,
diff --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 491759e2fcdbb..bdf77a5b35208 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -165,6 +165,7 @@ LANGOPT(ExperimentalLibrary, 1, 0, "enable unstable and 
experimental library fea
 LANGOPT(PointerAuthIntrinsics, 1, 0, "pointer authentication intrinsics")
 LANGOPT(PointerAuthCalls  , 1, 0, "function pointer authentication")
 LANGOPT(PointerAuthReturns, 1, 0, "return pointer authentication")
+LANGOPT(PointerAuthIndirectGotos, 1, 0, "indirect gotos pointer 
authentication")
 LANGOPT(PointerAuthAuthTraps, 1, 0, "pointer authentication failure traps")
 LANGOPT(PointerAuthVTPtrAddressDiscrimination, 1, 0, "incorporate address 
discrimination in authenticated vtable pointers")
 LANGOPT(PointerAuthVTPtrTypeDiscrimination, 1, 0, "incorporate type 
discrimination in authenticated vtable pointers")
diff --git a/clang/include/clang/Basic/PointerAuthOptions.h 
b/clang/include/clang/Basic/PointerAuthOptions.h
index 197d63642ca6d..2711639dbe299 100644
--- a/clang/include/clang/Basic/PointerAuthOptions.h
+++ b/clang/include/clang/Basic/PointerAuthOptions.h
@@ -154,6 +154,9 @@ class PointerAuthSchema {
 };
 
 struct PointerAuthOptions {
+  /// Do indirect goto label addresses need to be authenticated?
+  bool IndirectGotos = false;
+
   /// The ABI for C function pointers.
   PointerAuthSchema FunctionPointers;
 
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 58ca6f