[patch] Add new warning -Wtrampolines

[patch] Add new warning -Wtrampolines

[Magnus Granberg]

Hi

This patch add new warning -Wtrampolines for making the compile to warn when it 
generating trampoline in object that requires executable stack.
Tested on x86_64-unknown-linux-gnu and it have been in the Gentoo GCC patchset
for long time (2006).

Hardened at Gentoo.org
Magnus Granberg (Zorry) <zorry@gentoo.org>

gcc/

2010-05-06	Magnus Granberg		<zorry@gentoo.org>, Kevin F. Quinn <kevquinn@gentoo.org>

		* builtins.c (expand_builtin_init_trampoline):	if warn_trampolines make a warning.
		* common.opt:	Add -Wtrampolines.

gcc/doc

2010-05-06	Magnus Granberg		<zorry@gentoo.org>

		* invoke.texi:	Add -Wtrampolines.

gcc/testsuite/

2010-05-06	Magnus Granberg		<zorry@gentoo.org>

		* gcc.dg/Wtrampolines.c:	New.
---

--- gcc/builtins.c.zorry	2010-04-13 15:47:11.000000000 +0200
+++ gcc/builtins.c			2010-04-25 22:45:49.000000000 +0200
@@ -5150,6 +5150,10 @@
   targetm.calls.trampoline_init (m_tramp, t_func, r_chain);
 
   trampolines_created = 1;
+  
+  if (warn_trampolines)
+    warning (OPT_Wtrampolines, "generating trampoline in object (requires executable stack)");
+
   return const0_rtx;
 }
 
--- gcc/common.opt.zorry	2010-03-18 04:01:09.000000000 +0100
+++ gcc/common.opt			2010-05-06 00:44:18.000000000 +0200
@@ -192,6 +192,12 @@
 Common Var(warn_system_headers) Warning
 Do not suppress warnings from system headers
 
+
+Wtrampolines
+Common Var(warn_trampolines) Warnings
+Warn whenever a trampoline is generated
+
+
 Wtype-limits
 Common Var(warn_type_limits) Init(-1) Warning
 Warn if a comparison is always true or always false due to the limited range of the data type
--- gcc/doc/invoke.texi.zorry	2010-04-06 16:02:22.000000000 +0200
+++ gcc/doc/invoke.texi			2010-05-06 00:20:25.000000000 +0200
@@ -258,8 +258,8 @@
 -Wstrict-aliasing -Wstrict-aliasing=n @gol
 -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum -Wsync-nand @gol
--Wsystem-headers  -Wtrigraphs  -Wtype-limits  -Wundef  -Wuninitialized @gol
--Wunknown-pragmas  -Wno-pragmas @gol
+-Wsystem-headers  -Wtrampolines  -Wtrigraphs  -Wtype-limits  -Wundef @gol
+-Wuninitialized  -Wunknown-pragmas  -Wno-pragmas @gol
 -Wunsuffixed-float-constants  -Wunused  -Wunused-function @gol
 -Wunused-label  -Wunused-parameter -Wno-unused-result -Wunused-value  -Wunused-variable @gol
 -Wvariadic-macros -Wvla @gol
@@ -3603,6 +3603,12 @@
 option will @emph{not} warn about unknown pragmas in system
 headers---for that, @option{-Wunknown-pragmas} must also be used.
 
+@item -Wtrampolines
+@opindex Wtrampolines
+@opindex Wno-trampolines
+Make a warning when the compiler generating trampoline in object that
+requires executable stack.
+
 @item -Wfloat-equal
 @opindex Wfloat-equal
 @opindex Wno-float-equal
--- gcc/testsuite/gcc.dg/Wtrampolines.c.zorry	2010-05-05 12:53:11.000000000 +0200
+++ gcc/testsuite/gcc.dg/Wtrampolines.c			2010-05-06 00:26:05.000000000 +0200
@@ -0,0 +1,57 @@
+/* Origin: trampoline-1.c Waldek Hebisch <hebisch@math.uni.wroc.pl> */
+/* Ported to test -Wtrampolines Magnus Granberg <zorry@gentoo.org> */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target trampolines } */
+/* { dg-options "-O2 -Wtrampolines" } */
+
+#ifndef NO_TRAMPOLINES
+
+/* This used to fail on various versions of Solaris 2 because the
+   trampoline couldn't be made executable.  */
+
+extern void abort(void);
+extern double fabs(double);
+
+void foo (void)
+{
+  const int correct[1100] = {1, 0, -2, 0, 1, 0, 1, -1, -10, -30, -67};
+  int i;
+
+  double x1 (void) {return 1; }
+  double x2 (void) {return -1;}
+  double x3 (void) {return -1;}
+  double x4 (void) {return 1; }
+  double x5 (void) {return 0; }
+
+  typedef double pfun(void);
+
+  double a (int k, pfun x1, pfun x2, pfun x3, pfun x4, pfun x5)		/* { dg-warning "requires executable stack" } */
+  {
+    double b (void)
+    {
+      k = k - 1;
+      return a (k, b, x1, x2, x3, x4 );
+    }
+
+    if (k <= 0)
+      return x4 () + x5 ();
+    else
+      return b ();
+  }
+
+  for (i=0; i<=10; i++)
+  {
+    if (fabs(a( i, x1, x2, x3, x4, x5 ) - correct [i]) > 0.1)
+      abort();
+  }
+}
+#endif
+
+int main (void)
+{
+#ifndef NO_TRAMPOLINES
+  foo ();
+#endif
+  return 0;
+}

