[Bug tree-optimization/56982] New: Bad optimization with setjmp()

[Bug tree-optimization/56982] New: Bad optimization with setjmp()

[jdemeyer at cage dot ugent.be]

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

             Bug #: 56982
           Summary: Bad optimization with setjmp()
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jdemeyer@cage.ugent.be
            Target: x86_64-pc-linux-gnu


Created attachment 29883
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29883
Bug program

Compile the example program with gcc -Og jmp.c -o jmp

Run the program ./jmp and the output is

Returning 1
x = 0, n = 1
Returning 0
x = 42, n = 1
Aborted

The function g() is returning 0 the second time (after longjmp()) but the
return value, assigned to n, equals 1. With other optimization levels or with
earlier versions of gcc or with -Og -fno-tree-dominator-opts, the output is
what I expect:

Returning 1
x = 0, n = 1
Returning 0
x = 42, n = 0

This is with gcc version 4.8.0, GNU libc version 2.15.

[Bug tree-optimization/56982] Bad optimization with setjmp()

[jdemeyer at cage dot ugent.be]

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

--- Comment #1 from Jeroen Demeyer <jdemeyer at cage dot ugent.be> 2013-04-16 14:31:54 UTC ---
Created attachment 29884
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29884
Bug program (preprocessed)

[Bug tree-optimization/56982] [4.8/4.9 Regression] Bad optimization with setjmp()

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-16 16:04:10 UTC ---
At RTL time (besides it being a pessimization), the thing is that _3 is
assigned a pseudo (compared to before the change, where it had only a single
use and thus has been TERed), that pseudo is initialized from *e, and
initialized to 1 in the bb where the PHI had 1, and the pseudo is spilled to
the stack.  So it will initially contain the 0 value (correct), then that is
overwritten with 1 (fine for the first setjmp returning case), but when setjmp
returns second time, the value is still 1 rather than 0.

So the questions are:
- is it desirable that uncprop does anything to SSA_NAME_VAR == NULL phis?
- shouldn't something like that be not performed if current function calls
setjmp (or more narrowly, if there is a returns twice function somewhere in
between the considered setter and user)?
- what other optimizations might be similarly problematic across returns twice
calls?

[Bug tree-optimization/56982] [4.8/4.9 Regression] Bad optimization with setjmp()

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> 2013-04-17 08:26:20 UTC ---
I will have a look.

[Bug tree-optimization/56982] [4.8/4.9 Regression] Bad optimization with setjmp()

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-17 08:56:00 UTC ---
I don't see how we could declare the testcase invalid, why would n need to be
volatile?  It isn't live across the setjmp call, it is even declared after the
setjmp call, and it is always initialized after the setjmp call.

[Bug tree-optimization/56982] [4.8/4.9 Regression] Bad optimization with setjmp()

[rguenther at suse dot de]

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

--- Comment #7 from rguenther at suse dot de <rguenther at suse dot de> 2013-04-17 09:07:10 UTC ---
On Wed, 17 Apr 2013, jakub at gcc dot gnu.org wrote:

> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56982
> 
> --- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-17 08:56:00 UTC ---
> I don't see how we could declare the testcase invalid, why would n need to be
> volatile?  It isn't live across the setjmp call, it is even declared after the
> setjmp call, and it is always initialized after the setjmp call.

Then there is no other way but to model the abnormal control flow
properly.  Even simple CSE can break things otherwise.  Consider

int tmp = a + 1;
setjmp ()
int tmp2 = a + 1;

even on RTL CSE would break that, no?  setjmp doesn't even
forcefully start a new basic-block.

Hmm, maybe doing that, start a new BB for all returns-twice
calls and add an abnormal edge from FN entry is enough to
avoid all possibly dangerous transforms.

Richard.

[Bug tree-optimization/56982] [4.8/4.9 Regression] Bad optimization with setjmp()

[rguenth at gcc dot gnu.org]

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> 2013-04-17 09:57:52 UTC ---
Created attachment 29889
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29889
patch

