[PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Peter Bergner]

We currently only compute the offset for the ROP hash save location in
the stack frame for Altivec compiles.  For non-Altivec compiles when we
emit ROP mitigation instructions, we use a default offset of zero which
corresponds to the backchain save location which will get clobbered on
any call.  The fix is to compute the ROP hash save location for all
compiles.

This passed bootstrap and regtesting on powerpc64le-linux.
Ok for trunk and backports after some burn-in time?

Peter


gcc/
	PR target/115389
	* config/rs6000/rs6000-logue.cc (rs6000_stack_info): Compute
	rop_hash_save_offset for non-Altivec compiles.

gcc/testsuite/
	PR target/115389
	* gcc.target/powerpc/pr115389.c: New test.

diff --git a/gcc/config/rs6000/rs6000-logue.cc b/gcc/config/rs6000/rs6000-logue.cc
index d61a25a5126..cfa8a67a5f3 100644
--- a/gcc/config/rs6000/rs6000-logue.cc
+++ b/gcc/config/rs6000/rs6000-logue.cc
@@ -826,7 +826,14 @@ rs6000_stack_info (void)
 	  info->ehrd_offset -= info->rop_hash_size;
 	}
       else
-	info->ehrd_offset = info->gp_save_offset - ehrd_size;
+	{
+	  info->ehrd_offset = info->gp_save_offset - ehrd_size;
+
+	  /* Adjust for ROP protection.  */
+	  info->rop_hash_save_offset
+	    = info->gp_save_offset - info->rop_hash_size;
+	  info->ehrd_offset -= info->rop_hash_size;
+	}
 
       info->ehcr_offset = info->ehrd_offset - ehcr_size;
       info->cr_save_offset = reg_size; /* first word when 64-bit.  */
diff --git a/gcc/testsuite/gcc.target/powerpc/pr115389.c b/gcc/testsuite/gcc.target/powerpc/pr115389.c
new file mode 100644
index 00000000000..a091ee8a1be
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr115389.c
@@ -0,0 +1,17 @@
+/* PR target/115389 */
+/* { dg-do assemble } */
+/* { dg-options "-O2 -mdejagnu-cpu=power10 -mrop-protect -mno-vsx -mno-altivec -mabi=no-altivec -save-temps" } */
+/* { dg-require-effective-target rop_ok } */
+
+/* Verify we do not emit invalid offsets for our ROP insns.  */
+
+extern void foo (void);
+long
+bar (void)
+{
+  foo ();
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times {\mhashst\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mhashchk\M} 1 } } */

[PING][PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Peter Bergner]

Ping.

On 6/7/24 11:06 PM, Peter Bergner wrote:
> We currently only compute the offset for the ROP hash save location in
> the stack frame for Altivec compiles.  For non-Altivec compiles when we
> emit ROP mitigation instructions, we use a default offset of zero which
> corresponds to the backchain save location which will get clobbered on
> any call.  The fix is to compute the ROP hash save location for all
> compiles.
> 
> This passed bootstrap and regtesting on powerpc64le-linux.
> Ok for trunk and backports after some burn-in time?
> 
> Peter
> 
> 
> gcc/
> 	PR target/115389
> 	* config/rs6000/rs6000-logue.cc (rs6000_stack_info): Compute
> 	rop_hash_save_offset for non-Altivec compiles.
> 
> gcc/testsuite/
> 	PR target/115389
> 	* gcc.target/powerpc/pr115389.c: New test.
> 
> diff --git a/gcc/config/rs6000/rs6000-logue.cc b/gcc/config/rs6000/rs6000-logue.cc
> index d61a25a5126..cfa8a67a5f3 100644
> --- a/gcc/config/rs6000/rs6000-logue.cc
> +++ b/gcc/config/rs6000/rs6000-logue.cc
> @@ -826,7 +826,14 @@ rs6000_stack_info (void)
>  	  info->ehrd_offset -= info->rop_hash_size;
>  	}
>        else
> -	info->ehrd_offset = info->gp_save_offset - ehrd_size;
> +	{
> +	  info->ehrd_offset = info->gp_save_offset - ehrd_size;
> +
> +	  /* Adjust for ROP protection.  */
> +	  info->rop_hash_save_offset
> +	    = info->gp_save_offset - info->rop_hash_size;
> +	  info->ehrd_offset -= info->rop_hash_size;
> +	}
>  
>        info->ehcr_offset = info->ehrd_offset - ehcr_size;
>        info->cr_save_offset = reg_size; /* first word when 64-bit.  */
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr115389.c b/gcc/testsuite/gcc.target/powerpc/pr115389.c
> new file mode 100644
> index 00000000000..a091ee8a1be
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr115389.c
> @@ -0,0 +1,17 @@
> +/* PR target/115389 */
> +/* { dg-do assemble } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mrop-protect -mno-vsx -mno-altivec -mabi=no-altivec -save-temps" } */
> +/* { dg-require-effective-target rop_ok } */
> +
> +/* Verify we do not emit invalid offsets for our ROP insns.  */
> +
> +extern void foo (void);
> +long
> +bar (void)
> +{
> +  foo ();
> +  return 0;
> +}
> +
> +/* { dg-final { scan-assembler-times {\mhashst\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mhashchk\M} 1 } } */