Re: [patch] Add new warning -Wtrampolines

[Andrew Pinski]

On Wed, May 5, 2010 at 4:46 PM, Magnus Granberg <zorry@gentoo.org> wrote:
> +    warning (OPT_Wtrampolines, "generating trampoline in object (requires executable stack)");

The statement about requiring an executable stack is not true on some
targets (powerpc64-linux-gnu and powerpc*-aix).  On those two targets,
the trampoline is just a function descriptor with the chain as one of
the fields.

Thanks,
Andrew Pinski

Re: [patch] Add new warning -Wtrampolines

[Eric Botcazou]

> This patch add new warning -Wtrampolines for making the compile to warn
> when it generating trampoline in object that requires executable stack.
> Tested on x86_64-unknown-linux-gnu and it have been in the Gentoo GCC
> patchset for long time (2006).

As Andrew pointed out, the wording of the warning isn't correct for all 
platforms (IA-64 for example) so it needs to be rephrased, e.g:

+@item -Wtrampolines
+@opindex Wtrampolines
+@opindex Wno-trampolines
+ Warn about trampolines generated for pointers to nested functions.
+ 
+ A trampoline is a small piece of data or code that is created at run
+ time on the stack when the address of a nested function is taken, and
+ is used to call the nested function indirectly.  For some targets, it
+ is made up of data only and thus requires no special treatment.  But,
+ for most targets, it is made up of code and thus requires the stack
+ to be made executable in order for the program to work properly.

It would be better to issue it from tree-nested.c:convert_tramp_reference_op 
so that it is attached to the token that causes it to be created.  For extra 
points, you could even make it reference the nested function:

p2.adb: In function 'P2':
p2.adb:17:12: warning: trampoline generated for 'P2.F'
p2.adb:26:11: warning: address taken from here

-- 
Eric Botcazou

Re: [patch] Add new warning -Wtrampolines

[Ralf Wildenhues]

* Eric Botcazou wrote on Thu, May 06, 2010 at 09:24:46AM CEST:
> > This patch add new warning -Wtrampolines for making the compile to warn
> > when it generating trampoline in object that requires executable stack.

> As Andrew pointed out, the wording of the warning isn't correct for all 
> platforms (IA-64 for example) so it needs to be rephrased, e.g:

If it needs to be rephrased anyway, why not make it -Wexecutable-stack
instead?  Isn't it that the part that is interesting here?

Thanks,
Ralf

Re: [patch] Add new warning -Wtrampolines

[Jakub Jelinek]

On Thu, May 06, 2010 at 07:11:54PM +0200, Ralf Wildenhues wrote:
> * Eric Botcazou wrote on Thu, May 06, 2010 at 09:24:46AM CEST:
> > > This patch add new warning -Wtrampolines for making the compile to warn
> > > when it generating trampoline in object that requires executable stack.
> 
> > As Andrew pointed out, the wording of the warning isn't correct for all 
> > platforms (IA-64 for example) so it needs to be rephrased, e.g:
> 
> If it needs to be rephrased anyway, why not make it -Wexecutable-stack
> instead?  Isn't it that the part that is interesting here?

Wouldn't it be better then to warn in the linker when setting PT_GNU_STACK
to PF_X?  That wouldn't cover just the case where GCC compiled objects
need trampolines, but where assembly needs them (or hasn't been mistakenly
marked as not needing them).

	Jakub

Re: [patch] Add new warning -Wtrampolines

[Eric Botcazou]

> If it needs to be rephrased anyway, why not make it -Wexecutable-stack
> instead?  Isn't it that the part that is interesting here?

For portability reasons you might want to warn for any trampolines, even if 
they don't make the stack executable on a subset of platforms.

-- 
Eric Botcazou

Re: [patch] Add new warning -Wtrampolines

[Gabriel Dos Reis]

On Thu, May 6, 2010 at 12:11 PM, Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:
> * Eric Botcazou wrote on Thu, May 06, 2010 at 09:24:46AM CEST:
>> > This patch add new warning -Wtrampolines for making the compile to warn
>> > when it generating trampoline in object that requires executable stack.
>
>> As Andrew pointed out, the wording of the warning isn't correct for all
>> platforms (IA-64 for example) so it needs to be rephrased, e.g:
>
> If it needs to be rephrased anyway, why not make it -Wexecutable-stack
> instead?  Isn't it that the part that is interesting here?

I agree with this suggestion.

Re: [patch] Add new warning -Wtrampolines

[Gabriel Dos Reis]

On Thu, May 6, 2010 at 5:32 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> If it needs to be rephrased anyway, why not make it -Wexecutable-stack
>> instead?  Isn't it that the part that is interesting here?
>
> For portability reasons you might want to warn for any trampolines, even if
> they don't make the stack executable on a subset of platforms.

