[Bug c++/99234] New: Regression: wrong result for 1.0/3.0 when -fno-omit-frame-pointer -frounding-math used together

[Bug c++/99234] New: Regression: wrong result for 1.0/3.0 when -fno-omit-frame-pointer -frounding-math used together

[vz-gcc at zeitlins dot org]

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

            Bug ID: 99234
           Summary: Regression: wrong result for 1.0/3.0 when
                    -fno-omit-frame-pointer -frounding-math used together
           Product: gcc
           Version: 10.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vz-gcc at zeitlins dot org
  Target Milestone: ---

Please see the following test case minimized by cvise:
---------------------------------- >8 --------------------------------------
// When compiled with
//
//  g++ -Wall -O2 -fno-omit-frame-pointer -frounding-math test.cpp
//
// using x86_64-w64-mingw32-g++ version 10-win32 20210110 (GCC) from Debian
// g++-mingw-w64-x86-64-win32 10.2.1-6+24.1 package or gcc version 10.2.0
// (Rev6, Built by MSYS2 project) this test case computes a wrong result for
// the second division below.
#include <cmath>
#include <cstdio>
#include <stdexcept>
#include <string>

double numeric_io_cast(const std::string& from)
{
    char* rendptr;
    double value = std::strtod(from.c_str(), &rendptr);
    if('\0' != *rendptr)
        throw std::logic_error("");

    return value;
}

double numeric_io_cast(char const* from)
{
    return numeric_io_cast(std::string(from));
}

int main(int, char* [])
{
    std::printf("1.0 / 3.0 = %f\n", 1.0 / 3.0); // => 1.0 / 3.0 = 0.333333

    try {
        numeric_io_cast("");
        numeric_io_cast("1e");
    } catch(std::logic_error const&) {
    }

    std::printf("1.0 / 3.0 = %f\n", 1.0 / 3.0); // => 1.0 / 3.0 = 0.000000

    return 0;
}
---------------------------------- >8 --------------------------------------

The comments in the code indicate the output produced when using MinGW gcc
10.2. The same code worked fine with different versions of gcc 8 and also works
when using native Linux gcc 10.2.1. It always works fine for 32 bit compiler
(which produces completely different code).

Also, just in case it can be helpful (even though I'm afraid it isn't...), it
the problem disappears when using the 10.x compilers mentioned above if either
of the following is true:

1. You use -O0 or -O3 (but it gives the same results with -O1 and -O2).
2. You don't use -fno-omit-frame-pointer.
3. You don't use -frounding-math.
4. You use 2.0, 4.0 or 8.0 instead of 3.0 (but it fails similarly with 5.0, 7.0
etc).
5. You use something other than std::logic_error (or a class derived from it
such as std::invalid_argument), i.e. throwing a simple class, even deriving
from std::exception, makes the problem disappear.
6. You remove the (clearly unnecessary!) numeric_io_cast() overload taking
char* and rely on implicit conversion doing the same thing.

Looking at the generated code the problem seems relatively clear: the compiler
assumes the xmm register containing the result is preserved when it's
clobbered. But I have no idea where to start looking in gcc to find why does it
assume this and how to fix this.

I'd also like to note that IMO it's a pretty bad bug because it silently
results in completely wrong behaviour in the code which worked perfectly well
before and, of course, it took quite some time before we actually found what
was going on and why did the tests suddenly start failing in "impossible" ways
after upgrading to MinGW 10.

