Hi Marcus,
On 14 March 2014 19:42, Marcus Shawcroft <marcus.shawcr...@gmail.com> wrote:
> Hi Venkat
>
> On 5 February 2014 10:29, Venkataramanan Kumar
> <venkataramanan.ku...@linaro.org> wrote:
>> Hi Marcus,
>>
>>> + "ldr\\t%x2, %1\;str\\t%x2, %0\;mov\t%x2,0"
>>> + [(set_attr "length" "12")])
>>>
>>> This pattern emits an opaque sequence of instructions that cannot be
>>> scheduled, is that necessary? Can we not expand individual
>>> instructions or at least split ?
>>
>> Almost all the ports emits a template of assembly instructions.
>> I m not sure why they have to be generated this way.
>> But usage of these pattern is to clear the register that holds canary
>> value immediately after its usage.
>
> I've just read the thread Andrew pointed out, thanks, I'm happy that
> there is a good reason to do it this way. Andrew, thanks for
> providing the background.
>
> + [(set_attr "length" "12")])
> +
>
> These patterns should also set the "type" attribute, a reasonable
> value would be "multiple".
>
I have incorporated your review comments and split the patch into two.
The first patch attached here contains Aarch64 machine descriptions
for the stack protect patterns.
ChangeLog.
2014-03-19 Venkataramanan Kumar <venkataramanan.ku...@linaro.org>
* config/aarch64/aarch64.md (stack_protect_set, stack_protect_test)
(stack_protect_set_<mode>, stack_protect_test_<mode>): Add
machine descriptions for Stack Smashing Protector.
Tested for aarch64-none-linux-gnu target under QEMU .
regards,
Venkat.
Index: gcc/config/aarch64/aarch64.md
===================================================================
--- gcc/config/aarch64/aarch64.md (revision 208609)
+++ gcc/config/aarch64/aarch64.md (working copy)
@@ -102,6 +102,8 @@
UNSPEC_TLSDESC
UNSPEC_USHL_2S
UNSPEC_VSTRUCTDUMMY
+ UNSPEC_SP_SET
+ UNSPEC_SP_TEST
])
(define_c_enum "unspecv" [
@@ -3634,6 +3636,67 @@
DONE;
})
+;; Named patterns for stack smashing protection.
+(define_expand "stack_protect_set"
+ [(match_operand 0 "memory_operand")
+ (match_operand 1 "memory_operand")]
+ ""
+{
+ enum machine_mode mode = GET_MODE (operands[0]);
+
+ emit_insn ((mode == DImode
+ ? gen_stack_protect_set_di
+ : gen_stack_protect_set_si) (operands[0], operands[1]));
+ DONE;
+})
+
+(define_insn "stack_protect_set_<mode>"
+ [(set (match_operand:PTR 0 "memory_operand" "=m")
+ (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m")]
+ UNSPEC_SP_SET))
+ (set (match_scratch:PTR 2 "=&r") (const_int 0))]
+ ""
+ "ldr\\t%x2, %1\;str\\t%x2, %0\;mov\t%x2,0"
+ [(set_attr "length" "12")
+ (set_attr "type" "multiple")])
+
+(define_expand "stack_protect_test"
+ [(match_operand 0 "memory_operand")
+ (match_operand 1 "memory_operand")
+ (match_operand 2)]
+ ""
+{
+
+ rtx result = gen_reg_rtx (Pmode);
+
+ enum machine_mode mode = GET_MODE (operands[0]);
+
+ emit_insn ((mode == DImode
+ ? gen_stack_protect_test_di
+ : gen_stack_protect_test_si) (result,
+ operands[0],
+ operands[1]));
+
+ if (mode == DImode)
+ emit_jump_insn (gen_cbranchdi4 (gen_rtx_EQ (VOIDmode, result, const0_rtx),
+ result, const0_rtx, operands[2]));
+ else
+ emit_jump_insn (gen_cbranchsi4 (gen_rtx_EQ (VOIDmode, result, const0_rtx),
+ result, const0_rtx, operands[2]));
+ DONE;
+})
+
+(define_insn "stack_protect_test_<mode>"
+ [(set (match_operand:PTR 0 "register_operand")
+ (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m")
+ (match_operand:PTR 2 "memory_operand" "m")]
+ UNSPEC_SP_TEST))
+ (clobber (match_scratch:PTR 3 "=&r"))]
+ ""
+ "ldr\t%x3, %x1\;ldr\t%x0, %x2\;eor\t%x0, %x3, %x0"
+ [(set_attr "length" "12")
+ (set_attr "type" "multiple")])
+
;; AdvSIMD Stuff
(include "aarch64-simd.md")