Yes, but the real problem is not with trampolines, but with executable stacks.

Re: [patch] Add new warning -Wtrampolines

[Magnus Granberg]

torsdag 06 maj 2010 20.06.50 skrev  Jakub Jelinek:
> On Thu, May 06, 2010 at 07:11:54PM +0200, Ralf Wildenhues wrote:
> > * Eric Botcazou wrote on Thu, May 06, 2010 at 09:24:46AM CEST:
> > > > This patch add new warning -Wtrampolines for making the compile to
> > > > warn when it generating trampoline in object that requires executable
> > > > stack.
> > >
> > > As Andrew pointed out, the wording of the warning isn't correct for all
> > > platforms (IA-64 for example) so it needs to be rephrased, e.g:
> >
> > If it needs to be rephrased anyway, why not make it -Wexecutable-stack
> > instead?  Isn't it that the part that is interesting here?
> 
> Wouldn't it be better then to warn in the linker when setting PT_GNU_STACK
> to PF_X?  That wouldn't cover just the case where GCC compiled objects
> need trampolines, but where assembly needs them (or hasn't been mistakenly
> marked as not needing them).
> 
> 	Jakub
> 
It would be the way to go i think.

/Magnus

Re: [patch] Add new warning -Wtrampolines

[Eric Botcazou]

> Yes, but the real problem is not with trampolines, but with executable
> stacks.

Indeed, you just rephrased Ralf's message to which I had replied.

-- 
Eric Botcazou

Re: [patch] Add new warning -Wtrampolines

[Magnus Granberg]

torsdag 06 maj 2010 09.24.46 skrev  Eric Botcazou:
> It would be better to issue it from
>  tree-nested.c:convert_tramp_reference_op so that it is attached to the
>  token that causes it to be created.  For extra points, you could even make
>  it reference the nested function:
> 
> p2.adb: In function 'P2':
> p2.adb:17:12: warning: trampoline generated for 'P2.F'
> p2.adb:26:11: warning: address taken from here
> 
Have updated the patch to make two warnings one for the trampoline and the 
second one for makeing the stack executable. Did't add the reference to the 
nested function for extra points. Could not get the testcase ignore the second warning in the same line.

Hardened at gentoo.org
Magnus Granberg (Zorry) <zorry@gentoo.org>

gcc/

2010-05-06	Magnus Granberg		<zorry@gentoo.org>, Kevin F. Quinn <kevquinn@gentoo.org>

		* tree-nested:  #include flags.h		(convert_tramp_reference_op):	if warn_trampolines make a warning.
		* common.opt:	Add -Wtrampolines.
		* varasm.c:		(file_end_indicate_exec_stack): if warn_trampolines make a warning.

gcc/doc

2010-05-06	Magnus Granberg		<zorry@gentoo.org>

		* invoke.texi:	Add -Wtrampolines.

gcc/testsuite/

2010-05-06	Magnus Granberg		<zorry@gentoo.org>

		* gcc.dg/Wtrampolines.c:	New.
----
--- gcc/tree-nested.c.zorry	2009-11-25 11:55:54.000000000 +0100
+++ gcc/tree-nested.c		2010-05-29 14:49:13.000000000 +0200
@@ -36,6 +36,7 @@
 #include "langhooks.h"
 #include "pointer-set.h"
 #include "ggc.h"