Re: [PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Kewen.Lin]

Hi Peter,

on 2024/6/8 12:06, Peter Bergner wrote:
> We currently only compute the offset for the ROP hash save location in
> the stack frame for Altivec compiles.  For non-Altivec compiles when we
> emit ROP mitigation instructions, we use a default offset of zero which
> corresponds to the backchain save location which will get clobbered on
> any call.  The fix is to compute the ROP hash save location for all
> compiles.

Thanks for fixing this.

> 
> This passed bootstrap and regtesting on powerpc64le-linux.
> Ok for trunk and backports after some burn-in time?
> 
> Peter
> 
> 
> gcc/
> 	PR target/115389
> 	* config/rs6000/rs6000-logue.cc (rs6000_stack_info): Compute
> 	rop_hash_save_offset for non-Altivec compiles.
> 
> gcc/testsuite/
> 	PR target/115389
> 	* gcc.target/powerpc/pr115389.c: New test.
> 
> diff --git a/gcc/config/rs6000/rs6000-logue.cc b/gcc/config/rs6000/rs6000-logue.cc
> index d61a25a5126..cfa8a67a5f3 100644
> --- a/gcc/config/rs6000/rs6000-logue.cc
> +++ b/gcc/config/rs6000/rs6000-logue.cc
> @@ -826,7 +826,14 @@ rs6000_stack_info (void)
>  	  info->ehrd_offset -= info->rop_hash_size;
>  	}
>        else
> -	info->ehrd_offset = info->gp_save_offset - ehrd_size;
> +	{
> +	  info->ehrd_offset = info->gp_save_offset - ehrd_size;
> +
> +	  /* Adjust for ROP protection.  */
> +	  info->rop_hash_save_offset
> +	    = info->gp_save_offset - info->rop_hash_size;
> +	  info->ehrd_offset -= info->rop_hash_size;
> +	}

I understand this is just copied from the if arm, but if I read this right, it can be
simplified as:

diff --git a/gcc/config/rs6000/rs6000-logue.cc b/gcc/config/rs6000/rs6000-logue.cc
index bd5d56ba002..2559c974c6e 100644
--- a/gcc/config/rs6000/rs6000-logue.cc
+++ b/gcc/config/rs6000/rs6000-logue.cc
@@ -817,17 +817,14 @@ rs6000_stack_info (void)
 	  gcc_assert (info->altivec_size == 0
 		      || info->altivec_save_offset % 16 == 0);

-	  /* Adjust for AltiVec case.  */
-	  info->ehrd_offset = info->altivec_save_offset - ehrd_size;
-
- 	  /* Adjust for ROP protection.  */
 	  info->rop_hash_save_offset
 	    = info->altivec_save_offset - info->rop_hash_size;
-	  info->ehrd_offset -= info->rop_hash_size;
 	}
       else
-	info->ehrd_offset = info->gp_save_offset - ehrd_size;
+	info->rop_hash_save_offset = info->gp_save_offset - info->rop_hash_size;

+      info->ehrd_offset = info->rop_hash_save_offset - ehrd_size;
       info->ehcr_offset = info->ehrd_offset - ehcr_size;
       info->cr_save_offset = reg_size; /* first word when 64-bit.  */
       info->lr_save_offset = 2*reg_size;

