Re: [PATCH v4 9/9] aarch64: Add memtag-stack tests

[Claudiu Zissulescu-Ianculescu]

Hi,

On 8/22/25 5:51 PM, Richard Sandiford wrote:
>> +/* FIXME - scan-assembler-times-not subg ?  */
>> +/* FIXME - generate stgp instead of stg + str ?  */
> 
> Were you planning to address these in this series, or are they TODOs
> for future work?  Similarly for the other FIXMEs.

I am planning a new set of patches that will make use of stgp, and other
instructions. But it is not part of this set.

>> +/* { dg-final { scan-assembler-times {\tirg\t} 1 } } */
>> +/* { dg-final { scan-assembler-times {stg\t...?, \[sp, 48\]\n} 1 } } */
>> +/* { dg-final { scan-assembler-times {st2g\t...?, \[sp, 32\]\n} 1 } } */
>> +/* { dg-final { scan-assembler-times {stg\t...?, \[sp, 32\]\n} 1 } } */
>> +/* { dg-final { scan-assembler-times {stg\t...?, \[sp, 64\]\n} 2 } } */
> 
> I'm not sure this makes it sufficiently clear what bytes [16, 31] are
> being used for.

I have added this text:

/* 3 stack vars need 48 bytes (16 bytes granule x 3).  Each granule
holds the
   local variable stack address (8 bytes), and a padding (8 bytes).  The
rest
   of the stack holds LR and temporary varaibles (i.e., x19, x20, and x29).

   Expected: 3 stg to tag, 1 st2g + 1 stg to untag.  */

I hope it helps.


> 
>> [...]
>> diff --git a/gcc/testsuite/gcc.target/aarch64/memtag/large-array.c 
>> b/gcc/testsuite/gcc.target/aarch64/memtag/large-array.c
>> new file mode 100644
>> index 00000000000..f233ce04d35
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/memtag/large-array.c
>> @@ -0,0 +1,24 @@
>> +/* { dg-do compile } */
>> +/* { dg-additional-options "-O2" } */
>> +/* FIXME - add other checks later. For now make sure this does not ICE.  */
> 
> It looks from the dg-finals below that the test already does more than that.

Removed this one, no need for that FIXME, thank you for spoting it out.

> 

>> +/* FIXME - If x doesnt escape the function, why should MTE tagging be done 
>> for
>> +   x ?  */
> 
> Do we?  I would have expected x to be optimised away by gimple.  Does that
> not happen?
 > It's not really clear to me what this is testing.  Is it an anti-ICE
test?
> (No need to change it, just curious.)

Compiling with -O0, the x var is placed on the stack, thus the MTE stack
sanitizer is marking it accordingly. The FIXME is not accurate, and I
remove it. Thank you.



>> +proc check_effective_target_memtag_exec {} {
>> +    if ![check_runtime memtag_exec {
>> +      }
>> +    return 1;
>> +}
> 
> I might have said this before, but this seems like it should go in
> target-supports.exp.

Moved.

>> +/* { dg-final { scan-assembler-not "irg" } } */
>> +/* { dg-final { scan-assembler-not "stg" } } */
>> +/* { dg-final { scan-assembler-not "st2g" } } */
>> +/* { dg-final { scan-assembler-not "subg" } } */
>> +/* { dg-final { scan-assembler-not "addg" } } */
> 
> It's probably worth adding \t to the beginning of each string,
> so that we don't accidentally match fragments of filenames.
> 

Done. I also checked and corrected other tests too.

>> diff --git a/gcc/testsuite/gcc.target/aarch64/memtag/texec-1.c 
>> b/gcc/testsuite/gcc.target/aarch64/memtag/texec-1.c
>> new file mode 100644
>> index 00000000000..b63619b04d3
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/memtag/texec-1.c
>> @@ -0,0 +1,27 @@
>> +/* { dg-do run { xfail *-*-* } } */
> 
> Why are all but one of the run tests XFAILed?

I have three tests which I am expecting to XFail, and one which is
passing. The reason is to check if the MTE exceptions are working by
triggering the exception handler which returns exit(1).

>> +/* { dg-skip-if "" { *-*-* }  { "-O0" } { "" } } */
> 
> The last line shouldn't be necessary, since AFAICT each test is only run once,
> with -O2 in this case.
> 

Removed.

Thank you for your help. I will come back asap with the revisited
version of the patches which needed more attention.

Claudiu