+#include "flags.h"
 
 
 /* The object of this pass is to lower the representation of a set of nested
@@ -1913,6 +1914,9 @@
       x = init_tmp_var (info, x, &wi->gsi);
 
       *tp = x;
+	  if (warn_trampolines)
+        warning (OPT_Wtrampolines, "generating trampoline in object.");
+
       break;
 
     default:
--- gcc/common.opt.zorry	2010-03-18 04:01:09.000000000 +0100
+++ gcc/common.opt			2010-05-06 00:44:18.000000000 +0200
@@ -192,6 +192,10 @@
 Common Var(warn_system_headers) Warning
 Do not suppress warnings from system headers
 
+Wtrampolines
+Common Var(warn_trampolines) Warnings
+Warn whenever a trampoline is generated and warn if it requires executable stack to
+
 Wtype-limits
 Common Var(warn_type_limits) Init(-1) Warning
 Warn if a comparison is always true or always false due to the limited range of the data type
--- gcc/varasm.c.zorry	2010-03-27 12:56:30.000000000 +0100
+++ gcc/varasm.c		2010-05-29 15:06:33.000000000 +0200
@@ -6768,7 +6768,11 @@
 {
   unsigned int flags = SECTION_DEBUG;
   if (trampolines_created)
+    {
     flags |= SECTION_CODE;
+    if (warn_trampolines)
+      warning (OPT_Wtrampolines, "setting the stack as executable stack");
+	 }
 
   switch_to_section (get_section (".note.GNU-stack", flags, NULL));
 }
--- gcc/doc/invoke.texi.zorry	2010-04-06 16:02:22.000000000 +0200
+++ gcc/doc/invoke.texi			2010-05-06 00:20:25.000000000 +0200
@@ -258,8 +258,8 @@
 -Wstrict-aliasing -Wstrict-aliasing=n @gol
 -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum -Wsync-nand @gol
--Wsystem-headers  -Wtrigraphs  -Wtype-limits  -Wundef  -Wuninitialized @gol
--Wunknown-pragmas  -Wno-pragmas @gol
+-Wsystem-headers  -Wtrampolines  -Wtrigraphs  -Wtype-limits  -Wundef @gol
+-Wuninitialized  -Wunknown-pragmas  -Wno-pragmas @gol
 -Wunsuffixed-float-constants  -Wunused  -Wunused-function @gol
 -Wunused-label  -Wunused-parameter -Wno-unused-result -Wunused-value  -Wunused-variable @gol
 -Wvariadic-macros -Wvla @gol
@@ -3603,6 +3603,19 @@
 option will @emph{not} warn about unknown pragmas in system
 headers---for that, @option{-Wunknown-pragmas} must also be used.
 
+@item -Wtrampolines
+@opindex Wtrampolines
+@opindex Wno-trampolines
+ Warn about trampolines generated for pointers to nested functions.
+ Warn when the trampoline requires the stack to be made executable.
+ 
+ A trampoline is a small piece of data or code that is created at run
+ time on the stack when the address of a nested function is taken, and
+ is used to call the nested function indirectly.  For some targets, it
+ is made up of data only and thus requires no special treatment.  But,
+ for most targets, it is made up of code and thus requires the stack
+ to be made executable in order for the program to work properly.
+
 @item -Wfloat-equal
 @opindex Wfloat-equal
 @opindex Wno-float-equal
--- gcc/testsuite/gcc.dg/Wtrampolines.c.zorry	2010-05-05 12:53:11.000000000 +0200
+++ gcc/testsuite/gcc.dg/Wtrampolines.c			2010-05-06 00:26:05.000000000 +0200
@@ -0,0 +1,58 @@
+/* Origin: trampoline-1.c Waldek Hebisch <hebisch@math.uni.wroc.pl> */
+/* Ported to test -Wtrampolines Magnus Granberg <zorry@gentoo.org> */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target trampolines } */
+/* { dg-options "-O2 -Wtrampolines" } */
+/* { dg-warning "trampoline|stack" "" { target i?86-*-* x86_64-*-* } 58 } */
+
+#ifndef NO_TRAMPOLINES
+
+/* This used to fail on various versions of Solaris 2 because the
+   trampoline couldn't be made executable.  */
+
+extern void abort(void);
+extern double fabs(double);
+
+void foo (void)
+{
+  const int correct[1100] = {1, 0, -2, 0, 1, 0, 1, -1, -10, -30, -67};
+  int i;
+
+  double x1 (void) {return 1; }
+  double x2 (void) {return -1;}
+  double x3 (void) {return -1;}
+  double x4 (void) {return 1; }
+  double x5 (void) {return 0; }
+
+  typedef double pfun(void);
+
+  double a (int k, pfun x1, pfun x2, pfun x3, pfun x4, pfun x5)
+  {
+    double b (void)
+    {
+      k = k - 1;
+      return a (k, b, x1, x2, x3, x4 );
+    }
+
+    if (k <= 0)
+      return x4 () + x5 ();
+    else
+      return b ();
+  }
+
+  for (i=0; i<=10; i++)
+  {
+    if (fabs(a( i, x1, x2, x3, x4, x5 ) - correct [i]) > 0.1)
+      abort();
+  }
+}
+#endif
+
+int main (void)
+{
+#ifndef NO_TRAMPOLINES
+  foo ();
+#endif
+  return 0;
+}

Re: [patch] Add new warning -Wtrampolines

[Magnus Granberg]

