[PATCH] New flag -Wframe-larger-than-

[PATCH] New flag -Wframe-larger-than-

[Seongbae Park (박성배, 朴成培)]

[-- Attachment #1: Type: text/plain, Size: 1282 bytes --]

Hi,

The attached patch adds a new option -Wframe-larger-than-
which can be used to help noticing functions with larger than usual frame sizes,
as well as a way to enforce certain restrictions on the individual
stack frame size
- especially when combined with other options that will warn on
variable-size frames
(e.g. -Wvla and  -Wdisallowed-function-list=alloca).
Bootstrapped and regtested on i686.
Ok for 4.4 once 4.3 is branched ?

Seongbae


gcc/ChangeLog:

2008-02-15  Seongbae Park <seongbae.park@gmail.com>

        * doc/invoke.texi (Warning Options): Add new option
        -Wframe-larger-than-.
        (-Wframe-larger-than): Document.

        * flags.h (warn_frame_larger_than, frame_larger_than_size):
        Add declarations for new option variables.

        * final.c (final_start_function): Check the frame size
        before emission and issue a Wframe-larger-than warning.

        * opts.c (warn_frame_larger_than, frame_larger_than_size):
        Add definitions for new option variables.
        (common_handle_option): Handle new option OPT_Wframe_larger_than_.

        * common.opt (Wframe-larger-than-): New option.

gcc/testsuite/ChangeLog:

2008-02-15  Seongbae Park <seongbae.park@gmail.com>

        * gcc.dg/Wframe-larger-than.c: New option test.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: frame-larger-than-0215.diff --]
[-- Type: text/x-patch; name=frame-larger-than-0215.diff, Size: 4541 bytes --]

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 132359)
+++ doc/invoke.texi	(working copy)
@@ -236,7 +236,8 @@ Objective-C and Objective-C++ Dialects}.
 -Werror  -Werror=* @gol
 -Wfatal-errors  -Wfloat-equal  -Wformat  -Wformat=2 @gol
 -Wno-format-extra-args -Wformat-nonliteral @gol
--Wformat-security  -Wformat-y2k -Wignored-qualifiers @gol
+-Wformat-security  -Wformat-y2k @gol
+-Wframe-larger-than-@var{len} -Wignored-qualifiers @gol
 -Wimplicit  -Wimplicit-function-declaration  -Wimplicit-int @gol
 -Wimport  -Wno-import  -Winit-self  -Winline @gol
 -Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
@@ -3508,6 +3509,10 @@ global variable or whenever a built-in f
 @opindex Wlarger-than-@var{len}
 Warn whenever an object of larger than @var{len} bytes is defined.
 
+@item -Wframe-larger-than-@var{len}
+@opindex Wframe-larger-than
+Warn whenever the size of a function frame is larger than @var{len} bytes.
+
 @item -Wunsafe-loop-optimizations
 @opindex Wunsafe-loop-optimizations
 @opindex Wno-unsafe-loop-optimizations
Index: flags.h
===================================================================
--- flags.h	(revision 132359)
+++ flags.h	(working copy)
@@ -137,6 +137,12 @@ extern void set_Wstrict_aliasing (int on
 extern bool warn_larger_than;
 extern HOST_WIDE_INT larger_than_size;
 
+/* Nonzero means warn about any function whose frame size is larger
+   than N bytes. */
+
+extern bool warn_frame_larger_than;
+extern HOST_WIDE_INT frame_larger_than_size;
+
 /* Temporarily suppress certain warnings.
    This is set while reading code from a system header file.  */
 
Index: final.c
===================================================================
--- final.c	(revision 132359)
+++ final.c	(working copy)
@@ -1524,6 +1524,15 @@ final_start_function (rtx first ATTRIBUT
       TREE_ASM_WRITTEN (DECL_INITIAL (current_function_decl)) = 1;
     }
 
+  if (warn_frame_larger_than
+    && get_frame_size () > frame_larger_than_size)
+  {
+      /* Issue a warning */
+      warning (OPT_Wframe_larger_than_,
+               "the frame size of %wd bytes is larger than %wd bytes",
+               get_frame_size (), frame_larger_than_size);
+  }
+
   /* First output the function prologue: code to set up the stack frame.  */
   targetm.asm_out.function_prologue (file, get_frame_size ());
 
@@ -4289,4 +4298,3 @@ struct tree_opt_pass pass_clean_state =
   0,                                    /* todo_flags_finish */
   0                                     /* letter */
 };