Untested patch.  The patch handles setjmp similar to a non-local label,
thus force it to start a new basic-block and get abnormal edges from all
sites that can make a non-local goto or call longjmp.

Fixes the testcase for me.  Somewhat reduced:

#include <stdio.h>
#include <stdlib.h>
#include <setjmp.h>

static sigjmp_buf env;

static inline int g(int x)
{
    if (x)
    {
        fprintf(stderr, "Returning 0\n");
        return 0;
    }
    else
    {
        fprintf(stderr, "Returning 1\n");
        return 1;
    }
}

int f(int *e)
{
    if (*e)
      return 1;

    int x = setjmp(env);
    int n = g(x);
    if (n == 0)
      exit(0);
    if (x)
      abort();
    longjmp(env, 42);
}

int main(int argc, char** argv)
{
    int v = 0;
    return f(&v);
}

but I cannot remove the remaining printfs, so it's not appropriate for the
testsuite yet.

[Bug tree-optimization/56982] [4.8/4.9 Regression] Bad optimization with setjmp()

[bugfeed at online dot de]

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

--- Comment #10 from Leif Leonhardy <bugfeed at online dot de> 2013-04-18 01:17:31 UTC ---
"One proposed requirement on setjmp is that it be usable like any other
function, that is, that it be callable in *any* expression context, and that
the expression evaluate correctly whether the return from setjmp is direct or
via a call to longjmp. Unfortunately, any implementation of setjmp as a
conventional called function cannot know enough about the calling environment
to save any temporary registers or dynamic stack locations used part way
through an expression evaluation. [...] The temporaries may be correct on the
initial call to setjmp, but are not likely to be on any return initiated by a
corresponding call to longjmp. These considerations dictated the constraint
that setjmp be called only from within fairly simple expressions, ones not
likely to need temporary storage.

An alternative proposal considered by the C89 Committee was to require that
implementations recognize that calling setjmp is a special case, and hence that
they take whatever precautions are necessary to restore the setjmp environment
properly upon a longjmp call. This proposal was rejected on grounds of
consistency: implementations are currently allowed to implement library
functions specially, but no other situations require special treatment."


So according to this (The C99 Rationale [1], page 139 ff., likewise the Single
UNIX Specification), here setjmp() is simply used in an invalid context (i.e.,
in an assignment statement). ;-)

Still, with -Og at least, GCC 4.8.0 produces wrong code even if setjmp() is
used in an "allowed" context (as in e.g. if (setjmp(...)>0) ..., or switch
(setjmp(...)) { ... }), and no matter whether n is declared volatile or not.


[1] http://www.open-std.org/jtc1/sc22/wg14/www/C99RationaleV5.10.pdf

[Bug tree-optimization/56982] [4.8 Regression] Bad optimization with setjmp()

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.9.0
            Summary|[4.8/4.9 Regression] Bad    |[4.8 Regression] Bad
                   |optimization with setjmp()  |optimization with setjmp()

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> 2013-04-19 13:40:05 UTC ---
Author: rguenth
Date: Fri Apr 19 13:39:16 2013
New Revision: 198096

URL: http://gcc.gnu.org/viewcvs?rev=198096&root=gcc&view=rev
Log:
2013-04-19  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/56982
    * builtins.def (BUILT_IN_LONGJMP): longjmp is not a leaf
    function.
    * gimplify.c (gimplify_call_expr): Notice special calls.
    (gimplify_modify_expr): Likewise.
    * tree-cfg.c (make_abnormal_goto_edges): Handle setjmp-like
    abnormal control flow receivers.
    (call_can_make_abnormal_goto): Handle cfun->calls_setjmp
    in the same way as cfun->has_nonlocal_labels.
    (gimple_purge_dead_abnormal_call_edges): Likewise.
    (stmt_starts_bb_p): Make setjmp-like abnormal control flow
    receivers start a basic-block.

    * gcc.c-torture/execute/pr56982.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr56982.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/builtins.def
    trunk/gcc/gimplify.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-cfg.c