[-- Attachment #1: Type: Text/Plain, Size: 1420 bytes --]

söndag 30 maj 2010 02.21.53 skrev  Magnus Granberg:
> torsdag 06 maj 2010 09.24.46 skrev  Eric Botcazou:
> > It would be better to issue it from
> > 
> >  tree-nested.c:convert_tramp_reference_op so that it is attached to the
> >  token that causes it to be created.  For extra points, you could even
> >  make
> > 
> >  it reference the nested function:
> > p2.adb: In function 'P2':
> > p2.adb:17:12: warning: trampoline generated for 'P2.F'
> > p2.adb:26:11: warning: address taken from here
> 
> Have updated the patch to make two warnings one for the trampoline and the
> second one for makeing the stack executable. Did't add the reference to the
> nested function for extra points. Could not get the testcase ignore the
> second warning in the same line.
>
Have updated the patch. Can some one review it and commit it. 
If the patch is okey. Tested on x86_64-unknown-linux-gnu

/Magnus
----
2010-06-18	Magnus Granberg		<zorry@gentoo.org>, Kevin F. Quinn <kevquinn@gentoo.org>

		* builtins.c:  (expand_builtin_init_trampoline):	if warn_trampolines make a warning.
		* common.opt:	Add -Wtrampolines.
		* varasm.c:		(file_end_indicate_exec_stack): if warn_trampolines make a warning.

gcc/doc

2010-06-18	Magnus Granberg		<zorry@gentoo.org>

		* invoke.texi:	Add -Wtrampolines.

gcc/testsuite/

2010-06-18	Magnus Granberg		<zorry@gentoo.org>

		* gcc.dg/Wtrampolines.c:	New.

[-- Attachment #2: Wtrampolines2.patch --]
[-- Type: text/x-patch, Size: 4449 bytes --]

gcc/
--- gcc/builtins.c.zorry	2010-04-13 15:47:11.000000000 +0200
+++ gcc/builtins.c			2010-06-16 12:33:54.000000000 +0200
@@ -5150,6 +5150,10 @@
   targetm.calls.trampoline_init (m_tramp, t_func, r_chain);
 
   trampolines_created = 1;
+
+  if (warn_trampolines)
+    warning (OPT_Wtrampolines, "trampoline generated for nested function %s", (IDENTIFIER_POINTER(DECL_NAME(t_func))));
+
   return const0_rtx;
 }
 
--- gcc/common.opt.zorry	2010-03-18 04:01:09.000000000 +0100
+++ gcc/common.opt			2010-05-06 00:44:18.000000000 +0200
@@ -192,6 +192,10 @@
 Common Var(warn_system_headers) Warning
 Do not suppress warnings from system headers
 
+Wtrampolines
+Common Var(warn_trampolines) Warnings
+Warn whenever a trampoline is generated and warn if it requires executable stack to
+
 Wtype-limits
 Common Var(warn_type_limits) Init(-1) Warning
 Warn if a comparison is always true or always false due to the limited range of the data type
--- gcc/varasm.c.zorry	2010-03-27 12:56:30.000000000 +0100
+++ gcc/varasm.c		2010-05-29 15:06:33.000000000 +0200
@@ -6768,7 +6768,11 @@
 {
   unsigned int flags = SECTION_DEBUG;
   if (trampolines_created)
+    {
     flags |= SECTION_CODE;
+    if (warn_trampolines)
+      warning (OPT_Wtrampolines, "setting the stack as executable stack");
+	 }
 
   switch_to_section (get_section (".note.GNU-stack", flags, NULL));
 }
--- gcc/doc/invoke.texi.zorry	2010-04-06 16:02:22.000000000 +0200
+++ gcc/doc/invoke.texi			2010-05-06 00:20:25.000000000 +0200
@@ -258,8 +258,8 @@
 -Wstrict-aliasing -Wstrict-aliasing=n @gol
 -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum -Wsync-nand @gol
--Wsystem-headers  -Wtrigraphs  -Wtype-limits  -Wundef  -Wuninitialized @gol
--Wunknown-pragmas  -Wno-pragmas @gol
+-Wsystem-headers  -Wtrampolines  -Wtrigraphs  -Wtype-limits  -Wundef @gol
+-Wuninitialized  -Wunknown-pragmas  -Wno-pragmas @gol
 -Wunsuffixed-float-constants  -Wunused  -Wunused-function @gol
 -Wunused-label  -Wunused-parameter -Wno-unused-result -Wunused-value  -Wunused-variable @gol
 -Wvariadic-macros -Wvla @gol
@@ -3603,6 +3603,19 @@
 option will @emph{not} warn about unknown pragmas in system
 headers---for that, @option{-Wunknown-pragmas} must also be used.
 
+@item -Wtrampolines
+@opindex Wtrampolines
+@opindex Wno-trampolines
+ Warn about trampolines generated for pointers to nested functions.
+ Warn when the trampoline requires the stack to be made executable.
+ 
+ A trampoline is a small piece of data or code that is created at run
+ time on the stack when the address of a nested function is taken, and
+ is used to call the nested function indirectly.  For some targets, it
+ is made up of data only and thus requires no special treatment.  But,
+ for most targets, it is made up of code and thus requires the stack
+ to be made executable in order for the program to work properly.
+
 @item -Wfloat-equal
 @opindex Wfloat-equal
 @opindex Wno-float-equal
--- gcc/testsuite/gcc.dg/Wtrampolines.c.zorry	2010-05-05 12:53:11.000000000 +0200
+++ gcc/testsuite/gcc.dg/Wtrampolines.c			2010-05-06 00:26:05.000000000 +0200
@@ -0,0 +1,59 @@
+/* Origin: trampoline-1.c Waldek Hebisch <hebisch@math.uni.wroc.pl> */
+/* Ported to test -Wtrampolines Magnus Granberg <zorry@gentoo.org> */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target trampolines } */
+/* { dg-options "-O2 -Wtrampolines" } */
+/* { dg-warning "trampoline" "" { target *-*-* } 31 } */
+/* { dg-prune-output "stack" } */
+
+#ifndef NO_TRAMPOLINES
+
+/* This used to fail on various versions of Solaris 2 because the
+   trampoline couldn't be made executable.  */
+
+extern void abort(void);
+extern double fabs(double);
+
+void foo (void)
+{
+  const int correct[1100] = {1, 0, -2, 0, 1, 0, 1, -1, -10, -30, -67};
+  int i;
+
+  double x1 (void) {return 1; }
+  double x2 (void) {return -1;}
+  double x3 (void) {return -1;}
+  double x4 (void) {return 1; }
+  double x5 (void) {return 0; }
+
+  typedef double pfun(void);
+
+  double a (int k, pfun x1, pfun x2, pfun x3, pfun x4, pfun x5)
+  {
+    double b (void)
+    {
+      k = k - 1;
+      return a (k, b, x1, x2, x3, x4 );
+    }
+
+    if (k <= 0)
+      return x4 () + x5 ();
+    else
+      return b ();
+  }
+
+  for (i=0; i<=10; i++)
+  {
+    if (fabs(a( i, x1, x2, x3, x4, x5 ) - correct [i]) > 0.1)
+      abort();
+  }
+}
+#endif
+
+int main (void)
+{
+#ifndef NO_TRAMPOLINES
+  foo ();
+#endif
+  return 0;
+}

