| Message ID | 19943.31710.10474.411273@pilspetsen.it.uu.se |
|---|---|
| State | New |
| Headers | show |
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(); > +} >
--- 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(); +}