On 20/01/17 12:36, Jiong Wang wrote:
>
>
> On 20/01/17 11:15, Jiong Wang wrote:
>>
>>
>> On 20/01/17 03:39, Andrew Pinski wrote:
>>> On Fri, Jan 6, 2017 at 3:47 AM, Jiong Wang <jiong.w...@foss.arm.com>
>>> wrote:
>>>> On 11/11/16 18:22, Jiong Wang wrote:
>>>>> As described in the cover letter, this patch implements return address
>>>>> signing
>>>>> for AArch64, it's controlled by the new option:
>>>>>
>>>>> -msign-return-address=[none | non-leaf | all]
>>>>>
>>>>> "none" means don't do return address signing at all on any function.
>>>>> "non-leaf"
>>>>> means only sign non-leaf function. "all" means sign all functions.
>>>>> Return
>>>>> address signing is currently disabled on ILP32. I haven't tested it.
>>>>>
>>>>> The instructions added in the architecture are of 2 kinds.
>>>>>
>>>>> * In the NOP instruction space, which allows binaries to run
>>>>> without any
>>>>> traps
>>>>> on older versions of the architecture. This doesn't give any
>>>>> additional
>>>>> protection on older hardware but allows for the same binary to be
>>>>> used on
>>>>> earlier versions of the architecture and newer versions of the
>>>>> architecture.
>>>>>
>>>>> * New instructions that are only valid for v8.3 and will trap if
>>>>> used on
>>>>> earlier
>>>>> versions of the architecture.
>>>>>
>>>>> At default, once return address signing is enabled, it will only
>>>>> generates
>>>>> NOP
>>>>> instruction.
>>>>>
>>>>> While if -march=armv8.3-a specified, GCC will try to use the most
>>>>> efficient
>>>>> pointer authentication instruction as it can.
>>>>>
>>>>> The architecture has 2 user invisible system keys for signing and
>>>>> creating
>>>>> signed addresses as part of these instructions. For some use case, the
>>>>> user
>>>>> might want to use difference key for different functions. The new
>>>>> option
>>>>> "-msign-return-address-key=key_name" let GCC select the key used for
>>>>> return
>>>>> address signing. Permissible values are "a_key" for A key and
>>>>> "b_key" for
>>>>> B
>>>>> key, and this option are supported by function target attribute and
>>>>> LTO
>>>>> will
>>>>> hopefully just work.
>>>>>
>>>>>
>>>>>
>>>>> gcc/
>>>>> 2016-11-09 Jiong Wang<jiong.w...@arm.com>
>>>>>
>>>>> * config/aarch64/aarch64-opts.h
>>>>> (aarch64_pauth_key_index): New
>>>>> enum.
>>>>> (aarch64_function_type): New enum.
>>>>> * config/aarch64/aarch64-protos.h
>>>>> (aarch64_output_sign_auth_reg):
>>>>> New
>>>>> declaration.
>>>>> * config/aarch64/aarch64.c (aarch64_expand_prologue):
>>>>> Sign return
>>>>> address before it's pushed onto stack.
>>>>> (aarch64_expand_epilogue): Authenticate return address
>>>>> fetched
>>>>> from
>>>>> stack.
>>>>> (aarch64_output_sign_auth_reg): New function.
>>>>> (aarch64_override_options): Sanity check for ILP32 and
>>>>> ISA level.
>>>>> (aarch64_attributes): New function attributes for
>>>>> "sign-return-address",
>>>>> "pauth-key".
>>>>> * config/aarch64/aarch64.md (UNSPEC_AUTH_REG,
>>>>> UNSPEC_AUTH_REG1716,
>>>>> UNSPEC_SIGN_REG, UNSPEC_SIGN_REG1716, UNSPEC_STRIP_REG_SIGN,
>>>>> UNSPEC_STRIP_X30_SIGN): New unspecs.
>>>>> ("*do_return"): Generate combined instructions according
>>>>> to key
>>>>> index.
>>>>> ("sign_reg", "sign_reg1716", "auth_reg", "auth_reg1716",
>>>>> "strip_reg_sign", "strip_lr_sign"): New.
>>>>> * config/aarch64/aarch64.opt (msign-return-address,
>>>>> mpauth-key):
>>>>> New.
>>>>> * config/aarch64/predicates.md (aarch64_const0_const1): New
>>>>> predicate.
>>>>> * doc/extend.texi (AArch64 Function Attributes): Documents
>>>>> "sign-return-address=", "pauth-key".
>>>>> * doc/invoke.texi (AArch64 Options): Documents
>>>>> "-msign-return-address=",
>>>>> "-pauth-key".
>>>>>
>>>>> gcc/testsuite/
>>>>> 2016-11-09 Jiong Wang<jiong.w...@arm.com>
>>>>>
>>>>> * gcc.target/aarch64/return_address_sign_1.c: New testcase.
>>>>> * gcc.target/aarch64/return_address_sign_scope_1.c: New
>>>>> testcase.
>>>>
>>>> Update the patchset according to new DWARF proposal described at
>>>>
>>>> https://gcc.gnu.org/ml/gcc-patches/2016-11/msg03010.html
>>> One of these patches of this patch set break ILP32 building for
>>> aarch64-elf and most likely also aarch64-linux-gnu.
>>>
>>> /home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/libgcc/unwind-dw2.c:
>>>
>>> In function âuw_init_context_1â:
>>> /home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/libgcc/unwind-dw2.c:1567:6:
>>>
>>> internal compiler error: in emit_move_insn, at expr.c:3698
>>> ra = MD_POST_EXTRACT_ROOT_ADDR (ra);
>>> 0x8270cf emit_move_insn(rtx_def*, rtx_def*)
>>> /home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/expr.c:3697
>>>
>>> 0x80867b force_reg(machine_mode, rtx_def*)
>> Must be the Pmode issue under ILP32, I am testing a fix (I don't have
>> full ILP32 environment, so can only test simply by force libgcc build
>> with -mabi=ilp32)
>
> Here is the patch.
>
> For XPACLRI builtin which drops the signature in a pointer, it's
> prototype is "void *foo (void *)"
> FOR PAC/AUT builtin which sign or authenticate a pointer, it's prototype
> is "void *foo (void *, uint64)".
>
> This patch adjusted those modes to make sure they strictly follow the C
> prototype. I also borrow the type define in ARM backend
>
> typedef unsigned _uw64 __attribute__((mode(__DI__)));
>
> And this is need to type cast the salt value which is always DImode.
>
> It passed my local ILP32 cross build.
>
> OK for trunk?
>
> gcc/
> 2017-01-20 Jiong Wang <jiong.w...@arm.com>
> * config/aarch64/aarch64-builtins.c (aarch64_expand_builtin):
> Fix modes
> for AARCH64_PAUTH_BUILTIN_XPACLRI,
> AARCH64_PAUTH_BUILTIN_PACIA1716 and
> AARCH64_PAUTH_BUILTIN_AUTIA1716.
>
> libgcc/
> * config/aarch64/aarch64-unwind.h (_uw64): New typedef.
> (aarch64_post_extract_frame_addr): Cast salt to _uw64.
> (aarch64_post_frob_eh_handler_addr): Likewise.
>
>
Hmm, we currently don't support ILP32 at all for pointer signing (sorry
("Return address signing is only supported for -mabi=lp64");), so I
wonder if it would be better for now to simply suppress all the new
hooks in aarch64-unwind.h ifdef __ILP32__.
R.
> k.patch
>
>
> diff --git a/gcc/config/aarch64/aarch64-builtins.c
> b/gcc/config/aarch64/aarch64-builtins.c
> index 6c6530c..7ef351e 100644
> --- a/gcc/config/aarch64/aarch64-builtins.c
> +++ b/gcc/config/aarch64/aarch64-builtins.c
> @@ -1333,18 +1333,20 @@ aarch64_expand_builtin (tree exp,
> case AARCH64_PAUTH_BUILTIN_PACIA1716:
> case AARCH64_PAUTH_BUILTIN_XPACLRI:
> arg0 = CALL_EXPR_ARG (exp, 0);
> - op0 = force_reg (Pmode, expand_normal (arg0));
> + /* Operand0 should have ptr_mode as its a ptr_type_node, this makes
> both
> + LP64 and ILP32 expand correctly. */
> + op0 = force_reg (ptr_mode, expand_normal (arg0));
>
> if (!target)
> - target = gen_reg_rtx (Pmode);
> + target = gen_reg_rtx (ptr_mode);
> else
> - target = force_reg (Pmode, target);
> + target = force_reg (ptr_mode, target);
>
> emit_move_insn (target, op0);
>
> if (fcode == AARCH64_PAUTH_BUILTIN_XPACLRI)
> {
> - rtx lr = gen_rtx_REG (Pmode, R30_REGNUM);
> + rtx lr = gen_rtx_REG (ptr_mode, R30_REGNUM);
> icode = CODE_FOR_xpaclri;
> emit_move_insn (lr, op0);
> emit_insn (GEN_FCN (icode) ());
> @@ -1353,12 +1355,13 @@ aarch64_expand_builtin (tree exp,
> else
> {
> tree arg1 = CALL_EXPR_ARG (exp, 1);
> - rtx op1 = force_reg (Pmode, expand_normal (arg1));
> + /* Operand1 for either PAC or AUT is a unsigned_intDI_type_node. */
> + rtx op1 = force_reg (DImode, expand_normal (arg1));
> icode = (fcode == AARCH64_PAUTH_BUILTIN_PACIA1716
> ? CODE_FOR_paci1716 : CODE_FOR_auti1716);
>
> - rtx x16_reg = gen_rtx_REG (Pmode, R16_REGNUM);
> - rtx x17_reg = gen_rtx_REG (Pmode, R17_REGNUM);
> + rtx x16_reg = gen_rtx_REG (DImode, R16_REGNUM);
> + rtx x17_reg = gen_rtx_REG (ptr_mode, R17_REGNUM);
> emit_move_insn (x17_reg, op0);
> emit_move_insn (x16_reg, op1);
> emit_insn (GEN_FCN (icode) ());
> diff --git a/libgcc/config/aarch64/aarch64-unwind.h
> b/libgcc/config/aarch64/aarch64-unwind.h
> index a43d965..7dd2549 100644
> --- a/libgcc/config/aarch64/aarch64-unwind.h
> +++ b/libgcc/config/aarch64/aarch64-unwind.h
> @@ -35,6 +35,8 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
> If not, see
> #define MD_FROB_UPDATE_CONTEXT(context, fs) \
> aarch64_frob_update_context (context, fs)
>
> +typedef unsigned _uw64 __attribute__((mode(__DI__)));
> +
> /* Do AArch64 private extraction on ADDR based on context info CONTEXT and
> unwind frame info FS. If ADDR is signed, we do address authentication on
> it
> using CFA of current frame. */
> @@ -45,7 +47,7 @@ aarch64_post_extract_frame_addr (struct _Unwind_Context
> *context,
> {
> if (fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset & 0x1)
> {
> - _Unwind_Word salt = (_Unwind_Word) context->cfa;
> + _uw64 salt = (_uw64) context->cfa;
> return __builtin_aarch64_autia1716 (addr, salt);
> }
> else
> @@ -64,7 +66,7 @@ aarch64_post_frob_eh_handler_addr (struct _Unwind_Context
> *current,
> {
> if (current->flags & RA_A_SIGNED_BIT)
> return __builtin_aarch64_pacia1716 (handler_addr,
> - (_Unwind_Word) current->cfa);
> + (_uw64) current->cfa);
> else
> return handler_addr;
> }
>