, both if and else have info->rop_hash_save_offset as the base for offset computation further.

>  
>        info->ehcr_offset = info->ehrd_offset - ehcr_size;
>        info->cr_save_offset = reg_size; /* first word when 64-bit.  */
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr115389.c b/gcc/testsuite/gcc.target/powerpc/pr115389.c
> new file mode 100644
> index 00000000000..a091ee8a1be
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr115389.c
> @@ -0,0 +1,17 @@
> +/* PR target/115389 */
> +/* { dg-do assemble } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mrop-protect -mno-vsx -mno-altivec -mabi=no-altivec -save-temps" } */

I'd expect -mabi=no-altivec is default for -mno-altivec, but specifying it explicitly
looks fine to me. :)

BR,
Kewen

> +/* { dg-require-effective-target rop_ok } */
> +
> +/* Verify we do not emit invalid offsets for our ROP insns.  */
> +
> +extern void foo (void);
> +long
> +bar (void)
> +{
> +  foo ();
> +  return 0;
> +}
> +
> +/* { dg-final { scan-assembler-times {\mhashst\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mhashchk\M} 1 } } */

Re: [PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Peter Bergner]

On 6/13/24 12:35 AM, Kewen.Lin wrote:
>> @@ -826,7 +826,14 @@ rs6000_stack_info (void)
>>  	  info->ehrd_offset -= info->rop_hash_size;
>>  	}
>>        else
>> -	info->ehrd_offset = info->gp_save_offset - ehrd_size;
>> +	{
>> +	  info->ehrd_offset = info->gp_save_offset - ehrd_size;
>> +
>> +	  /* Adjust for ROP protection.  */
>> +	  info->rop_hash_save_offset
>> +	    = info->gp_save_offset - info->rop_hash_size;
>> +	  info->ehrd_offset -= info->rop_hash_size;
>> +	}
> 
> I understand this is just copied from the if arm, but if I read this right, it can be
> simplified as:

Ok, I'll retest with that simplification.





>> +/* { dg-do assemble } */
>> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mrop-protect -mno-vsx -mno-altivec -mabi=no-altivec -save-temps" } */
> 
> I'd expect -mabi=no-altivec is default for -mno-altivec, but specifying it explicitly
> looks fine to me. :)

That's what I expected too! :-)  However, I was surprised to learn that -mno-altivec
does *not* disable TARGET_ALTIVEC_ABI.  I had to explicitly use the -mabi= option to
expose the bug.



Peter

Re: [PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Kewen.Lin]

on 2024/6/13 21:24, Peter Bergner wrote:
> On 6/13/24 12:35 AM, Kewen.Lin wrote:
>>> @@ -826,7 +826,14 @@ rs6000_stack_info (void)
>>>  	  info->ehrd_offset -= info->rop_hash_size;
>>>  	}
>>>        else
>>> -	info->ehrd_offset = info->gp_save_offset - ehrd_size;
>>> +	{
>>> +	  info->ehrd_offset = info->gp_save_offset - ehrd_size;
>>> +
>>> +	  /* Adjust for ROP protection.  */
>>> +	  info->rop_hash_save_offset
>>> +	    = info->gp_save_offset - info->rop_hash_size;
>>> +	  info->ehrd_offset -= info->rop_hash_size;
>>> +	}
>>
>> I understand this is just copied from the if arm, but if I read this right, it can be
>> simplified as:
> 
> Ok, I'll retest with that simplification.

Thanks!
>>> +/* { dg-do assemble } */
>>> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mrop-protect -mno-vsx -mno-altivec -mabi=no-altivec -save-temps" } */
>>
>> I'd expect -mabi=no-altivec is default for -mno-altivec, but specifying it explicitly
>> looks fine to me. :)
> 
> That's what I expected too! :-)  However, I was surprised to learn that -mno-altivec
> does *not* disable TARGET_ALTIVEC_ABI.  I had to explicitly use the -mabi= option to
> expose the bug.

oh, it's surprising, I learn something today! :) I guess it's not intentional but just no
one noticed it, as it seems nonsense to have altivec ABI extension but not using any altivec
features.

