Patchwork make attribute((returns_twice)) actually work (PR tree-optimization/49243)

login
register
mail settings
Submitter Mikael Pettersson
Date June 2, 2011, 12:02 p.m.
Message ID <19943.31710.10474.411273@pilspetsen.it.uu.se>
Download mbox | patch
Permalink /patch/98372/
State New
Headers show

Comments

Mikael Pettersson - June 2, 2011, 12:02 p.m.
GCC has attribute((returns_twice)) which is supposed to allow the safe
use of alternate implementations of setjmp-like functions.  In particular,
a function that calls a setjmp-like function must itself not be inlined,
because that would enable unsafe optimizations.  This works for calls to
"setjmp" (a few alternate spellings are allowed), but not to e.g. "my_setjmp"
even if that function is declared with attribute((returns_twice)).  This
bug affects the entire gcc-4.x series, gcc-3.x worked; see PR49243.

A function that calls "setjmp" is marked non-inlinable because setjmp_call_p
is applied to the function position, and it deduces via special_function_p
that the callee is ECF_RETURNS_TWICE.  But special_function_p only looks at
the name, so setjmp_call_p fails to detect attribute((returns_twice)) callees.

The fix is to have setjmp_call_p also check if the returns_twice attribute
is present, via DECL_IS_RETURNS_TWICE.  It could call flags_from_decl_or_type
instead, but that would perform quite a bit of redundant work for this case.

The test case uses -Winline to check that gcc refuses to inline a function
that calls a returns_twice callee.  This is sufficient to verify the fix, and
avoids the machine-specific code needed in the original runtime test case.

Tested w/o regressions with gcc trunk and 4.6 on x86_64-linux.  The added test
case does fail without the fix and pass with it.

OK for trunk, and perhaps 4.6?