[Bug target/99234] Regression: wrong result for 1.0/3.0 when -fno-omit-frame-pointer -frounding-math used together

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Works fine on x86_64-linux.  Can you attach preprocessed source (most
developers don't have access to Windows) and -fdump-tree-optimized dump and
assembly you get?

[Bug target/99234] Regression: wrong result for 1.0/3.0 when -fno-omit-frame-pointer -frounding-math used together

[vz-gcc at zeitlins dot org]

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

--- Comment #2 from Vadim Zeitlin <vz-gcc at zeitlins dot org> ---
Created attachment 50250
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50250&action=edit
Compressed output of the preprocessor (-E)

[Bug target/99234] Regression: wrong result for 1.0/3.0 when -fno-omit-frame-pointer -frounding-math used together

[vz-gcc at zeitlins dot org]

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

--- Comment #3 from Vadim Zeitlin <vz-gcc at zeitlins dot org> ---
Created attachment 50251
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50251&action=edit
File created by -fdump-tree-optimized

[Bug target/99234] Regression: wrong result for 1.0/3.0 when -fno-omit-frame-pointer -frounding-math used together

[vz-gcc at zeitlins dot org]

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

--- Comment #4 from Vadim Zeitlin <vz-gcc at zeitlins dot org> ---
Created attachment 50252
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50252&action=edit
Assembly output (-S)

[Bug target/99234] Regression: wrong result for 1.0/3.0 when -fno-omit-frame-pointer -frounding-math used together

[vz-gcc at zeitlins dot org]

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

--- Comment #5 from Vadim Zeitlin <vz-gcc at zeitlins dot org> ---
> Works fine on x86_64-linux.

Yes, I mentioned this :-/

> Can you attach preprocessed source (most developers don't have access to Windows)

If you can install MinGW cross-compiler and Wine, you don't need Windows to
reproduce this, but here is the (compressed, as it's unfortunately quite big
after the standard headers expansion) preprocessed output.

> and -fdump-tree-optimized dump

Also attached.

> and assembly you get?

Attached too. The register I mentioned in the original message is xmm6.

Thanks for looking at this!

[Bug target/99234] Regression: wrong result for 1.0/3.0 when -fno-omit-frame-pointer -frounding-math used together

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu.org,
                   |                            |gingold at gcc dot gnu.org,
                   |                            |jyong at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So the *.optimized dump shows that the division is CSEd by SCCVN the same way
as on Linux, so it is all about the 1/3.0 result living in the %xmm6 register
and exception being thrown in some callee and caught in the same function as
the %xmm6 register is set and later used.
I know nothing about Windows SEH, so CCing some people who have touched it in
the last few years, from the quick skim at the assembly EH is the only
possibility how the proper value in the %xmm6 register wouldn't be restored.

[Bug target/99234] Regression: wrong result for 1.0/3.0 when -fno-omit-frame-pointer -frounding-math used together

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-02-25
             Status|UNCONFIRMED                 |NEW

--- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
I can reproduce.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.2
            Summary|Regression: wrong result    |[10/11 regression] wrong
                   |for 1.0/3.0 when            |result for 1.0/3.0 with -O2
                   |-fno-omit-frame-pointer     |-fno-omit-frame-pointer
                   |-frounding-math used        |-frounding-math
                   |together                    |
           Assignee|unassigned at gcc dot gnu.org      |ebotcazou at gcc dot gnu.org

--- Comment #8 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Investigating.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[jyong at gcc dot gnu.org]

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

jyong at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ktietz70 at googlemail dot com,
                   |                            |lh_mouse at 126 dot com

--- Comment #9 from jyong at gcc dot gnu.org ---
Adding more Windows devs.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[ebotcazou at gcc dot gnu.org]

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

--- Comment #10 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Created attachment 50254
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50254&action=edit
Tentative fix

To be serious tested since it tweaks the SEH prologue.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[lh_mouse at 126 dot com]

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

--- Comment #11 from Liu Hao <lh_mouse at 126 dot com> ---
My personally built GCC [1] didn't seem able to reproduce this, using exact the
same options. The latest commit from GCC repo was
700dcc60b5646cc64ae3ba72a79a7542b4902b50.

Were there any differences when building GCC itself?


[1]
https://github.com/lhmouse/MINGW-packages-dev/blob/master/mingw-w64-gcc-git/PKGBUILD

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #50254|0                           |1
        is obsolete|                            |

--- Comment #12 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Created attachment 50257
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50257&action=edit
Tentative fix #2

More robust version with more verbose commentary.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #50257|0                           |1
        is obsolete|                            |

--- Comment #13 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Created attachment 50258
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50258&action=edit
Final fix

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[lh_mouse at 126 dot com]

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

--- Comment #14 from Liu Hao <lh_mouse at 126 dot com> ---
Created attachment 50259
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50259&action=edit
a much simpler testcase

Added a much simpler testcase.

`xmm6` is set to `123.456`, and an exception is thrown and caught, and `xmm6`
is copied back into a `double` and printed. With GCC 10.2.1 a garbage value is
output. The bug goes away if the `throw` is commented out.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[lh_mouse at 126 dot com]

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

--- Comment #15 from Liu Hao <lh_mouse at 126 dot com> ---
Thanks. The 'Final fix' looks good to me.

I applied it locally and built GCC. With a debugger, I verified that after the
try-catch statement, all non-volatile XMM registers (6-15) had been restored
properly.



```
// x86_64-w64-mingw32-g++ -O2 -fno-omit-frame-pointer test.cc

__attribute__((__noinline__)) void clobber_xmm6()
  {
    __asm__ volatile ("xorpd %%xmm6, %%xmm6" ::: "xmm6");
    throw 42;   // remove this to get expected result
  }

int main()
  {
    static const double xmm5  =  5.123456;
    static const double xmm6  =  6.123456;
    static const double xmm7  =  7.123456;
    static const double xmm8  =  8.123456;
    static const double xmm9  =  9.123456;
    static const double xmm10 = 10.123456;
    static const double xmm11 = 11.123456;
    static const double xmm12 = 12.123456;
    static const double xmm13 = 13.123456;
    static const double xmm14 = 14.123456;
    static const double xmm15 = 15.123456;
    __asm__ volatile ("movsd %0, %%xmm5 " :: "m"(xmm5 ));
    __asm__ volatile ("movsd %0, %%xmm6 " :: "m"(xmm6 ));
    __asm__ volatile ("movsd %0, %%xmm7 " :: "m"(xmm7 ));
    __asm__ volatile ("movsd %0, %%xmm8 " :: "m"(xmm8 ));
    __asm__ volatile ("movsd %0, %%xmm9 " :: "m"(xmm9 ));
    __asm__ volatile ("movsd %0, %%xmm10" :: "m"(xmm10));
    __asm__ volatile ("movsd %0, %%xmm11" :: "m"(xmm11));
    __asm__ volatile ("movsd %0, %%xmm12" :: "m"(xmm12));
    __asm__ volatile ("movsd %0, %%xmm13" :: "m"(xmm13));
    __asm__ volatile ("movsd %0, %%xmm14" :: "m"(xmm14));
    __asm__ volatile ("movsd %0, %%xmm15" :: "m"(xmm15));

    try {
      clobber_xmm6();
    }
    catch(...) { }

    double value;
    __asm__ volatile ("movsd %%xmm6, %0" : "=x"(value));
    __builtin_printf("value = %g\n", value);      // expect `123.456`
  }


```

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[ebotcazou at gcc dot gnu.org]

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

--- Comment #16 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> I applied it locally and built GCC. With a debugger, I verified that after
> the try-catch statement, all non-volatile XMM registers (6-15) had been
> restored properly.

Great, thanks!  I'm putting it through as much automatic testing as I can.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[jakub at gcc dot gnu.org]

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

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Liu Hao from comment #15)
> Thanks. The 'Final fix' looks good to me.
> 
> I applied it locally and built GCC. With a debugger, I verified that after
> the try-catch statement, all non-volatile XMM registers (6-15) had been
> restored properly.
> 
> 
> 
> ```
> // x86_64-w64-mingw32-g++ -O2 -fno-omit-frame-pointer test.cc
> 
> __attribute__((__noinline__)) void clobber_xmm6()
>   {
>     __asm__ volatile ("xorpd %%xmm6, %%xmm6" ::: "xmm6");
>     throw 42;   // remove this to get expected result
>   }
> 
> int main()
>   {
>     static const double xmm5  =  5.123456;
>     static const double xmm6  =  6.123456;
>     static const double xmm7  =  7.123456;
>     static const double xmm8  =  8.123456;
>     static const double xmm9  =  9.123456;
>     static const double xmm10 = 10.123456;
>     static const double xmm11 = 11.123456;
>     static const double xmm12 = 12.123456;
>     static const double xmm13 = 13.123456;
>     static const double xmm14 = 14.123456;
>     static const double xmm15 = 15.123456;
>     __asm__ volatile ("movsd %0, %%xmm5 " :: "m"(xmm5 ));

That would certainly need : "xmm5" in clobbers (etc.).
And isn't really portable anyway, the compiler can choose to put
anything into %xmm6 in between that and the asm before printf.
But, loading the value from memory into some double temporary (with "=x" for
many of them)
in an asm and then e.g. just "+x" them in empty asms and then comparing values
will likely reproduce it too.

>     __asm__ volatile ("movsd %0, %%xmm6 " :: "m"(xmm6 ));
>     __asm__ volatile ("movsd %0, %%xmm7 " :: "m"(xmm7 ));
>     __asm__ volatile ("movsd %0, %%xmm8 " :: "m"(xmm8 ));
>     __asm__ volatile ("movsd %0, %%xmm9 " :: "m"(xmm9 ));
>     __asm__ volatile ("movsd %0, %%xmm10" :: "m"(xmm10));
>     __asm__ volatile ("movsd %0, %%xmm11" :: "m"(xmm11));
>     __asm__ volatile ("movsd %0, %%xmm12" :: "m"(xmm12));
>     __asm__ volatile ("movsd %0, %%xmm13" :: "m"(xmm13));
>     __asm__ volatile ("movsd %0, %%xmm14" :: "m"(xmm14));
>     __asm__ volatile ("movsd %0, %%xmm15" :: "m"(xmm15));
> 
>     try {
>       clobber_xmm6();
>     }
>     catch(...) { }
> 
>     double value;
>     __asm__ volatile ("movsd %%xmm6, %0" : "=x"(value));
>     __builtin_printf("value = %g\n", value);      // expect `123.456`

You mean 6.123456 ?  And in any case, I'd suggest whatever.625 or other
constants which won't suffer from rounding etc.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[lh_mouse at 126 dot com]

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

--- Comment #18 from Liu Hao <lh_mouse at 126 dot com> ---
(In reply to Jakub Jelinek from comment #17)
> 
> That would certainly need : "xmm5" in clobbers (etc.).
> And isn't really portable anyway, the compiler can choose to put
> anything into %xmm6 in between that and the asm before printf.
> But, loading the value from memory into some double temporary (with "=x" for
> many of them)
> in an asm and then e.g. just "+x" them in empty asms and then comparing
> values will likely reproduce it too.
> 

Yes this needs to be polished a bit for automated testing.

As explained above, I was using a debugger, so all I wanted to do was to tell
the compiler XMM5-XMM15 were clobbered, then it would generate a handler that
could recover them (XMM5 is caller-saved so no need to recover it).

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #19 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Eric Botcazou <ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:074226d5aa86cd3de517014acfe34c7f69a2ccc7

commit r11-7439-g074226d5aa86cd3de517014acfe34c7f69a2ccc7
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Mon Mar 1 07:53:05 2021 +0100

    Fix wrong result for 1.0/3.0 at -O2 -fno-omit-frame-pointer -frounding-math

    This wrong-code PR for the C++ compiler on x86-64/Windows is a regression
    in GCC 9 and later, but the underlying issue has probably been there since
    SEH was implemented and is exposed by this comment in config/i386/winnt.c:

      /* SEH records saves relative to the "current" stack pointer, whether
         or not there's a frame pointer in place.  This tracks the current
         stack pointer offset from the CFA.  */
      HOST_WIDE_INT sp_offset;

    That's not what the (current) Microsoft documentation says; instead it
says:

      /* SEH records offsets relative to the lowest address of the fixed stack
         allocation.  If there is no frame pointer, these offsets are from the
         stack pointer; if there is a frame pointer, these offsets are from the
         value of the stack pointer when the frame pointer was established,
i.e.
         the frame pointer minus the offset in the .seh_setframe directive.  */

    That's why the implementation is correct only under the condition that the
    frame pointer be established *after* the fixed stack allocation; as a
matter
    of fact, that's clearly the model underpinning SEH, but is the opposite of
    what is done e.g. on Linux.

    However the issue is mostly papered over in practice because:

      1. SEH forces use_fast_prologue_epilogue to false, which in turns forces
    save_regs_using_mov to false, so the general regs are always pushed when
    they need to be saved, which eliminates the offset computation for them.

      2. As soon as a frame is larger than 240 bytes, the frame pointer is
fixed
    arbitrarily to 128 bytes above the stack pointer, which of course requires
    that it be established after the fixed stack allocation.

    So you need a small frame clobbering one of the call-saved XMM registers in
    order to generate wrong SEH unwind info.

    The attached fix makes sure that the frame pointer is always established
    after the fixed stack allocation by pointing it at or below the lowest used
    register save area, i.e. the SSE save area, and removing the special early
    saves in the prologue; the end result is a uniform prologue sequence for
    SEH whatever the frame size.  And it avoids a discrepancy between cases
    where the number of saved general regs is even and cases where it is odd.

    gcc/
            PR target/99234
            * config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
            point the hard frame pointer to the SSE register save area instead
            of the general register save area.  Perform only minimal adjustment
            for small frames if it is initially not correctly aligned.
            (ix86_expand_prologue): Remove early saves for a SEH target.
            * config/i386/winnt.c (struct seh_frame_state): Document
constraint.
    gcc/testsuite/
            * g++.dg/eh/seh-xmm-unwind.C: New test.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #20 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Eric Botcazou
<ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:49a714e59194a7c549aa6657676a1b4be4520650

commit r10-9397-g49a714e59194a7c549aa6657676a1b4be4520650
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Mon Mar 1 07:53:05 2021 +0100

    Fix wrong result for 1.0/3.0 at -O2 -fno-omit-frame-pointer -frounding-math

    This wrong-code PR for the C++ compiler on x86-64/Windows is a regression
    in GCC 9 and later, but the underlying issue has probably been there since
    SEH was implemented and is exposed by this comment in config/i386/winnt.c:

      /* SEH records saves relative to the "current" stack pointer, whether
         or not there's a frame pointer in place.  This tracks the current
         stack pointer offset from the CFA.  */
      HOST_WIDE_INT sp_offset;

    That's not what the (current) Microsoft documentation says; instead it
says:

      /* SEH records offsets relative to the lowest address of the fixed stack
         allocation.  If there is no frame pointer, these offsets are from the
         stack pointer; if there is a frame pointer, these offsets are from the
         value of the stack pointer when the frame pointer was established,
i.e.
         the frame pointer minus the offset in the .seh_setframe directive.  */

    That's why the implementation is correct only under the condition that the
    frame pointer be established *after* the fixed stack allocation; as a
matter
    of fact, that's clearly the model underpinning SEH, but is the opposite of
    what is done e.g. on Linux.

    However the issue is mostly papered over in practice because:

      1. SEH forces use_fast_prologue_epilogue to false, which in turns forces
    save_regs_using_mov to false, so the general regs are always pushed when
    they need to be saved, which eliminates the offset computation for them.

      2. As soon as a frame is larger than 240 bytes, the frame pointer is
fixed
    arbitrarily to 128 bytes above the stack pointer, which of course requires
    that it be established after the fixed stack allocation.

    So you need a small frame clobbering one of the call-saved XMM registers in
    order to generate wrong SEH unwind info.

    The attached fix makes sure that the frame pointer is always established
    after the fixed stack allocation by pointing it at or below the lowest used
    register save area, i.e. the SSE save area, and removing the special early
    saves in the prologue; the end result is a uniform prologue sequence for
    SEH whatever the frame size.  And it avoids a discrepancy between cases
    where the number of saved general regs is even and cases where it is odd.

    gcc/
            PR target/99234
            * config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
            point the hard frame pointer to the SSE register save area instead
            of the general register save area.  Perform only minimal adjustment
            for small frames if it is initially not correctly aligned.
            (ix86_expand_prologue): Remove early saves for a SEH target.
            * config/i386/winnt.c (struct seh_frame_state): Document
constraint.
    gcc/testsuite/
            * g++.dg/eh/seh-xmm-unwind.C: New test.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #21 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Eric Botcazou
<ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:838adf69533ca15a259d47dd67e056c4101af368

commit r9-9258-g838adf69533ca15a259d47dd67e056c4101af368
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Mon Mar 1 07:53:05 2021 +0100

    Fix wrong result for 1.0/3.0 at -O2 -fno-omit-frame-pointer -frounding-math

    This wrong-code PR for the C++ compiler on x86-64/Windows is a regression
    in GCC 9 and later, but the underlying issue has probably been there since
    SEH was implemented and is exposed by this comment in config/i386/winnt.c:

      /* SEH records saves relative to the "current" stack pointer, whether
         or not there's a frame pointer in place.  This tracks the current
         stack pointer offset from the CFA.  */
      HOST_WIDE_INT sp_offset;

    That's not what the (current) Microsoft documentation says; instead it
says:

      /* SEH records offsets relative to the lowest address of the fixed stack
         allocation.  If there is no frame pointer, these offsets are from the
         stack pointer; if there is a frame pointer, these offsets are from the
         value of the stack pointer when the frame pointer was established,
i.e.
         the frame pointer minus the offset in the .seh_setframe directive.  */

    That's why the implementation is correct only under the condition that the
    frame pointer be established *after* the fixed stack allocation; as a
matter
    of fact, that's clearly the model underpinning SEH, but is the opposite of
    what is done e.g. on Linux.

    However the issue is mostly papered over in practice because:

      1. SEH forces use_fast_prologue_epilogue to false, which in turns forces
    save_regs_using_mov to false, so the general regs are always pushed when
    they need to be saved, which eliminates the offset computation for them.

      2. As soon as a frame is larger than 240 bytes, the frame pointer is
fixed
    arbitrarily to 128 bytes above the stack pointer, which of course requires
    that it be established after the fixed stack allocation.

    So you need a small frame clobbering one of the call-saved XMM registers in
    order to generate wrong SEH unwind info.

    The attached fix makes sure that the frame pointer is always established
    after the fixed stack allocation by pointing it at or below the lowest used
    register save area, i.e. the SSE save area, and removing the special early
    saves in the prologue; the end result is a uniform prologue sequence for
    SEH whatever the frame size.  And it avoids a discrepancy between cases
    where the number of saved general regs is even and cases where it is odd.

    gcc/
            PR target/99234
            * config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
            point the hard frame pointer to the SSE register save area instead
            of the general register save area.  Perform only minimal adjustment
            for small frames if it is initially not correctly aligned.
            (ix86_expand_prologue): Remove early saves for a SEH target.
            * config/i386/winnt.c (struct seh_frame_state): Document
constraint.
    gcc/testsuite/
            * g++.dg/eh/seh-xmm-unwind.C: New test.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

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

--- Comment #22 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Thanks for reporting the problem.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[vz-gcc at zeitlins dot org]

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

--- Comment #23 from Vadim Zeitlin <vz-gcc at zeitlins dot org> ---
(In reply to Eric Botcazou from comment #22)
> Thanks for reporting the problem.

Thanks a lot for fixing it so quickly!

And I've also appreciated the explanation in the commit message, it's nice to
understand what the problem really was, even if it's fixed now.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #24 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Eric Botcazou <ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:357c4350680bf29f0c7a115424e3da11c53b5582

commit r11-7475-g357c4350680bf29f0c7a115424e3da11c53b5582
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Wed Mar 3 12:25:03 2021 +0100

    Fix ICE with pathologically large frames

    gcc/
            PR target/99234
            * config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
            point back the hard frame pointer to its default location when the
            frame is larger than SEH_MAX_FRAME_SIZE.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #25 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Eric Botcazou
<ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:2939b358936bb824330888def98ad848dea41483

commit r10-9409-g2939b358936bb824330888def98ad848dea41483
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Wed Mar 3 12:25:03 2021 +0100

    Fix ICE with pathologically large frames

    gcc/
            PR target/99234
            * config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
            point back the hard frame pointer to its default location when the
            frame is larger than SEH_MAX_FRAME_SIZE.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #26 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Eric Botcazou
<ebotcazou@gcc.gnu.org>:

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

commit r9-9261-gd57d8bb5c528397e8fc21f82f098ad96568ea67f
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Wed Mar 3 12:25:03 2021 +0100

    Fix ICE with pathologically large frames

    gcc/
            PR target/99234
            * config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
            point back the hard frame pointer to its default location when the
            frame is larger than SEH_MAX_FRAME_SIZE.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #27 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Eric Botcazou <ebotcazou@gcc.gnu.org>:

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

commit r12-525-ge8d1ca7d2c344a411779892616c423e157f4aea8
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Wed May 5 22:48:51 2021 +0200

    Fix PR target/100402

    This is a regression for 64-bit Windows present from mainline down to the 9
    branch and introduced by the fix for PR target/99234.  Again SEH, but with
    a twist related to the way MinGW implements setjmp/longjmp, which turns out
    to be piggybacked on SEH with recent versions of MinGW, i.e. the longjmp
    performs a bona-fide unwinding of the stack, because it calls RtlUnwindEx
    with the second argument initially passed to setjmp, which is the result of
    __builtin_frame_address (0) in the MinGW header file:

      define setjmp(BUF) _setjmp((BUF), __builtin_frame_address (0))

    This means that we directly expose the frame pointer to the SEH machinery
    here (unlike with regular exception handling where we use an intermediate
    CFA) and thus that we cannot do whatever we want with it.  The old code
    would leave it unaligned, i.e. not multiple of 16, whereas the new code
    aligns it, but this breaks for some reason; at least it appears that a
    .seh_setframe directive with 0 as second argument always works, so the
    fix aligns it this way.

    gcc/
            PR target/100402
            * config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
            always return the establisher frame for __builtin_frame_address
(0).
    gcc/testsuite/
            * gcc.c-torture/execute/20210505-1.c: New test.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #28 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Eric Botcazou
<ebotcazou@gcc.gnu.org>:

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

commit r11-8358-ge9a8d6852c966e0511f2cfe40224fd81cbeaae24
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Wed May 5 22:48:51 2021 +0200

    Fix PR target/100402

    This is a regression for 64-bit Windows present from mainline down to the 9
    branch and introduced by the fix for PR target/99234.  Again SEH, but with
    a twist related to the way MinGW implements setjmp/longjmp, which turns out
    to be piggybacked on SEH with recent versions of MinGW, i.e. the longjmp
    performs a bona-fide unwinding of the stack, because it calls RtlUnwindEx
    with the second argument initially passed to setjmp, which is the result of
    __builtin_frame_address (0) in the MinGW header file:

      define setjmp(BUF) _setjmp((BUF), __builtin_frame_address (0))

    This means that we directly expose the frame pointer to the SEH machinery
    here (unlike with regular exception handling where we use an intermediate
    CFA) and thus that we cannot do whatever we want with it.  The old code
    would leave it unaligned, i.e. not multiple of 16, whereas the new code
    aligns it, but this breaks for some reason; at least it appears that a
    .seh_setframe directive with 0 as second argument always works, so the
    fix aligns it this way.

    gcc/
            PR target/100402
            * config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
            always return the establisher frame for __builtin_frame_address
(0).
    gcc/testsuite/
            * gcc.c-torture/execute/20210505-1.c: New test.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #29 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Eric Botcazou
<ebotcazou@gcc.gnu.org>:

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

commit r10-9804-ge3abcc56d2604b9d2652b615ff9e68981cb7f79e
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Wed May 5 22:48:51 2021 +0200

    Fix PR target/100402

    This is a regression for 64-bit Windows present from mainline down to the 9
    branch and introduced by the fix for PR target/99234.  Again SEH, but with
    a twist related to the way MinGW implements setjmp/longjmp, which turns out
    to be piggybacked on SEH with recent versions of MinGW, i.e. the longjmp
    performs a bona-fide unwinding of the stack, because it calls RtlUnwindEx
    with the second argument initially passed to setjmp, which is the result of
    __builtin_frame_address (0) in the MinGW header file:

      define setjmp(BUF) _setjmp((BUF), __builtin_frame_address (0))

    This means that we directly expose the frame pointer to the SEH machinery
    here (unlike with regular exception handling where we use an intermediate
    CFA) and thus that we cannot do whatever we want with it.  The old code
    would leave it unaligned, i.e. not multiple of 16, whereas the new code
    aligns it, but this breaks for some reason; at least it appears that a
    .seh_setframe directive with 0 as second argument always works, so the
    fix aligns it this way.

    gcc/
            PR target/100402
            * config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
            always return the establisher frame for __builtin_frame_address
(0).
    gcc/testsuite/
            * gcc.c-torture/execute/20210505-1.c: New test.

[Bug target/99234] [10/11 regression] wrong result for 1.0/3.0 with -O2 -fno-omit-frame-pointer -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #30 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Eric Botcazou
<ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:182dba3fd507487a724090f1c95eea99a1b9ccad

commit r9-9515-g182dba3fd507487a724090f1c95eea99a1b9ccad
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Wed May 5 22:48:51 2021 +0200

    Fix PR target/100402

    This is a regression for 64-bit Windows present from mainline down to the 9
    branch and introduced by the fix for PR target/99234.  Again SEH, but with
    a twist related to the way MinGW implements setjmp/longjmp, which turns out
    to be piggybacked on SEH with recent versions of MinGW, i.e. the longjmp
    performs a bona-fide unwinding of the stack, because it calls RtlUnwindEx
    with the second argument initially passed to setjmp, which is the result of
    __builtin_frame_address (0) in the MinGW header file:

      define setjmp(BUF) _setjmp((BUF), __builtin_frame_address (0))

    This means that we directly expose the frame pointer to the SEH machinery
    here (unlike with regular exception handling where we use an intermediate
    CFA) and thus that we cannot do whatever we want with it.  The old code
    would leave it unaligned, i.e. not multiple of 16, whereas the new code
    aligns it, but this breaks for some reason; at least it appears that a
    .seh_setframe directive with 0 as second argument always works, so the
    fix aligns it this way.

    gcc/
            PR target/100402
            * config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
            always return the establisher frame for __builtin_frame_address
(0).
    gcc/testsuite/
            * gcc.c-torture/execute/20210505-1.c: New test.