Re: [patch] Add new warning -Wtrampolines

[Manuel López-Ibáñez]

Magnus,

Was this reviewed? If nobody offers to commit it, I will try to find
some time to do it.

Cheers,

Manuel.

On 18 June 2010 04:02, Magnus Granberg <zorry@gentoo.org> wrote:
> söndag 30 maj 2010 02.21.53 skrev  Magnus Granberg:
>> torsdag 06 maj 2010 09.24.46 skrev  Eric Botcazou:
>> > It would be better to issue it from
>> >
>> >  tree-nested.c:convert_tramp_reference_op so that it is attached to the
>> >  token that causes it to be created.  For extra points, you could even
>> >  make
>> >
>> >  it reference the nested function:
>> > p2.adb: In function 'P2':
>> > p2.adb:17:12: warning: trampoline generated for 'P2.F'
>> > p2.adb:26:11: warning: address taken from here
>>
>> Have updated the patch to make two warnings one for the trampoline and the
>> second one for makeing the stack executable. Did't add the reference to the
>> nested function for extra points. Could not get the testcase ignore the
>> second warning in the same line.
>>
> Have updated the patch. Can some one review it and commit it.
> If the patch is okey. Tested on x86_64-unknown-linux-gnu
>
> /Magnus
> ----
> 2010-06-18      Magnus Granberg         <zorry@gentoo.org>, Kevin F. Quinn <kevquinn@gentoo.org>
>
>                * builtins.c:  (expand_builtin_init_trampoline):        if warn_trampolines make a warning.
>                * common.opt:   Add -Wtrampolines.
>                * varasm.c:             (file_end_indicate_exec_stack): if warn_trampolines make a warning.
>
> gcc/doc
>
> 2010-06-18      Magnus Granberg         <zorry@gentoo.org>
>
>                * invoke.texi:  Add -Wtrampolines.
>
> gcc/testsuite/
>
> 2010-06-18      Magnus Granberg         <zorry@gentoo.org>
>
>                * gcc.dg/Wtrampolines.c:        New.
>

Re: [patch] Add new warning -Wtrampolines

[Gabriel Dos Reis]

On Thu, Jun 17, 2010 at 9:02 PM, Magnus Granberg <zorry@gentoo.org> wrote:
> söndag 30 maj 2010 02.21.53 skrev  Magnus Granberg:
>> torsdag 06 maj 2010 09.24.46 skrev  Eric Botcazou:
>> > It would be better to issue it from
>> >
>> >  tree-nested.c:convert_tramp_reference_op so that it is attached to the
>> >  token that causes it to be created.  For extra points, you could even
>> >  make
>> >
>> >  it reference the nested function:
>> > p2.adb: In function 'P2':
>> > p2.adb:17:12: warning: trampoline generated for 'P2.F'
>> > p2.adb:26:11: warning: address taken from here
>>
>> Have updated the patch to make two warnings one for the trampoline and the
>> second one for makeing the stack executable. Did't add the reference to the
>> nested function for extra points. Could not get the testcase ignore the
>> second warning in the same line.
>>
> Have updated the patch. Can some one review it and commit it.
> If the patch is okey. Tested on x86_64-unknown-linux-gnu
>
> /Magnus
> ----
> 2010-06-18      Magnus Granberg         <zorry@gentoo.org>, Kevin F. Quinn <kevquinn@gentoo.org>
>
>                * builtins.c:  (expand_builtin_init_trampoline):        if warn_trampolines make a warning.
>                * common.opt:   Add -Wtrampolines.
>                * varasm.c:             (file_end_indicate_exec_stack): if warn_trampolines make a warning.
>
> gcc/doc
>
> 2010-06-18      Magnus Granberg         <zorry@gentoo.org>
>
>                * invoke.texi:  Add -Wtrampolines.
>
> gcc/testsuite/
>
> 2010-06-18      Magnus Granberg         <zorry@gentoo.org>
>
>                * gcc.dg/Wtrampolines.c:        New.
>

Hi,

  I guess a question I have had since the beginning is:
Why are you warning all the time a trampoline is generated, and not just
for the case where the trampoline would require an executable stack?

Re: [patch] Add new warning -Wtrampolines

[Magnus Granberg]