BR,
Kewen

Re: [PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Peter Bergner]

On 6/13/24 9:26 PM, Kewen.Lin wrote:
> on 2024/6/13 21:24, Peter Bergner wrote:
>> On 6/13/24 12:35 AM, Kewen.Lin wrote:
>>>> @@ -826,7 +826,14 @@ rs6000_stack_info (void)
>>>>  	  info->ehrd_offset -= info->rop_hash_size;
>>>>  	}
>>>>        else
>>>> -	info->ehrd_offset = info->gp_save_offset - ehrd_size;
>>>> +	{
>>>> +	  info->ehrd_offset = info->gp_save_offset - ehrd_size;
>>>> +
>>>> +	  /* Adjust for ROP protection.  */
>>>> +	  info->rop_hash_save_offset
>>>> +	    = info->gp_save_offset - info->rop_hash_size;
>>>> +	  info->ehrd_offset -= info->rop_hash_size;
>>>> +	}
>>>
>>> I understand this is just copied from the if arm, but if I read this right, it can be
>>> simplified as:
>>
>> Ok, I'll retest with that simplification.

So I retested a normal powerpc64le-linux build (ie, we default to Power8
with Altivec) and it bootstrapped and regtested with no regressions.
I then attempted a --with-cpu=power5 build to test the non-altivec path,
but both the unpatched and patched builds died building libgfortran with
the following error: "error: ‘_Float128’ is not supported on this target".
I believe that is related to PR113652.  I'll kick off the build again,
this time disabling Fortran and seeing if the build completes.



>>>> +/* { dg-do assemble } */
>>>> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mrop-protect -mno-vsx -mno-altivec -mabi=no-altivec -save-temps" } */
>>>
>>> I'd expect -mabi=no-altivec is default for -mno-altivec, but specifying it explicitly
>>> looks fine to me. :)
>>
>> That's what I expected too! :-)  However, I was surprised to learn that -mno-altivec
>> does *not* disable TARGET_ALTIVEC_ABI.  I had to explicitly use the -mabi= option to
>> expose the bug.
> 
> oh, it's surprising, I learn something today! :) I guess it's not intentional but just no
> one noticed it, as it seems nonsense to have altivec ABI extension but not using any altivec
> features.

Agreed!

Peter

Re: [PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Peter Bergner]

On 6/13/24 10:26 PM, Peter Bergner wrote:
> On 6/13/24 9:26 PM, Kewen.Lin wrote:
>>>> I understand this is just copied from the if arm, but if I read this right, it can be
>>>> simplified as:
>>>
>>> Ok, I'll retest with that simplification.
> 
> So I retested a normal powerpc64le-linux build (ie, we default to Power8
> with Altivec) and it bootstrapped and regtested with no regressions.
> I then attempted a --with-cpu=power5 build to test the non-altivec path,
> but both the unpatched and patched builds died building libgfortran with
> the following error: "error: ‘_Float128’ is not supported on this target".
> I believe that is related to PR113652.  I'll kick off the build again,
> this time disabling Fortran and seeing if the build completes.

My bad for calling the --with-cpu=power5 bootstrap build on ELFv2 a "bug".
It's not, since ELFv2 mandates a cpu with at least ISA 2.07 (eg. Power8)
support and some of the libgfortran code was written assuming that, so what
I was trying to do was really not supported (ie, luser error).

That said, the --with-cpu=power5 build without fortran did bootstrap and
regtest with no regressions, so the build did test that code path and
exposed no problems.



>>> That's what I expected too! :-)  However, I was surprised to learn that -mno-altivec
>>> does *not* disable TARGET_ALTIVEC_ABI.  I had to explicitly use the -mabi= option to
>>> expose the bug.
>>
>> oh, it's surprising, I learn something today! :) I guess it's not intentional but just no
>> one noticed it, as it seems nonsense to have altivec ABI extension but not using any altivec
>> features.

Currently, TARGET_ALTIVEC_ABI is defined as:

  #define TARGET_ALTIVEC_ABI rs6000_altivec_abi

Would it make sense to redine it to:

  #define TARGET_ALTIVEC_ABI (TARGET_ALTIVEC && rs6000_altivec_abi)