(I don't have svn write access.)

/Mikael

gcc/

2011-06-02  Mikael Pettersson  <mikpe@it.uu.se>

	PR tree-optimization/49243
	* calls.c (setjmp_call_p): Also check if fndecl has the
	returns_twice attribute.

gcc/testsuite/

2011-06-02  Mikael Pettersson  <mikpe@it.uu.se>

	PR tree-optimization/49243
	* gcc.dg/pr49243.c: New.
Richard Guenther - June 6, 2011, 11:15 a.m.
On Thu, Jun 2, 2011 at 2:02 PM, Mikael Pettersson <mikpe@it.uu.se> wrote:
> GCC has attribute((returns_twice)) which is supposed to allow the safe
> use of alternate implementations of setjmp-like functions.  In particular,
> a function that calls a setjmp-like function must itself not be inlined,
> because that would enable unsafe optimizations.  This works for calls to
> "setjmp" (a few alternate spellings are allowed), but not to e.g. "my_setjmp"
> even if that function is declared with attribute((returns_twice)).  This
> bug affects the entire gcc-4.x series, gcc-3.x worked; see PR49243.
>
> A function that calls "setjmp" is marked non-inlinable because setjmp_call_p
> is applied to the function position, and it deduces via special_function_p
> that the callee is ECF_RETURNS_TWICE.  But special_function_p only looks at
> the name, so setjmp_call_p fails to detect attribute((returns_twice)) callees.
>
> The fix is to have setjmp_call_p also check if the returns_twice attribute
> is present, via DECL_IS_RETURNS_TWICE.  It could call flags_from_decl_or_type
> instead, but that would perform quite a bit of redundant work for this case.
>
> The test case uses -Winline to check that gcc refuses to inline a function
> that calls a returns_twice callee.  This is sufficient to verify the fix, and
> avoids the machine-specific code needed in the original runtime test case.
>
> Tested w/o regressions with gcc trunk and 4.6 on x86_64-linux.  The added test
> case does fail without the fix and pass with it.
>
> OK for trunk, and perhaps 4.6?

Ok.  I'll take care of applying it.

Thanks,
Richard.

> (I don't have svn write access.)
>
> /Mikael
>
> gcc/
>
> 2011-06-02  Mikael Pettersson  <mikpe@it.uu.se>
>
>        PR tree-optimization/49243
>        * calls.c (setjmp_call_p): Also check if fndecl has the
>        returns_twice attribute.
>
> gcc/testsuite/
>
> 2011-06-02  Mikael Pettersson  <mikpe@it.uu.se>
>
>        PR tree-optimization/49243
>        * gcc.dg/pr49243.c: New.
>
> --- gcc-4.7-20110528/gcc/calls.c.~1~    2011-05-25 13:00:14.000000000 +0200
> +++ gcc-4.7-20110528/gcc/calls.c        2011-06-02 12:55:32.000000000 +0200
> @@ -554,6 +554,8 @@ special_function_p (const_tree fndecl, i
>  int
>  setjmp_call_p (const_tree fndecl)
>  {
> +  if (DECL_IS_RETURNS_TWICE (fndecl))
> +    return ECF_RETURNS_TWICE;
>   return special_function_p (fndecl, 0) & ECF_RETURNS_TWICE;
>  }
>
> --- gcc-4.7-20110528/gcc/testsuite/gcc.dg/pr49243.c.~1~ 1970-01-01 01:00:00.000000000 +0100
> +++ gcc-4.7-20110528/gcc/testsuite/gcc.dg/pr49243.c     2011-06-02 12:55:32.000000000 +0200
> @@ -0,0 +1,25 @@
> +/* PR tree-optimization/49243 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Winline" } */
> +
> +extern unsigned long jb[];
> +extern int my_setjmp(unsigned long jb[]) __attribute__((returns_twice));
> +extern int decode(const char*);
> +
> +static inline int wrapper(const char **s_ptr) /* { dg-warning "(inlining failed|function 'wrapper' can never be inlined because it uses setjmp)" } */
> +{
> +    if (my_setjmp(jb) == 0) {
> +       const char *s = *s_ptr;
> +       while (decode(s) != 0)
> +           *s_ptr = ++s;
> +       return 0;
> +    } else
> +       return -1;
> +}
> +
> +void parse(const char *data)
> +{
> +    const char *s = data;
> +    if (!(wrapper(&s) == -1 && (s - data) == 1)) /* { dg-warning "called from here" } */
> +       __builtin_abort();
> +}
>

Patch

--- gcc-4.7-20110528/gcc/calls.c.~1~	2011-05-25 13:00:14.000000000 +0200
+++ gcc-4.7-20110528/gcc/calls.c	2011-06-02 12:55:32.000000000 +0200
@@ -554,6 +554,8 @@  special_function_p (const_tree fndecl, i
 int
 setjmp_call_p (const_tree fndecl)
 {
+  if (DECL_IS_RETURNS_TWICE (fndecl))
+    return ECF_RETURNS_TWICE;
   return special_function_p (fndecl, 0) & ECF_RETURNS_TWICE;
 }
 
--- gcc-4.7-20110528/gcc/testsuite/gcc.dg/pr49243.c.~1~	1970-01-01 01:00:00.000000000 +0100
+++ gcc-4.7-20110528/gcc/testsuite/gcc.dg/pr49243.c	2011-06-02 12:55:32.000000000 +0200
@@ -0,0 +1,25 @@ 
+/* PR tree-optimization/49243 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Winline" } */
+
+extern unsigned long jb[];
+extern int my_setjmp(unsigned long jb[]) __attribute__((returns_twice));
+extern int decode(const char*);
+
+static inline int wrapper(const char **s_ptr) /* { dg-warning "(inlining failed|function 'wrapper' can never be inlined because it uses setjmp)" } */
+{
+    if (my_setjmp(jb) == 0) {
+	const char *s = *s_ptr;
+	while (decode(s) != 0)
+	    *s_ptr = ++s;
+	return 0;
+    } else
+	return -1;
+}
+
+void parse(const char *data)
+{
+    const char *s = data;
+    if (!(wrapper(&s) == -1 && (s - data) == 1)) /* { dg-warning "called from here" } */
+	__builtin_abort();
+}