[-- Attachment #1: Type: Text/Plain, Size: 939 bytes --]

On Monday 12 July 2010 16.22.30 you wrote:
> On 12 July 2010 13:53, Magnus Granberg <zorry@gentoo.org> wrote:
> > On Friday 02 July 2010 13.07.49 you wrote:
> >> Magnus,
> >> 
> >> Was this reviewed? If nobody offers to commit it, I will try to find
> >> some time to do it.
> >> 
> >> Cheers,
> >> 
> >> Manuel.
> > 
> > it was reviewed in irc and i need to change the location of the warning
> > msg in the testcase so i need to use warnings_at instead of warnings
> 
> Did you commit it? If you need someone to commit it, please send the
> updated patch and a changelog.
> 
> Cheers,
> 
> Manuel.
Updated patch with changelog for new review
gcc/
2010-07-13	Magnus Granberg <zorry@gentoo.org>, Kevin F. Quinn <kevquinn@gentoo.org>

		* builtins.c:  (expand_builtin_init_trampoline):	if -Wtrampolines make a warning.
		* common.opt:	Add -Wtrampolines.
		* doc/invoke.texi:	Add -Wtrampolines.
		* testsuite/gcc.dg/Wtrampolines.c:	New.
---

[-- Attachment #2: Wtrampolines.patch --]
[-- Type: text/x-patch, Size: 3911 bytes --]

--- gcc/builtins.c.zorry	2010-04-13 15:47:11.000000000 +0200
+++ gcc/builtins.c			2010-06-16 12:33:54.000000000 +0200
@@ -5150,6 +5150,11 @@
   targetm.calls.trampoline_init (m_tramp, t_func, r_chain);
 
   trampolines_created = 1;
+
+  if (warn_trampolines)
+    warning_at (DECL_SOURCE_LOCATION (t_func), OPT_Wtrampolines,
+                       "trampoline generated for nested function %qD", t_func);
+
   return const0_rtx;
 }
 
--- gcc/common.opt.zorry	2010-03-18 04:01:09.000000000 +0100
+++ gcc/common.opt			2010-05-06 00:44:18.000000000 +0200
@@ -192,6 +192,10 @@
 Common Var(warn_system_headers) Warning
 Do not suppress warnings from system headers
 
+Wtrampolines
+Common Var(warn_trampolines) Warnings
+Warn whenever a trampoline is generated
+
 Wtype-limits
 Common Var(warn_type_limits) Init(-1) Warning
 Warn if a comparison is always true or always false due to the limited range of the data type
--- gcc/doc/invoke.texi.zorry	2010-04-06 16:02:22.000000000 +0200
+++ gcc/doc/invoke.texi			2010-05-06 00:20:25.000000000 +0200
@@ -258,8 +258,8 @@
 -Wstrict-aliasing -Wstrict-aliasing=n @gol
 -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum -Wsync-nand @gol
--Wsystem-headers  -Wtrigraphs  -Wtype-limits  -Wundef  -Wuninitialized @gol
--Wunknown-pragmas  -Wno-pragmas @gol
+-Wsystem-headers  -Wtrampolines  -Wtrigraphs  -Wtype-limits  -Wundef @gol
+-Wuninitialized  -Wunknown-pragmas  -Wno-pragmas @gol
 -Wunsuffixed-float-constants  -Wunused  -Wunused-function @gol
 -Wunused-label  -Wunused-parameter -Wno-unused-result -Wunused-value  -Wunused-variable @gol
 -Wvariadic-macros -Wvla @gol
@@ -3603,6 +3603,18 @@
 option will @emph{not} warn about unknown pragmas in system
 headers---for that, @option{-Wunknown-pragmas} must also be used.
 
+@item -Wtrampolines
+@opindex Wtrampolines
+@opindex Wno-trampolines
+ Warn about trampolines generated for pointers to nested functions.
+ 
+ A trampoline is a small piece of data or code that is created at run
+ time on the stack when the address of a nested function is taken, and
+ is used to call the nested function indirectly.  For some targets, it
+ is made up of data only and thus requires no special treatment.  But,
+ for most targets, it is made up of code and thus requires the stack
+ to be made executable in order for the program to work properly.
+
 @item -Wfloat-equal
 @opindex Wfloat-equal
 @opindex Wno-float-equal
--- gcc/testsuite/gcc.dg/Wtrampolines.c.zorry	2010-05-05 12:53:11.000000000 +0200
+++ gcc/testsuite/gcc.dg/Wtrampolines.c			2010-05-06 00:26:05.000000000 +0200
@@ -0,0 +1,57 @@
+/* Origin: trampoline-1.c Waldek Hebisch <hebisch@math.uni.wroc.pl> */
+/* Ported to test -Wtrampolines Magnus Granberg <zorry@gentoo.org> */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target trampolines } */
+/* { dg-options "-O2 -Wtrampolines" } */
+
+#ifndef NO_TRAMPOLINES
+
+/* This used to fail on various versions of Solaris 2 because the
+   trampoline couldn't be made executable.  */
+
+extern void abort(void);
+extern double fabs(double);
+
+void foo (void)
+{
+  const int correct[1100] = {1, 0, -2, 0, 1, 0, 1, -1, -10, -30, -67};
+  int i;
+
+  double x1 (void) {return 1; }
+  double x2 (void) {return -1;}
+  double x3 (void) {return -1;}
+  double x4 (void) {return 1; }
+  double x5 (void) {return 0; }
+
+  typedef double pfun(void);
+
+  double a (int k, pfun x1, pfun x2, pfun x3, pfun x4, pfun x5)
+  {
+    double b (void)  /* { dg-warning "trampoline generated for nested function 'b'" } */
+    { 
+      k = k - 1;
+      return a (k, b, x1, x2, x3, x4 );
+    }
+
+    if (k <= 0)
+      return x4 () + x5 ();
+    else
+      return b ();
+  }
+
+  for (i=0; i<=10; i++)
+  {
+    if (fabs(a( i, x1, x2, x3, x4, x5 ) - correct [i]) > 0.1)
+      abort();
+  }
+}
+#endif
+
+int main (void)
+{
+#ifndef NO_TRAMPOLINES
+  foo ();
+#endif
+  return 0;
+}