-
Index: testsuite/gcc.dg/Wframe-larger-than.c
===================================================================
--- testsuite/gcc.dg/Wframe-larger-than.c	(revision 0)
+++ testsuite/gcc.dg/Wframe-larger-than.c	(revision 0)
@@ -0,0 +1,13 @@
+/* Test -Wframe-larger-than for warning
+   when the frame size is bigger than specified.
+   Origin: Seongbae Park <seongbae.park@gmail.com> */
+
+/* { dg-do compile } */
+/* { dg-options "-Wframe-larger-than-2048" } */
+
+extern void func(char *);
+
+void foo (void) {
+  char array[4096];
+  func(array);
+} /* { dg-warning "the frame size of .* bytes is larger than 2048 bytes" } */
Index: opts.c
===================================================================
--- opts.c	(revision 132359)
+++ opts.c	(working copy)
@@ -58,6 +58,11 @@ bool extra_warnings;
 bool warn_larger_than;
 HOST_WIDE_INT larger_than_size;
 
+/* True to warn about any function whose frame size is larger
+ * than N bytes. */
+bool warn_frame_larger_than;
+HOST_WIDE_INT frame_larger_than_size;
+
 /* Hack for cooperation between set_Wunused and set_Wextra.  */
 static bool maybe_warn_unused_parameter;
 
