Re: [PATCH] Fix logic error in 32-bit trampolines, PR target/98952

[Segher Boessenkool <segher@kernel.crashing.org>]

On Fri, Apr 09, 2021 at 05:09:07PM -0400, Michael Meissner wrote:
> Fix logic error in 32-bit trampolines, PR target/98952.
> 
> The test in the PowerPC 32-bit trampoline support is backwards.  It aborts
> if the trampoline size is greater than the expected size.  It should abort
> when the trampoline size is less than the expected size.

> 	PR target/98952
> 	* config/rs6000/tramp.S (__trampoline_setup): Fix trampoline size
> 	comparison in 32-bit.

> --- a/libgcc/config/rs6000/tramp.S
> +++ b/libgcc/config/rs6000/tramp.S
> @@ -64,8 +64,7 @@ FUNC_START(__trampoline_setup)
>          mflr	r11
>          addi	r7,r11,trampoline_initial-4-.LCF0 /* trampoline address -4 */
>  
> -	li	r8,trampoline_size	/* verify that the trampoline is big enough */
> -	cmpw	cr1,r8,r4
> +	cmpwi	cr1,r4,trampoline_size	/* verify that the trampoline is big enough */
>  	srwi	r4,r4,2		/* # words to move */
>  	addi	r9,r3,-4	/* adjust pointer for lwzu */
>  	mtctr	r4

As Will says, it looks like the ELFv2 version has the same bug.  Please
fix that the same way.

In the commit message and the changelog, point out that you folded the
cmp with the li while you were at it.  It is easier to read code like
this so the change is fine, but do point it out.

Can you test this in a testcase somehow?  That would have found the
ELFv2 case, for example.

Okay for trunk.  Okay for backport to 11 when that branch opens again.
Does this need more backports?  (Those should follow after 11 of
course).

Thanks,


Segher