...or add some code in rs6000 option handling to disable rs6000_altivec_abi
when TARGET_ALTIVEC is false?  ....or do we care enough to even change it? :-)

Peter

Re: [PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Kewen.Lin]

on 2024/6/15 01:05, Peter Bergner wrote:
> On 6/13/24 10:26 PM, Peter Bergner wrote:
>> On 6/13/24 9:26 PM, Kewen.Lin wrote:
>>>>> I understand this is just copied from the if arm, but if I read this right, it can be
>>>>> simplified as:
>>>>
>>>> Ok, I'll retest with that simplification.
>>
>> So I retested a normal powerpc64le-linux build (ie, we default to Power8
>> with Altivec) and it bootstrapped and regtested with no regressions.
>> I then attempted a --with-cpu=power5 build to test the non-altivec path,
>> but both the unpatched and patched builds died building libgfortran with
>> the following error: "error: ‘_Float128’ is not supported on this target".
>> I believe that is related to PR113652.  I'll kick off the build again,
>> this time disabling Fortran and seeing if the build completes.
> 
> My bad for calling the --with-cpu=power5 bootstrap build on ELFv2 a "bug".
> It's not, since ELFv2 mandates a cpu with at least ISA 2.07 (eg. Power8)
> support and some of the libgfortran code was written assuming that, so what
> I was trying to do was really not supported (ie, luser error).
> 
> That said, the --with-cpu=power5 build without fortran did bootstrap and
> regtest with no regressions, so the build did test that code path and
> exposed no problems.

OK, nice!  Thanks!

> 
> 
> 
>>>> That's what I expected too! :-)  However, I was surprised to learn that -mno-altivec
>>>> does *not* disable TARGET_ALTIVEC_ABI.  I had to explicitly use the -mabi= option to
>>>> expose the bug.
>>>
>>> oh, it's surprising, I learn something today! :) I guess it's not intentional but just no
>>> one noticed it, as it seems nonsense to have altivec ABI extension but not using any altivec
>>> features.
> 
> Currently, TARGET_ALTIVEC_ABI is defined as:
> 
>   #define TARGET_ALTIVEC_ABI rs6000_altivec_abi
> 
> Would it make sense to redine it to:
> 
>   #define TARGET_ALTIVEC_ABI (TARGET_ALTIVEC && rs6000_altivec_abi)
> 
> ...or add some code in rs6000 option handling to disable rs6000_altivec_abi
> when TARGET_ALTIVEC is false?  ....or do we care enough to even change it? :-)

Assuming the current code is robust enough (perfectly guarded by some altivec related
condition like this altivec register saving slot), there may not any actual errors,
but considering not surprising people, I'm inclined to add some option handlings for
it, like unsetting rs6000_altivec_abi if !TARGET_ALTIVEC and give some warning if it's
explicitly specified, what do you think?

BR,
Kewen

Re: [PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Peter Bergner]

On 6/16/24 9:10 PM, Kewen.Lin wrote:
> on 2024/6/15 01:05, Peter Bergner wrote:
>> That said, the --with-cpu=power5 build without fortran did bootstrap and
>> regtest with no regressions, so the build did test that code path and
>> exposed no problems.
> 
> OK, nice!  Thanks!

I assume this means you're "OK" with the updated patch, correct?




>> Currently, TARGET_ALTIVEC_ABI is defined as:
>>
>>   #define TARGET_ALTIVEC_ABI rs6000_altivec_abi
>>
>> Would it make sense to redine it to:
>>
>>   #define TARGET_ALTIVEC_ABI (TARGET_ALTIVEC && rs6000_altivec_abi)
>>
>> ...or add some code in rs6000 option handling to disable rs6000_altivec_abi
>> when TARGET_ALTIVEC is false?  ....or do we care enough to even change it? :-)
> 
> Assuming the current code is robust enough (perfectly guarded by some altivec related
> condition like this altivec register saving slot), there may not any actual errors,
> but considering not surprising people, I'm inclined to add some option handlings for
> it, like unsetting rs6000_altivec_abi if !TARGET_ALTIVEC and give some warning if it's
> explicitly specified, what do you think?

