* Reinstate generic stack checking warning with LRA
@ 2016-03-01 20:05 Eric Botcazou
2016-03-03 6:46 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2016-03-01 20:05 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]
When stack checking is entirely done by the middle-end (default if no specific
back-end support or forced by -fstack-check=generic), the checking for the
prologue is actually done in the caller with a default range and the hope is
that the callee's static frame is not too large... There are a few tricks to
make it sort of work, but it's obviously not bullet proof so a warning will be
issued in the problematic cases (lot of small variables).
But it is issued at the end of reload and LRA didn't replicate it, so it will
no longer be issued e.g. for s390, as seen under PR ada/70017. Therefore the
attached patch moves it to the end of do_reload and also streamlines it a bit.
Tested on x86_64-suse-linux, applied on the mainline as obvious.
2016-03-01 Eric Botcazou <ebotcazou@adacore.com>
PR ada/70017
* ira.c (do_reload): Issue warning for generic stack checking here...
* reload1.c (reload): ...instead of here and streamline it.
2016-03-01 Eric Botcazou <ebotcazou@adacore.com>
* gcc.dg/pr70017.c: New test.
--
Eric Botcazou
[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 2242 bytes --]
Index: ira.c
===================================================================
--- ira.c (revision 233840)
+++ ira.c (working copy)
@@ -5404,9 +5404,8 @@ do_reload (void)
{
df_set_flags (DF_NO_INSN_RESCAN);
build_insn_chain ();
-
- need_dce = reload (get_insns (), ira_conflicts_p);
+ need_dce = reload (get_insns (), ira_conflicts_p);
}
timevar_pop (TV_RELOAD);
@@ -5484,6 +5483,20 @@ do_reload (void)
inform (DECL_SOURCE_LOCATION (decl), "for %qD", decl);
}
+ /* If we are doing generic stack checking, give a warning if this
+ function's frame size is larger than we expect. */
+ if (flag_stack_check == GENERIC_STACK_CHECK)
+ {
+ HOST_WIDE_INT size = get_frame_size () + STACK_CHECK_FIXED_FRAME_SIZE;
+
+ for (int i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+ if (df_regs_ever_live_p (i) && !fixed_regs[i] && call_used_regs[i])
+ size += UNITS_PER_WORD;
+
+ if (size > STACK_CHECK_MAX_FRAME_SIZE)
+ warning (0, "frame size too large for reliable stack checking");
+ }
+
if (pic_offset_table_regno != INVALID_REGNUM)
pic_offset_table_rtx = gen_rtx_REG (Pmode, pic_offset_table_regno);
Index: reload1.c
===================================================================
--- reload1.c (revision 233840)
+++ reload1.c (working copy)
@@ -1258,28 +1258,6 @@ reload (rtx_insn *first, int global)
}
}
- /* If we are doing generic stack checking, give a warning if this
- function's frame size is larger than we expect. */
- if (flag_stack_check == GENERIC_STACK_CHECK)
- {
- HOST_WIDE_INT size = get_frame_size () + STACK_CHECK_FIXED_FRAME_SIZE;
- static int verbose_warned = 0;
-
- for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (df_regs_ever_live_p (i) && ! fixed_regs[i] && call_used_regs[i])
- size += UNITS_PER_WORD;
-
- if (size > STACK_CHECK_MAX_FRAME_SIZE)
- {
- warning (0, "frame size too large for reliable stack checking");
- if (! verbose_warned)
- {
- warning (0, "try reducing the number of local variables");
- verbose_warned = 1;
- }
- }
- }
-
free (temp_pseudo_reg_arr);
/* Indicate that we no longer have known memory locations or constants. */
[-- Attachment #3: pr70017.c --]
[-- Type: text/x-csrc, Size: 558 bytes --]
/* { dg-do compile } */
/* { dg-options "-fstack-check=generic" } */
/* Check that the expected warning is issued for large frames. */
#define ONE(s) char a##s[32];
#define TEN(s) ONE(s##0) ONE(s##1) ONE(s##2) ONE(s##3) ONE(s##4) \
ONE(s##5) ONE(s##6) ONE(s##7) ONE(s##8) ONE(s##9)
#define HUNDRED(s) TEN(s##0) TEN(s##1) TEN(s##2) TEN(s##3) TEN(s##4) \
TEN(s##5) TEN(s##6) TEN(s##7) TEN(s##8) TEN(s##9)
void foo(void)
{
HUNDRED(a)
HUNDRED(b)
} /* { dg-warning "frame size too large for reliable stack checking" } */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Reinstate generic stack checking warning with LRA
2016-03-01 20:05 Reinstate generic stack checking warning with LRA Eric Botcazou
@ 2016-03-03 6:46 ` Jakub Jelinek
2016-03-03 7:00 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2016-03-03 6:46 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Tue, Mar 01, 2016 at 09:04:54PM +0100, Eric Botcazou wrote:
> When stack checking is entirely done by the middle-end (default if no specific
> back-end support or forced by -fstack-check=generic), the checking for the
> prologue is actually done in the caller with a default range and the hope is
> that the callee's static frame is not too large... There are a few tricks to
> make it sort of work, but it's obviously not bullet proof so a warning will be
> issued in the problematic cases (lot of small variables).
>
> But it is issued at the end of reload and LRA didn't replicate it, so it will
> no longer be issued e.g. for s390, as seen under PR ada/70017. Therefore the
> attached patch moves it to the end of do_reload and also streamlines it a bit.
Note that the testcase fails with -fstack-protector-strong, I'm testing
in the rpm builds with --target_board=unix\{,-fstack-protector-strong\}
because the latter option is what we build packages with, but with
-fstack-protector-strong there is no warning from the testcase.
Is that a -fstack-check=generic bug, or is -fstack-check= being known
incompatible with -fstack-protector*, or testcase bug?
> 2016-03-01 Eric Botcazou <ebotcazou@adacore.com>
>
> * gcc.dg/pr70017.c: New test.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Reinstate generic stack checking warning with LRA
2016-03-03 6:46 ` Jakub Jelinek
@ 2016-03-03 7:00 ` Jakub Jelinek
2016-03-03 11:32 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2016-03-03 7:00 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Thu, Mar 03, 2016 at 07:46:49AM +0100, Jakub Jelinek wrote:
> On Tue, Mar 01, 2016 at 09:04:54PM +0100, Eric Botcazou wrote:
> > When stack checking is entirely done by the middle-end (default if no specific
> > back-end support or forced by -fstack-check=generic), the checking for the
> > prologue is actually done in the caller with a default range and the hope is
> > that the callee's static frame is not too large... There are a few tricks to
> > make it sort of work, but it's obviously not bullet proof so a warning will be
> > issued in the problematic cases (lot of small variables).
> >
> > But it is issued at the end of reload and LRA didn't replicate it, so it will
> > no longer be issued e.g. for s390, as seen under PR ada/70017. Therefore the
> > attached patch moves it to the end of do_reload and also streamlines it a bit.
>
> Note that the testcase fails with -fstack-protector-strong, I'm testing
> in the rpm builds with --target_board=unix\{,-fstack-protector-strong\}
> because the latter option is what we build packages with, but with
> -fstack-protector-strong there is no warning from the testcase.
> Is that a -fstack-check=generic bug, or is -fstack-check= being known
> incompatible with -fstack-protector*, or testcase bug?
None of the vars in the stack frame are actually used, so I wonder why the
expansion actually allocates anything in the stack frame for them (there is
just a CLOBBER for them).
Anyway, looking at pro_and_epilogue dumps, with additional
-fstack-protector-strong we decrease sp only by 4176, while without it by
8224 (on x86_64; the testcase fails on all targets I've tried so far
({x86_64,i686,powerpc64{,le},s390{,x},aarch64,armv7hl}-linux).
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Reinstate generic stack checking warning with LRA
2016-03-03 7:00 ` Jakub Jelinek
@ 2016-03-03 11:32 ` Eric Botcazou
2016-03-03 18:24 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2016-03-03 11:32 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
> Anyway, looking at pro_and_epilogue dumps, with additional
> -fstack-protector-strong we decrease sp only by 4176, while without it by
> 8224 (on x86_64; the testcase fails on all targets I've tried so far
> ({x86_64,i686,powerpc64{,le},s390{,x},aarch64,armv7hl}-linux).
Yeah, the threshold is around 4KB, feel free to add a few more HUNDREDs.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Reinstate generic stack checking warning with LRA
2016-03-03 11:32 ` Eric Botcazou
@ 2016-03-03 18:24 ` Jakub Jelinek
2016-03-03 20:43 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2016-03-03 18:24 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Thu, Mar 03, 2016 at 12:31:52PM +0100, Eric Botcazou wrote:
> > Anyway, looking at pro_and_epilogue dumps, with additional
> > -fstack-protector-strong we decrease sp only by 4176, while without it by
> > 8224 (on x86_64; the testcase fails on all targets I've tried so far
> > ({x86_64,i686,powerpc64{,le},s390{,x},aarch64,armv7hl}-linux).
>
> Yeah, the threshold is around 4KB, feel free to add a few more HUNDREDs.
I've mislooked, with -fstack-protector-strong it just has 48 bytes, and
adding many more HUNDREDs doesn't change anything.
This works though, ok for trunk?
2016-03-03 Jakub Jelinek <jakub@redhat.com>
PR ada/70017
* gcc.dg/pr70017.c (foo): Store 0 to first element of each array.
--- gcc/testsuite/gcc.dg/pr70017.c.jj 2016-03-02 07:39:10.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr70017.c 2016-03-03 19:22:02.098801850 +0100
@@ -13,4 +13,8 @@ void foo(void)
{
HUNDRED(a)
HUNDRED(b)
+#undef ONE
+#define ONE(s) a##s[0] = 0;
+ HUNDRED(a)
+ HUNDRED(b)
} /* { dg-warning "frame size too large for reliable stack checking" } */
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Reinstate generic stack checking warning with LRA
2016-03-03 18:24 ` Jakub Jelinek
@ 2016-03-03 20:43 ` Eric Botcazou
0 siblings, 0 replies; 6+ messages in thread
From: Eric Botcazou @ 2016-03-03 20:43 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
> This works though, ok for trunk?
>
> 2016-03-03 Jakub Jelinek <jakub@redhat.com>
>
> PR ada/70017
> * gcc.dg/pr70017.c (foo): Store 0 to first element of each array.
Sure, thanks.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-03 20:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-01 20:05 Reinstate generic stack checking warning with LRA Eric Botcazou
2016-03-03 6:46 ` Jakub Jelinek
2016-03-03 7:00 ` Jakub Jelinek
2016-03-03 11:32 ` Eric Botcazou
2016-03-03 18:24 ` Jakub Jelinek
2016-03-03 20:43 ` Eric Botcazou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).