@@ -1450,6 +1455,11 @@ common_handle_option (size_t scode, cons
       warn_larger_than = value != -1;
       break;
 
+    case OPT_Wframe_larger_than_:
+      frame_larger_than_size = value;
+      warn_frame_larger_than = value != -1;
+      break;
+
     case OPT_Wstrict_aliasing:
       set_Wstrict_aliasing (value);
       break;
Index: common.opt
===================================================================
--- common.opt	(revision 132359)
+++ common.opt	(working copy)
@@ -110,6 +110,10 @@ Wfatal-errors
 Common Var(flag_fatal_errors)
 Exit on the first error occurred
 
+Wframe-larger-than-
+Common RejectNegative Joined UInteger
+-Wframe-larger-than-<number> Warn if the frame size of a function is larger than <number> bytes
+
 Winline
 Common Var(warn_inline) Warning
 Warn when an inlined function cannot be inlined

Re: [PATCH] New flag -Wframe-larger-than-

[Seongbae Park (박성배, 朴成培)]

[+CC gdr, diagnostic maintainer]

On Fri, Feb 15, 2008 at 4:54 PM, Seongbae Park (박성배, 朴成培)
<seongbae.park@gmail.com> wrote:
> Hi,
>
>  The attached patch adds a new option -Wframe-larger-than-
>  which can be used to help noticing functions with larger than usual frame sizes,
>  as well as a way to enforce certain restrictions on the individual
>  stack frame size
>  - especially when combined with other options that will warn on
>  variable-size frames
>  (e.g. -Wvla and  -Wdisallowed-function-list=alloca).
>  Bootstrapped and regtested on i686.
>  Ok for 4.4 once 4.3 is branched ?
>
>  Seongbae
>
>
>  gcc/ChangeLog:
>
>  2008-02-15  Seongbae Park <seongbae.park@gmail.com>
>
>         * doc/invoke.texi (Warning Options): Add new option
>         -Wframe-larger-than-.
>         (-Wframe-larger-than): Document.
>
>         * flags.h (warn_frame_larger_than, frame_larger_than_size):
>         Add declarations for new option variables.
>
>         * final.c (final_start_function): Check the frame size
>         before emission and issue a Wframe-larger-than warning.
>
>         * opts.c (warn_frame_larger_than, frame_larger_than_size):
>         Add definitions for new option variables.
>         (common_handle_option): Handle new option OPT_Wframe_larger_than_.
>
>         * common.opt (Wframe-larger-than-): New option.
>
>  gcc/testsuite/ChangeLog:
>
>  2008-02-15  Seongbae Park <seongbae.park@gmail.com>
>
>         * gcc.dg/Wframe-larger-than.c: New option test.
>



-- 
#pragma ident "Seongbae Park, compiler, http://seongbae.blogspot.com"

Re: [PATCH] New flag -Wframe-larger-than-

[Richard Sandiford]

""Seongbae Park (박성배, 朴成培)"" <seongbae.park@gmail.com> writes:
> +@item -Wframe-larger-than-@var{len}
> +@opindex Wframe-larger-than
> +Warn whenever the size of a function frame is larger than @var{len} bytes.

I might be being paranoid, but would it be worth adding some weasel
words to say that frame sizes might go up as well as down when moving
to a newer release?  This isn't the sort of thing you could safely use
with -Werror, for example.

(Also, this patch only accounts for the target-independent parts
of the frame size.  It doesn't include register save areas, etc.)

Richard

Re: [PATCH] New flag -Wframe-larger-than-

[Manuel López-Ibáñez]

On 16/02/2008, Seongbae Park (박성배, 朴成培) <seongbae.park@gmail.com> wrote:
> Hi,
>
> The attached patch adds a new option -Wframe-larger-than-

I will propose that you name it -Wframe-larger-than=, not a huge
difference but commandline options that take a parameter in other GNU
programs use this form. Another example is rsync.

Yes, I know -Wlarger-than- exists already. I will submit a patch to
add the variant -Wlarger-than= and make it the only advertised.

Cheers,

Manuel.

Re: [PATCH] New flag -Wframe-larger-than-

[Seongbae Park (박성배, 朴成培)]

On Feb 16, 2008 4:28 AM, Richard Sandiford <rsandifo@nildram.co.uk> wrote:
> ""Seongbae Park (박성배, 朴成培)"" <seongbae.park@gmail.com> writes:
> > +@item -Wframe-larger-than-@var{len}
> > +@opindex Wframe-larger-than
> > +Warn whenever the size of a function frame is larger than @var{len} bytes.
>
> I might be being paranoid, but would it be worth adding some weasel
> words to say that frame sizes might go up as well as down when moving
> to a newer release?  This isn't the sort of thing you could safely use
> with -Werror, for example.

I prefer to keep it simple, but others may disagree.
Naturally, the option doesn't and can't guarantee what will happen
in the next release - it could very well go up in the new release
and then the option will start triggering if it exceeded the threshold there.
The intent is to prevent function bodies which frame size
exceeds certain threshold - i.e. the warning is not about the source code,
but about the object code (since frame size is a characteristic
of the object code).

> (Also, this patch only accounts for the target-independent parts
> of the frame size.  It doesn't include register save areas, etc.)

True. Consider this as the first order approximation for targets with such.
I guess targets with significant register save areas may consider
implementing this flag inside the backend.

Seongbae

Re: [PATCH] New flag -Wframe-larger-than-

[Seongbae Park (박성배, 朴成培)]

I have no strong objection, but I'd prefer to keep - form
as well if we go = route,
since we've been using - form internally for a while already.

Seongbae

2008/2/16 Manuel López-Ibáñez <lopezibanez@gmail.com>:
> On 16/02/2008, Seongbae Park (박성배, 朴成培) <seongbae.park@gmail.com> wrote:
> > Hi,
> >
> > The attached patch adds a new option -Wframe-larger-than-
>
> I will propose that you name it -Wframe-larger-than=, not a huge
> difference but commandline options that take a parameter in other GNU
> programs use this form. Another example is rsync.
>
> Yes, I know -Wlarger-than- exists already. I will submit a patch to
> add the variant -Wlarger-than= and make it the only advertised.
>
> Cheers,
>
> Manuel.

Re: [PATCH] New flag -Wframe-larger-than-

[Mark Mitchell]

Seongbae Park (박성배, 朴成培) wrote:
> I have no strong objection, but I'd prefer to keep - form
> as well if we go = route,
> since we've been using - form internally for a while already.

For FSF purposes, the "=" form is definitely better, and there's no 
reason to carry the "-" form forward.  So, the internal-use argument 
carries no weight; all of us who develop internal patches take this risk.

> +Warn whenever the size of a function frame is larger than @var{len} bytes.

I think the documentation here should be expanded.  As Richard says, 
this is only measuring the target-independent parts of the frame size. 
We need to be careful that we not encourage people to use this in RTOS 
as a way of determining exactly how many bytes to put on the stack.

So, something like:

"Warn whenever a function's stack frame requires more than @var{len} 
bytes.  The computation done to determine the stack frame size is 
approximate and not conservative.  The actual requirements may be 
somewhat greater than @var{len} even if you do not get a warning.  In 
addition, any space allocated via @code{alloca}, variable-length arrays, 
or related constructs is not included by the compiler when determining 
whether or not to issue a warning."

OK for 4.4 with that change.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [PATCH] New flag -Wframe-larger-than-

[Seongbae Park (박성배, 朴成培)]

[-- Attachment #1: Type: text/plain, Size: 1613 bytes --]

On Mon, Feb 18, 2008 at 3:54 PM, Mark Mitchell <mark@codesourcery.com> wrote:
> Seongbae Park (박성배, 朴成培) wrote:
>  > I have no strong objection, but I'd prefer to keep - form
>  > as well if we go = route,
>  > since we've been using - form internally for a while already.
>
>  For FSF purposes, the "=" form is definitely better, and there's no
>  reason to carry the "-" form forward.  So, the internal-use argument
>  carries no weight; all of us who develop internal patches take this risk.

Ok. I guess I looked at the wrong precedent.
Attached is the updated patch with only = form.
I just applied it to the trunk with your proposed documentation below,
as revision 132496.

>  > +Warn whenever the size of a function frame is larger than @var{len} bytes.
>
>  I think the documentation here should be expanded.  As Richard says,
>  this is only measuring the target-independent parts of the frame size.
>  We need to be careful that we not encourage people to use this in RTOS
>  as a way of determining exactly how many bytes to put on the stack.
>
>  So, something like:
>
>  "Warn whenever a function's stack frame requires more than @var{len}
>  bytes.  The computation done to determine the stack frame size is
>  approximate and not conservative.  The actual requirements may be
>  somewhat greater than @var{len} even if you do not get a warning.  In
>  addition, any space allocated via @code{alloca}, variable-length arrays,
>  or related constructs is not included by the compiler when determining
>  whether or not to issue a warning."
>
>  OK for 4.4 with that change.

Thanks!

Seongbae

[-- Attachment #2: frame-larger-than-20080220.diff --]
[-- Type: text/plain, Size: 4815 bytes --]

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 132487)
+++ gcc/doc/invoke.texi	(working copy)
@@ -236,7 +236,8 @@ Objective-C and Objective-C++ Dialects}.
 -Werror  -Werror=* @gol
 -Wfatal-errors  -Wfloat-equal  -Wformat  -Wformat=2 @gol
 -Wno-format-extra-args -Wformat-nonliteral @gol
--Wformat-security  -Wformat-y2k -Wignored-qualifiers @gol
+-Wformat-security  -Wformat-y2k @gol
+-Wframe-larger-than=@var{len} -Wignored-qualifiers @gol
 -Wimplicit  -Wimplicit-function-declaration  -Wimplicit-int @gol
 -Wimport  -Wno-import  -Winit-self  -Winline @gol
 -Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
@@ -3518,6 +3519,10 @@ global variable or whenever a built-in f
 @opindex Wlarger-than-@var{len}
 Warn whenever an object of larger than @var{len} bytes is defined.
 
+@item -Wframe-larger-than=@var{len}
+@opindex Wframe-larger-than
+Warn whenever the size of a function frame is larger than @var{len} bytes.
+
 @item -Wunsafe-loop-optimizations
 @opindex Wunsafe-loop-optimizations
 @opindex Wno-unsafe-loop-optimizations
Index: gcc/flags.h
===================================================================
--- gcc/flags.h	(revision 132487)
+++ gcc/flags.h	(working copy)
@@ -137,6 +137,12 @@ extern void set_Wstrict_aliasing (int on
 extern bool warn_larger_than;
 extern HOST_WIDE_INT larger_than_size;
 
+/* Nonzero means warn about any function whose frame size is larger
+   than N bytes. */
+
+extern bool warn_frame_larger_than;
+extern HOST_WIDE_INT frame_larger_than_size;
+
 /* Temporarily suppress certain warnings.
    This is set while reading code from a system header file.  */
 
Index: gcc/final.c
===================================================================
--- gcc/final.c	(revision 132487)
+++ gcc/final.c	(working copy)
@@ -1524,6 +1524,15 @@ final_start_function (rtx first ATTRIBUT
       TREE_ASM_WRITTEN (DECL_INITIAL (current_function_decl)) = 1;
     }
 
+  if (warn_frame_larger_than
+    && get_frame_size () > frame_larger_than_size)
+  {
+      /* Issue a warning */
+      warning (OPT_Wframe_larger_than_,
+               "the frame size of %wd bytes is larger than %wd bytes",
+               get_frame_size (), frame_larger_than_size);
+  }
+
   /* First output the function prologue: code to set up the stack frame.  */
   targetm.asm_out.function_prologue (file, get_frame_size ());
 
Index: gcc/testsuite/gcc.dg/Wframe-larger-than.c
===================================================================
--- gcc/testsuite/gcc.dg/Wframe-larger-than.c	(revision 0)
+++ gcc/testsuite/gcc.dg/Wframe-larger-than.c	(revision 0)
@@ -0,0 +1,13 @@
+/* Test -Wframe-larger-than for warning
+   when the frame size is bigger than specified.
+   Origin: Seongbae Park <seongbae.park@gmail.com> */
+
+/* { dg-do compile } */
+/* { dg-options "-Wframe-larger-than=2048" } */
+
+extern void func(char *);
+
+void foo (void) {
+  char array[4096];
+  func(array);
+} /* { dg-warning "the frame size of .* bytes is larger than 2048 bytes" } */
Index: gcc/opts.c
===================================================================
--- gcc/opts.c	(revision 132487)
+++ gcc/opts.c	(working copy)
@@ -58,6 +58,11 @@ bool extra_warnings;
 bool warn_larger_than;
 HOST_WIDE_INT larger_than_size;
 
+/* True to warn about any function whose frame size is larger
+ * than N bytes. */
+bool warn_frame_larger_than;
+HOST_WIDE_INT frame_larger_than_size;
+
 /* Hack for cooperation between set_Wunused and set_Wextra.  */
 static bool maybe_warn_unused_parameter;
 
@@ -1498,6 +1503,11 @@ common_handle_option (size_t scode, cons
       warn_larger_than = value != -1;
       break;
 
+    case OPT_Wframe_larger_than_:
+      frame_larger_than_size = value;
+      warn_frame_larger_than = value != -1;
+      break;
+
     case OPT_Wstrict_aliasing:
       set_Wstrict_aliasing (value);
       break;
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 132487)
+++ gcc/common.opt	(working copy)
@@ -110,6 +110,17 @@ Wfatal-errors
 Common Var(flag_fatal_errors)
 Exit on the first error occurred
 
+Wframe-larger-than=
+Common RejectNegative Joined UInteger
+-Wframe-larger-than=@var{len} Warn whenever a function's stack frame requires
+more than @var{len} bytes.  The computation done to determine
+the stack frame size is approximate and not conservative.
+The actual requirements may be somewhat greater than @var{len}
+even if you do not get a warning.  In addition, any space allocated
+via @code{alloca}, variable-length arrays, or related constructs
+is not included by the compiler when determining
+whether or not to issue a warning.
+
 Winline
 Common Var(warn_inline) Warning
 Warn when an inlined function cannot be inlined