[Bug tree-optimization/56982] [4.8 Regression] Bad optimization with setjmp()

[rguenth at gcc dot gnu.org]

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

Bug 56982 depends on bug 57349, which changed state.

Bug 57349 Summary: [4.9 Regression] ICE on 253.perlbmk with pgo after r198096
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57349

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

[Bug tree-optimization/56982] [4.8 Regression] Bad optimization with setjmp()

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.1                       |4.8.2

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.1 has been released.

[Bug tree-optimization/56982] [4.8 Regression] Bad optimization with setjmp()

[rguenth at gcc dot gnu.org]

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

Bug 56982 depends on bug 57584, which changed state.

Bug 57584 Summary: [4.9 Regression] ice: SSA corruption: Unable to coalesce ssa_names
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57584

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

[Bug tree-optimization/56982] [4.8 Regression] Bad optimization with setjmp()

[bernd.edlinger at hotmail dot de]

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

--- Comment #13 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Created attachment 30431
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30431&action=edit
another example of wrong compilation

This is another example where the optimization can
go wrong.

The attached program produces expected results if
compiled with -O0:
x=0, a=1
x=1, a=1
a=1

But if compiled with -O3 and if the value "a" is placed
in a register the result is like this:
x=0, a=1
x=1, a=0
a=0

That is because longjmp has more semantic than just a branch:
It branches to the setjmp, and restores all callee saved registers to
the previos value.

[Bug tree-optimization/56982] [4.8 Regression] Bad optimization with setjmp()

[mikpe at it dot uu.se]

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

--- Comment #14 from Mikael Pettersson <mikpe at it dot uu.se> ---
(In reply to Bernd Edlinger from comment #13)
> Created attachment 30431 [details]
> another example of wrong compilation
> 
> This is another example where the optimization can
> go wrong.
> 
> The attached program produces expected results if
> compiled with -O0:
> x=0, a=1
> x=1, a=1
> a=1
> 
> But if compiled with -O3 and if the value "a" is placed
> in a register the result is like this:
> x=0, a=1
> x=1, a=0
> a=0
> 
> That is because longjmp has more semantic than just a branch:
> It branches to the setjmp, and restores all callee saved registers to
> the previos value.

Your example is invalid C.  Referring to WG14 n1494.pdf (there may be more
recent C1x documents, but it's the one I had available right now):

- you violate 7.13.1.1 which specifies where setjmp() may be called, an
assignment statement is not one of the permitted contexts

- more importantly, your auto variable a is not volatile-qualified, which means
that its value is indeterminate after the longjmp (7.13.2.1).

Please fix these issues and check again if it yields wrong results.

[Bug tree-optimization/56982] [4.8 Regression] Bad optimization with setjmp()

[bernd.edlinger at hotmail dot de]

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

--- Comment #15 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to Mikael Pettersson from comment #14)
> Your example is invalid C.  Referring to WG14 n1494.pdf (there may be more
> recent C1x documents, but it's the one I had available right now):
> 
> - you violate 7.13.1.1 which specifies where setjmp() may be called, an
> assignment statement is not one of the permitted contexts
> 
> - more importantly, your auto variable a is not volatile-qualified, which
> means that its value is indeterminate after the longjmp (7.13.2.1).
> 
> Please fix these issues and check again if it yields wrong results.

Thanks for pointing that out.

When I add volatile to the auto variable, the code is OK.

[Bug tree-optimization/56982] [4.8 Regression] Bad optimization with setjmp()

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.2                       |4.8.3

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.2 has been released.

[Bug tree-optimization/56982] [4.8 Regression] Bad optimization with setjmp()

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56982

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.3                       |4.8.4

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 4.8.3 is being released, adjusting target milestone.

[Bug tree-optimization/56982] [4.8 Regression] Bad optimization with setjmp()

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56982

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|4.8.4                       |4.9.0
      Known to fail|                            |4.8.4

--- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> ---
wontfix for 4.8, the fix triggered very many latent issues.