Re: [patch] Add new warning -Wtrampolines

[Manuel López-Ibáñez]

I understood that it was oked in IRC subject to changes. If you just
did the changes you do not need a new review. If that is the case, I
will commit it if no one gets to it faster than me.

Thanks,

Manuel.

On 13 July 2010 02:30, Magnus Granberg <zorry@gentoo.org> wrote:
> On Monday 12 July 2010 16.22.30 you wrote:
>> On 12 July 2010 13:53, Magnus Granberg <zorry@gentoo.org> wrote:
>> > On Friday 02 July 2010 13.07.49 you wrote:
>> >> Magnus,
>> >>
>> >> Was this reviewed? If nobody offers to commit it, I will try to find
>> >> some time to do it.
>> >>
>> >> Cheers,
>> >>
>> >> Manuel.
>> >
>> > it was reviewed in irc and i need to change the location of the warning
>> > msg in the testcase so i need to use warnings_at instead of warnings
>>
>> Did you commit it? If you need someone to commit it, please send the
>> updated patch and a changelog.
>>
>> Cheers,
>>
>> Manuel.
> Updated patch with changelog for new review
> gcc/
> 2010-07-13      Magnus Granberg <zorry@gentoo.org>, Kevin F. Quinn <kevquinn@gentoo.org>
>
>                * builtins.c:  (expand_builtin_init_trampoline):        if -Wtrampolines make a warning.
>                * common.opt:   Add -Wtrampolines.
>                * doc/invoke.texi:      Add -Wtrampolines.
>                * testsuite/gcc.dg/Wtrampolines.c:      New.
> ---
>

Re: [patch] Add new warning -Wtrampolines

[Magnus Granberg]

On Tuesday 13 July 2010 02.36.27 Manuel López-Ibáñez wrote:
> I understood that it was oked in IRC subject to changes. If you just
> did the changes you do not need a new review. If that is the case, I
> will commit it if no one gets to it faster than me.
> 
> Thanks,
> 
> Manuel.
> 
[14:16:21] <iant> Zorry: pong
[14:16:53] <Zorry> iant: havetime for some more review of the wtrampolin patch have move the location now http://gcc.gnu.org/ml/gcc-patches/2010-07/msg01019.html
[14:18:20] <iant> Zorry: that is OK
So fill free to commit the patch.
/Magnus

Re: [patch] Add new warning -Wtrampolines

[Manuel López-Ibáñez]

Committed revision 162205.

Sorry for the dealy, I am on the road and it is difficult to find some
internet sometimes.

I fixed some errors in the patch, like using Warnings instead of
Warning in the opt file. Thanks for the patch, I hope to see more
patches from you guys in the future.

Cheers,

Manuel.

On 13 July 2010 14:31, Magnus Granberg <zorry@gentoo.org> wrote:
> On Tuesday 13 July 2010 02.36.27 Manuel López-Ibáñez wrote:
>> I understood that it was oked in IRC subject to changes. If you just
>> did the changes you do not need a new review. If that is the case, I
>> will commit it if no one gets to it faster than me.
>>
>> Thanks,
>>
>> Manuel.
>>
> [14:16:21] <iant> Zorry: pong
> [14:16:53] <Zorry> iant: havetime for some more review of the wtrampolin patch have move the location now http://gcc.gnu.org/ml/gcc-patches/2010-07/msg01019.html
> [14:18:20] <iant> Zorry: that is OK
> So fill free to commit the patch.
> /Magnus
>

Re: [patch] Add new warning -Wtrampolines

[Magnus Granberg]

On Thursday 15 July 2010 07.48.07 you wrote:
> Committed revision 162205.
> 
> Sorry for the dealy, I am on the road and it is difficult to find some
> internet sometimes.
> 
> I fixed some errors in the patch, like using Warnings instead of
> Warning in the opt file. Thanks for the patch, I hope to see more
> patches from you guys in the future.
> 
> Cheers,
> 
> Manuel.
> 
Thank you all for the commit and help.

/Magnus