I like it, since if Altivec is disabled, having TARGET_ALTIVEC_ABI enabled makes no
sense to me.  That is orthogonal to this bug though, so should be a separate patch.
Do you want to take a stab at writing that or do you want me to do that?


Peter

Re: [PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Kewen.Lin]

on 2024/6/17 10:31, Peter Bergner wrote:
> On 6/16/24 9:10 PM, Kewen.Lin wrote:
>> on 2024/6/15 01:05, Peter Bergner wrote:
>>> That said, the --with-cpu=power5 build without fortran did bootstrap and
>>> regtest with no regressions, so the build did test that code path and
>>> exposed no problems.
>>
>> OK, nice!  Thanks!
> 
> I assume this means you're "OK" with the updated patch, correct?

Yes, OK for trunk, thanks!

>>> Currently, TARGET_ALTIVEC_ABI is defined as:
>>>
>>>   #define TARGET_ALTIVEC_ABI rs6000_altivec_abi
>>>
>>> Would it make sense to redine it to:
>>>
>>>   #define TARGET_ALTIVEC_ABI (TARGET_ALTIVEC && rs6000_altivec_abi)
>>>
>>> ...or add some code in rs6000 option handling to disable rs6000_altivec_abi
>>> when TARGET_ALTIVEC is false?  ....or do we care enough to even change it? :-)
>>
>> Assuming the current code is robust enough (perfectly guarded by some altivec related
>> condition like this altivec register saving slot), there may not any actual errors,
>> but considering not surprising people, I'm inclined to add some option handlings for
>> it, like unsetting rs6000_altivec_abi if !TARGET_ALTIVEC and give some warning if it's
>> explicitly specified, what do you think?
> 
> I like it, since if Altivec is disabled, having TARGET_ALTIVEC_ABI enabled makes no
> sense to me.  That is orthogonal to this bug though, so should be a separate patch.

Yes.

> Do you want to take a stab at writing that or do you want me to do that?

Either is fine for me, then let me give it a shot.

BR,
Kewen

Re: [PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Peter Bergner]

On 6/16/24 9:40 PM, Kewen.Lin wrote:
> on 2024/6/17 10:31, Peter Bergner wrote:
>> On 6/16/24 9:10 PM, Kewen.Lin wrote:
>>> on 2024/6/15 01:05, Peter Bergner wrote:
>>>> That said, the --with-cpu=power5 build without fortran did bootstrap and
>>>> regtest with no regressions, so the build did test that code path and
>>>> exposed no problems.
>>>
>>> OK, nice!  Thanks!
>>
>> I assume this means you're "OK" with the updated patch, correct?
> 
> Yes, OK for trunk, thanks!

Thanks.  We will need backports to GCC 11, as it is broken back to when
ROP was first added then.  I'll let things burn-in on trunk for a couple
of days so Bill's CI builders have a chance to test it on all of our
configs.  





>> Do you want to take a stab at writing that or do you want me to do that?
> 
> Either is fine for me, then let me give it a shot.

Sounds good, thanks.  That will allow me to handle the other ROP issues
I came across, which are reported in PR114759.

Peter

Re: [PATCH] rs6000: Compute rop_hash_save_offset for non-Altivec compiles [PR115389]

[Kewen.Lin]

on 2024/6/17 20:57, Peter Bergner wrote:
> On 6/16/24 9:40 PM, Kewen.Lin wrote:
>> on 2024/6/17 10:31, Peter Bergner wrote:
>>> On 6/16/24 9:10 PM, Kewen.Lin wrote:
>>>> on 2024/6/15 01:05, Peter Bergner wrote:
>>>>> That said, the --with-cpu=power5 build without fortran did bootstrap and
>>>>> regtest with no regressions, so the build did test that code path and
>>>>> exposed no problems.
>>>>
>>>> OK, nice!  Thanks!
>>>
>>> I assume this means you're "OK" with the updated patch, correct?
>>
>> Yes, OK for trunk, thanks!
> 
> Thanks.  We will need backports to GCC 11, as it is broken back to when
> ROP was first added then.  I'll let things burn-in on trunk for a couple
> of days so Bill's CI builders have a chance to test it on all of our
> configs.  

OK for backporting too (if you still need an explicit OK).  Thanks!

BR,
Kewen