[Bug libgcc/98952] New: powerpc*: __trampoline_setup inverted test for trampoline size

[Bug libgcc/98952] New: powerpc*: __trampoline_setup inverted test for trampoline size

[tuliom at ascii dot art.br]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

            Bug ID: 98952
           Summary: powerpc*: __trampoline_setup inverted test for
                    trampoline size
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libgcc
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tuliom at ascii dot art.br
  Target Milestone: ---

In tramp.S, we have the following:

/* R3 = stack address to store trampoline */
/* R4 = length of trampoline area */
/* R5 = function address */
/* R6 = static chain */

FUNC_START(__trampoline_setup)
...
        li      r8,trampoline_size      /* verify that the trampoline is big
enough */
        cmpw    cr1,r8,r4
...
        blt     cr1,.Labort

It's aborting if r8 < r4.
However, I expected it to abort if r4 < r8, which means the allocated
trampoline area is not enough to fit the trampoline.

One could replace li + cmpw with just:

        cmpwi    cr1,r4,trampoline_size

I can't reproduce this issue on GCC because the allocated length (r4) is always
equals to the required length (r8).

However, this happens when mixing other compilers, e.g.
https://github.com/JuliaLang/julia/issues/32154#issuecomment-766536590

[Bug libgcc/98952] powerpc*: __trampoline_setup inverted test for trampoline size

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |meissner at gcc dot gnu.org

--- Comment #1 from Peter Bergner <bergner at gcc dot gnu.org> ---
git blame seems to show Mike was the owner/writer of this code.  Adding him to
the CC list.

[Bug libgcc/98952] powerpc*: __trampoline_setup inverted test for trampoline size

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

--- Comment #2 from Segher Boessenkool <segher at gcc dot gnu.org> ---
And after that it always copies r4 bytes, too (rounded down to a multiple
of four bytes).

[Bug libgcc/98952] powerpc*: __trampoline_setup inverted test for trampoline size

