https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97822
--- Comment #16 from Werner Zeh <werner.zeh at coreboot dot org> ---
(In reply to Uroš Bizjak from comment #14)
> (In reply to Werner Zeh from comment #7)
>
> > The analysis has shown that the observed behavior was introduced with commit
> > 122f9da15d1db58bd5f96a8a49d81d529ca07a1e between version 7 and 8.
> >
> > Looking deeper into this change, you see the following change made in
> > gcc/config/i386/i386.c:
> > 1 static inline bool
> > 2 fp_valid_at (HOST_WIDE_INT cfa_offset)
> > 3 {
> > 4 const struct machine_frame_state &fs = cfun->machine->fs;
> > 5 - return fs.fp_valid && !(fs.sp_valid && fs.sp_realigned
> > 6 - && cfa_offset > fs.sp_realigned_offset);
> > 7 + if (fs.sp_realigned && cfa_offset > fs.sp_realigned_fp_last)
> > 8 + {
> > 9 + /* Validate that the cfa_offset isn't in a "no-man's land". */
> > 10 + gcc_assert (cfa_offset >= fs.sp_realigned_offset);
> > 11 + return false;
> > 12 + }
> > 13 + return fs.fp_valid;
> > 14 }
> >
> > The problem is that this change modified the condition in line 5 to line 7,
> > which removed the term fs.sp_valid.
>
> I think the above analysis is correct. Also, in ix86_expand_prologue we have
> the following comment:
>
> /* SEH unwind emit doesn't currently support REG_CFA_EXPRESSION, which
> is needed to describe where a register is saved using a realigned
> stack pointer, so we need to invalidate the stack pointer for that
> target. */
> if (TARGET_SEH)
> m->fs.sp_valid = false;
>
> This is Werner's proposed change in a patch form:
>
> --cut here--
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index 722c1713a6d..f5283c2c6b7 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -7403,7 +7403,7 @@ static inline bool
> fp_valid_at (HOST_WIDE_INT cfa_offset)
> {
> const struct machine_frame_state &fs = cfun->machine->fs;
> - if (fs.sp_realigned && cfa_offset > fs.sp_realigned_fp_last)
> + if (fs.sp_valid && fs.sp_realigned && cfa_offset >
> fs.sp_realigned_fp_last)
> {
> /* Validate that the cfa_offset isn't in a "no-man's land". */
> gcc_assert (cfa_offset >= fs.sp_realigned_offset);
> --cut here--
>
> @Jonathan, can you please bootstrap and regression test the patch on MingW?
> I will test it on linux, but according to the above comment I expect it will
> not make any impact on non-SEH target.
>
> @Werner, can you please post the patch following [1] to gcc-patches@ mailing
> list for eventual approval by MingW maintainer? The patch submission needs
> your Signed-off-By: tag.
>
> [1] https://gcc.gnu.org/contribute.html
Thanks for confirming, Uros.
In the meantime we bisected on top of latest commit and found that the behavior
was changed in commit dea7b9a78b11b5ca0c85b971521144ba07a66aca. Here, the
default definition of STACK_REALIGN_DEFAULT was changed which hides the error
now:
diff --git a/gcc/config/i386/cygming.h b/gcc/config/i386/cygming.h
index 3ddcbecb22f..d587d25a58a 100644
--- a/gcc/config/i386/cygming.h
+++ b/gcc/config/i386/cygming.h
@@ -36,7 +36,7 @@ along with GCC; see the file COPYING3. If not see
/* 32-bit Windows aligns the stack on a 4-byte boundary but SSE instructions
may require 16-byte alignment. */
#undef STACK_REALIGN_DEFAULT
-#define STACK_REALIGN_DEFAULT TARGET_SSE
+#define STACK_REALIGN_DEFAULT (TARGET_64BIT ? 0 : 1)
/* Support hooks for SEH. */
#undef TARGET_ASM_UNWIND_EMIT
This is why you do not see in a normal compile. But you will get the error back
even on upstream if you compile with -mstackrealign option enabled.
I will submit the patch as requested to the mentioned list.
