[Bug c/49243] New: attribute((returns_twice)) doesn't work

[Bug c/49243] New: attribute((returns_twice)) doesn't work

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49243

           Summary: attribute((returns_twice)) doesn't work
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: mikpe@it.uu.se


According to the documentation, attribute((returns_twice)) is supposed to allow
people to call alternate implementations of setjmp-like functions.

It doesn't work.

In my test case, calling "_setjmp" (glibc's name) works and prevents inlining
and other optimizations that are invalid in the presence of setjmp and longjmp,
but calling "my_setjmp" (declared with returns_twice) doesn't prevent those
invalid optimizations.  This results in important side-effects disappearing if
my longjmp replacement is called.

The test case is a little long to inline here (114 lines) so I'll summarize.

There's a decoder that reads and processes characters, and it's called in a
loop to process strings of unknown length.

In the real application it's possible for the input to be followed by an
inaccessible page.  This is handled by a wrapper function that calls setjmp()
before calling the decoding loop, and having the read fault handler longjmp to
the wrapper, which then returns a special value.  The test case simulates the
read fault by calling longjmp() on the second input character.

In case of a read fault the toplevel needs to know the corresponding invalid
input address.  To do this it passes down the address of a local variable that
initially points to the start of the input.  The decoder loop increments this
variable after each successful decoding step.  After a read fault, the toplevel
uses the updated variable to see how much was successfully decoded.

The purpose of the intermediate wrapper function and passing down a pointer by
reference is to limit the number of auto variables that become indeterminate
after a longjmp, and to avoid having to mark auto variables as volatile, see
the longjmp spec in C99 TC2 or C1X.

The real application is sort-of embedded and uses it's own mini-libc, including
setjmp/longjmp replacements written in assembly code.  The test case simulates
that by wrapping libc's setjmp behind new names, and declares the setjmp()
replacement with __attribute__((returns_twice)) following gcc's documentation. 
The test case can also be compiled to use libc's setjmp as-is.

First try with libc's setjmp as-is:
> gcc -O2 returns-twice-bug.c; ./a.out
(No abort == success.)

Now try with the setjmp replacement:
> gcc -O2 -DMYSETJMP returns-twice-bug.c ; ./a.out
Abort

What happened is that the read fault was detected but the toplevel variable
that should have been updated to point to the invalid address wasn't.

Repeating the above two gcc calls with -S and comparing the .s files one sees
that the wrapper function with the setjmp call is eliminated and inlined into
the toplevel, and the decoding loop no longer writes to the in-memory pointer
variable.

Now try again with the setjmp replacement but declare the wrapper function with
__attribute__((noinline)):
> gcc -O2 -DMYSETJMP -DKLUDGE returns-twice-bug.c ; ./a.out
(No abort == success.)

Comparing the assembly code from the first and third steps we see that they are
now essentially the same, differing only in the third step code having the
setjmp/longjmp wrapper function definitions, the labels in subsequent code, and
the name of the setjmp function called from the wrapper.

So the important effect of calling libc's setjmp is that it prevents inlining
of the calling function, but declaring a function of a different name but with
returns_twice doesn't have that required effect.  That's the bug.

A quick check shows that glibc's setjmp() isn't declared with returns_twice,
and gcc recognizes it from its name alone.  Apparently the name-based checks
cause more magic to happen than attribute((returns_twice)).

This bug occurs on x86_64-linux and i686-linux with every gcc-4.x including
current 4.7 trunk.  With gcc-3.x it doesn't occur.  I haven't checked non-x86
targets.

[Bug c/49243] attribute((returns_twice)) doesn't work

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49243

--- Comment #1 from Mikael Pettersson <mikpe at it dot uu.se> 2011-05-31 16:22:43 UTC ---
Created attachment 24404
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24404
test case

[Bug c/49243] attribute((returns_twice)) doesn't work

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49243

--- Comment #2 from Mikael Pettersson <mikpe at it dot uu.se> 2011-05-31 22:49:36 UTC ---
Comparing tree dumps from gcc-4.7 on a reduced compile-only test case shows
that the code that calls _setjmp (a) and the code that calls my_setjmp (b)
start to differ after 019t.inline_param1:

--- a/pr49243.c.019t.inline_param1      2011-05-31 23:29:47.000000000 +0200
+++ b/pr49243.c.019t.inline_param1      2011-05-31 23:30:10.000000000 +0200
@@ -4,7 +4,7 @@

 Analyzing function body size: wrapper

-Inline summary for wrapper/0
+Inline summary for wrapper/0 inlinable
   self time:       33
   global time:     0
   self size:       16

Adding -Winline one finds that the code that calls _setjmp is marked
non-inlinable because setjmp_call_p says so in inline_forbidden_p_stmt,
however it doesn't say so for the code that calls my_setjmp.  setjmp_call_p
calls special_function_p, which only looks at the spelling of the function. 
The closely related flags_from_decl_or_type does look at both attributes and
spelling; the following crude patch to make setjmp_call_p call the latter
instead fixes the reduced test case for me:

--- 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-01 00:02:13.000000000 +0200
@@ -554,7 +554,7 @@ special_function_p (const_tree fndecl, i
 int
 setjmp_call_p (const_tree fndecl)
 {
-  return special_function_p (fndecl, 0) & ECF_RETURNS_TWICE;
+  return flags_from_decl_or_type (fndecl) & ECF_RETURNS_TWICE;
 }


(This is a tad expensive and can be optimized.)

The other calls to special_function_p () look safe though.

[Bug tree-optimization/49243] attribute((returns_twice)) doesn't work

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49243

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-06-01 10:30:42 UTC ---
Looks good.  Can you test the patch and post it to gcc-patches?

[Bug tree-optimization/49243] attribute((returns_twice)) doesn't work

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49243

--- Comment #4 from Mikael Pettersson <mikpe at it dot uu.se> 2011-06-02 15:33:40 UTC ---
Patch posted:
http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00145.html

[Bug tree-optimization/49243] attribute((returns_twice)) doesn't work

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49243

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-06-06 11:43:34 UTC ---
Author: rguenth
Date: Mon Jun  6 11:43:31 2011
New Revision: 174695

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174695
Log:
2011-06-06  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.dg/pr49243.c: New.

Added:
    trunk/gcc/testsuite/gcc.dg/pr49243.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/calls.c
    trunk/gcc/testsuite/ChangeLog

[Bug tree-optimization/49243] attribute((returns_twice)) doesn't work

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49243

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-06-06 11:46:16 UTC ---
Author: rguenth
Date: Mon Jun  6 11:46:14 2011
New Revision: 174696

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174696
Log:
2011-06-06  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.dg/pr49243.c: New.

Added:
    branches/gcc-4_6-branch/gcc/testsuite/gcc.dg/pr49243.c
Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/calls.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog

[Bug tree-optimization/49243] attribute((returns_twice)) doesn't work

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49243

Mikael Pettersson <mikpe at it dot uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
      Known to work|                            |4.6.1, 4.7.0
         Resolution|                            |FIXED
      Known to fail|4.7.0                       |

--- Comment #7 from Mikael Pettersson <mikpe at it dot uu.se> 2011-06-06 13:32:23 UTC ---
Fixed for 4.6.1/4.7.0.  The fix does work for 4.5 and 4.4 too, but given the
age and relative obscurity of the bug I'm not pushing it to older branches.

[Bug tree-optimization/49243] attribute((returns_twice)) doesn't work

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49243

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.6.1