Re: [RFC] [PATCH, AARCH64] : Using standard patterns for stack protection.

[Marcus Shawcroft <marcus dot shawcroft at gmail dot com>]

Hi Venkat

On 5 February 2014 10:29, Venkataramanan Kumar
<venkataramanan.kumar@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".

>> -/* { dg-do compile { target i?86-*-* x86_64-*-* rs6000-*-* s390x-*-* } } */
>> +/* { dg-do compile { target stack_protection } } */
>>  /* { dg-options "-O2 -fstack-protector-strong" } */
>>
>> Do we need a new effective target test, why is the existing
>> "fstack_protector" not appropriate?
>
> "stack_protector" does a run time test. It failed in cross compilation
> environment and these are compile only tests.

This works fine in my cross environment, how does yours fail?


> Also I thought  richard suggested  me to add a new option for this.
> ref: http://gcc.gnu.org/ml/gcc-patches/2013-11/msg03358.html

I read that comment to mean use an effective target test instead of
matching triples. I don't see that re-using an existing effective
target test contradicts that suggestion.

Looking through the test suite I see that there are:

6 tests that use dg-do compile with dg-require-effective-target fstack_protector

4 tests that use dg-do run with dg-require-effective-target fstack_protector

2 tests that use dg-do run {target native} dg-require-effective-target
fstack_protector

and finally the 2 tests we are discussing that use dg-compile with a
triple test.

so there are already tests in the testsuite that use dg-do compile
with the existing effective target test.

I see no immediately obvious reason why the two tests that require
target native require the native constraint... but I guess that is a
different issue.

The proposed patch moves the triple matching from the test case into
the .exp file, iff the existing run time test is inappropriate, would
it not be better to write a new effective target test that performs a
compile/link test rather resorting to a triple match?

This patch as presented would also result in two effective target
tests with names that are easily confused and provide no hint as to
their different purposes:

fstack_protector
fstack_protection

... if we are going to have two similar effective target tests then
they ought to be named in a fashion that doesn;t lead to confusion in
the future.

Can you respin and split the patch into two please one with the
aarch64 target change the other with the testsuite effective target
change?

Cheers
/Marcus