[meissner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

Michael Meissner <meissner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-04-09
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |meissner at gcc dot gnu.org

[Bug libgcc/98952] powerpc*: __trampoline_setup inverted test for trampoline size

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Michael Meissner <meissner@gcc.gnu.org>:

https://gcc.gnu.org/g:9a30a3f06b908e4e781324c2e813cd1db87119df

commit r12-97-g9a30a3f06b908e4e781324c2e813cd1db87119df
Author: Michael Meissner <meissner@linux.ibm.com>
Date:   Fri Apr 23 18:16:03 2021 -0400

    Fix logic error in 32-bit trampolines.

    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.  I fixed the test
    so the operands are reversed.  I then folded the load immediate into the
    compare instruction.

    I verified this by creating a 32-bit trampoline program and manually
    changing the size of the trampoline to be 48 instead of 40.  The program
    aborted with the larger size.  I updated this code and ran the test again
    and it passed.

    I added a test case that runs on PowerPC 32-bit Linux systems and it calls
    the __trampoline_setup function with a larger buffer size than the
    compiler uses.  The test is not run on 64-bit systems, since the function
    __trampoline_setup is not called.  I also limited the test to just Linux
    systems, in case trampolines are handled differently in other systems.

    libgcc/
    2021-04-23  Michael Meissner  <meissner@linux.ibm.com>

            PR target/98952
            * config/rs6000/tramp.S (__trampoline_setup, elfv1 #ifdef): Fix
            trampoline size comparison in 32-bit by reversing test and
            combining load immediate with compare.
            (__trampoline_setup, elfv2 #ifdef): Fix trampoline size comparison
            in 32-bit by reversing test and combining load immediate with
            compare.

    gcc/testsuite/
    2021-04-23  Michael Meissner  <meissner@linux.ibm.com>

            PR target/98952
            * gcc.target/powerpc/pr98952.c: New test.

[Bug libgcc/98952] powerpc*: __trampoline_setup inverted test for trampoline size

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

--- Comment #4 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Fixed on trunk.  Needs backports to 11 and whatever else is still an open
branch when the backports are done :-)

[Bug libgcc/98952] powerpc*: __trampoline_setup inverted test for trampoline size

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Michael Meissner
<meissner@gcc.gnu.org>:

https://gcc.gnu.org/g:078d2c5efbc6d372411fa2b8f07efb50e23f70b9

commit r8-10923-g078d2c5efbc6d372411fa2b8f07efb50e23f70b9
Author: Michael Meissner <meissner@linux.ibm.com>
Date:   Mon Apr 26 18:26:16 2021 -0400

    [PATCH] Backport fix for PR target/989r2

    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.  I fixed the test
    so the operands are reversed.  I then folded the load immediate into the
    compare instruction.

    I verified this by creating a 32-bit trampoline program and manually
    changing the size of the trampoline to be 48 instead of 40.  The program
    aborted with the larger size.  I updated this code and ran the test again
    and it passed.

    I added a test case that runs on PowerPC 32-bit Linux systems and it calls
    the __trampoline_setup function with a larger buffer size than the
    compiler uses.  The test is not run on 64-bit systems, since the function
    __trampoline_setup is not called.  I also limited the test to just Linux
    systems, in case trampolines are handled differently in other systems.

    libgcc/
    2021-04-26  Michael Meissner  <meissner@linux.ibm.com>

            PR target/98952
            * config/rs6000/tramp.S (__trampoline_setup, elfv1 #ifdef): Fix
            trampoline size comparison in 32-bit by reversing test and
            combining load immediate with compare.  Fix backported from trunk
            change on 4/23, 886b6c1e8af502b69e3f318b9830b73b88215878.
            (__trampoline_setup, elfv2 #ifdef): Fix trampoline size comparison
            in 32-bit by reversing test and combining load immediate with
            compare.

    gcc/testsuite/
    2021-04-26  Michael Meissner  <meissner@linux.ibm.com>

            PR target/98952
            * gcc.target/powerpc/pr98952.c: New test.  Test backported from
            trunk change on 4/23, 886b6c1e8af502b69e3f318b9830b73b88215878.

[Bug libgcc/98952] powerpc*: __trampoline_setup inverted test for trampoline size

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Michael Meissner
<meissner@gcc.gnu.org>:

https://gcc.gnu.org/g:39f46514ca8a78a0fc2e1e0a73d0934fe515a78d

commit r9-9467-g39f46514ca8a78a0fc2e1e0a73d0934fe515a78d
Author: Michael Meissner <meissner@linux.ibm.com>
Date:   Mon Apr 26 19:58:45 2021 -0400

    [PATCH] Backport fix for 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.  I fixed the test
    so the operands are reversed.  I then folded the load immediate into the
    compare instruction.

    I verified this by creating a 32-bit trampoline program and manually
    changing the size of the trampoline to be 48 instead of 40.  The program
    aborted with the larger size.  I updated this code and ran the test again
    and it passed.

    I added a test case that runs on PowerPC 32-bit Linux systems and it calls
    the __trampoline_setup function with a larger buffer size than the
    compiler uses.  The test is not run on 64-bit systems, since the function
    __trampoline_setup is not called.  I also limited the test to just Linux
    systems, in case trampolines are handled differently in other systems.

    libgcc/
    2021-04-26  Michael Meissner  <meissner@linux.ibm.com>

            PR target/98952
            * config/rs6000/tramp.S (__trampoline_setup, elfv1 #ifdef): Fix
            trampoline size comparison in 32-bit by reversing test and
            combining load immediate with compare.  Fix backported from trunk
            change on 4/23, 886b6c1e8af502b69e3f318b9830b73b88215878.
            (__trampoline_setup, elfv2 #ifdef): Fix trampoline size comparison
            in 32-bit by reversing test and combining load immediate with
            compare.

    gcc/testsuite/
    2021-04-26  Michael Meissner  <meissner@linux.ibm.com>

            PR target/98952
            * gcc.target/powerpc/pr98952.c: New test.  Test backported from
            trunk change on 4/23, 886b6c1e8af502b69e3f318b9830b73b88215878.

[Bug libgcc/98952] powerpc*: __trampoline_setup inverted test for trampoline size

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Michael Meissner
<meissner@gcc.gnu.org>:

https://gcc.gnu.org/g:a633f7f96daac391fe0bc5d87427c0e7cf1ab1b3

commit r10-9772-ga633f7f96daac391fe0bc5d87427c0e7cf1ab1b3
Author: Michael Meissner <meissner@linux.ibm.com>
Date:   Mon Apr 26 22:45:02 2021 -0400

    [PATCH] Backport fix for 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.  I fixed the test
    so the operands are reversed.  I then folded the load immediate into the
    compare instruction.

    I verified this by creating a 32-bit trampoline program and manually
    changing the size of the trampoline to be 48 instead of 40.  The program
    aborted with the larger size.  I updated this code and ran the test again
    and it passed.

    I added a test case that runs on PowerPC 32-bit Linux systems and it calls
    the __trampoline_setup function with a larger buffer size than the
    compiler uses.  The test is not run on 64-bit systems, since the function
    __trampoline_setup is not called.  I also limited the test to just Linux
    systems, in case trampolines are handled differently in other systems.

    libgcc/
    2021-04-26  Michael Meissner  <meissner@linux.ibm.com>

            PR target/98952
            * config/rs6000/tramp.S (__trampoline_setup, elfv1 #ifdef): Fix
            trampoline size comparison in 32-bit by reversing test and
            combining load immediate with compare.  Fix backported from trunk
            change on 4/23, 886b6c1e8af502b69e3f318b9830b73b88215878.
            (__trampoline_setup, elfv2 #ifdef): Fix trampoline size comparison
            in 32-bit by reversing test and combining load immediate with
            compare.

    gcc/testsuite/
    2021-04-26  Michael Meissner  <meissner@linux.ibm.com>

            PR target/98952
            * gcc.target/powerpc/pr98952.c: New test.  Test backported from
            trunk change on 4/23, 886b6c1e8af502b69e3f318b9830b73b88215878.

[Bug libgcc/98952] powerpc*: __trampoline_setup inverted test for trampoline size

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Michael Meissner
<meissner@gcc.gnu.org>:

https://gcc.gnu.org/g:9e80a135fffa5c1b36b6007e7e57d303535fbe84

commit r11-8302-g9e80a135fffa5c1b36b6007e7e57d303535fbe84
Author: Michael Meissner <meissner@linux.ibm.com>
Date:   Tue Apr 27 10:52:57 2021 -0400

    [PATCH] Backport fix for 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.  I fixed the test
    so the operands are reversed.  I then folded the load immediate into the
    compare instruction.

    I verified this by creating a 32-bit trampoline program and manually
    changing the size of the trampoline to be 48 instead of 40.  The program
    aborted with the larger size.  I updated this code and ran the test again
    and it passed.

    I added a test case that runs on PowerPC 32-bit Linux systems and it calls
    the __trampoline_setup function with a larger buffer size than the
    compiler uses.  The test is not run on 64-bit systems, since the function
    __trampoline_setup is not called.  I also limited the test to just Linux
    systems, in case trampolines are handled differently in other systems.

    libgcc/
    2021-04-27  Michael Meissner  <meissner@linux.ibm.com>

            PR target/98952
            * config/rs6000/tramp.S (__trampoline_setup, elfv1 #ifdef): Fix
            trampoline size comparison in 32-bit by reversing test and
            combining load immediate with compare.  Fix backported from trunk
            change on 4/23, 886b6c1e8af502b69e3f318b9830b73b88215878.
            (__trampoline_setup, elfv2 #ifdef): Fix trampoline size comparison
            in 32-bit by reversing test and combining load immediate with
            compare.

    gcc/testsuite/
    2021-04-27  Michael Meissner  <meissner@linux.ibm.com>

            PR target/98952
            * gcc.target/powerpc/pr98952.c: New test.  Test backported from
            trunk change on 4/23, 886b6c1e8af502b69e3f318b9830b73b88215878.

[Bug libgcc/98952] powerpc*: __trampoline_setup inverted test for trampoline size

[meissner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

Michael Meissner <meissner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #9 from Michael Meissner <meissner at gcc dot gnu.org> ---
Patches applied to trunk, GCC 11, GCC 10, GCC 9, and GCC 8 branches.

[Bug libgcc/98952] powerpc*: __trampoline_setup inverted test for trampoline size

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98952

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |8.5