What it does
============
A fundamental limitation of the new approach is that it requires phi
nodes for variables to perform the analysis it needs to issue the
warning for them. No phis - no warning. In particular, it doesn't deal
with register asm variables (see gcc.target/i386/attr-returns_twice-1.c
which I xfailed) because of how they are currently implemented in GCC.
Also, in theory there can be struct members that SRA doesn't scalarize,
but some other compiler might.
Another "downgrade" from the old implementation is that currently, the
new pass is placed in such a position in the pipeline that it requires
optimize > 0 in order to be run. But there's nothing fundamental about
it; that is something I just haven't experimented with.
I placed the new pass in tree-ssa-uninit.c, because I used static
functions defined there. The pass ended up using only struct pred_info
and is_pred_expr_subset_of().
The main PR tracking the problems of the old implementation is 21161.
I also went through the "See also" list of PRs for it. This new
implementation addresses them all, and the only failing test I saw for
x86_64-pc-linux-gnu is the already mentioned register asm test.
The existing RTL implementation doesn't check return values of
setjmp/vfork and thus does not distinguish the paths only realizable
upon a non-abnormal [*] return (this is what Wclobbered-pr21161-2.c is
about). The new implementation does deal with it.
How it does it
==============
Though I think, the comments for the new pass are extensive, they are
scattered throughout the code, so here's a brief overview of the patch.
The pass considers all returns-twice functions, though {,_,__}setjmp,
{,_,__}sigsetjmp, and vfork are special in that we know the meaning of
their return values and that allows to suppress false positives (see
below [fp]). I refer to the "setjmp-like (returns-twice) function call
site" as the setjmp_bb or simply "setjmp" in the code and the comments.
Without losing generality, let's do the same for the purpose of this
discussion.
The representation currently used for setjmp calls is convenient enough
for the analysis. There is one ABNORMAL_DISPATCHER block per function
and abnormal edges from it to all the setjmps in the function. (A setjmp
call is the first non-debug instruction in its block). Also, there is
an abnormal edge for every function call leading from the point right
after the call to the abnormal dispatcher.
Now let me walk you through the code in the order of execution (leading
to producing a warning).
First, the gate() method checks whether calls_setjmp is set for the
function. No need to do anything if it isn't.
Then the main driver - warn_clobbered_setjmp() - is run. It iterates
over all the setjmps. For a setjmp, all phi functions in its block are
considered. The basic idea is to warn when there's a redefinition of a
variable on the path from setjmp to a longjmp call (i.e. to the abnormal
dispatcher). That redefinition is exactly the thing we intend to warn
about: its effect might not be visible (the value might get clobbered)
after an abnormal return from setjmp - when some register values get
restored from a stash. A warning is warranted in such a situation
(given that there's also an initial definition flowing into the phi),
because the value of the variable upon an abnormal return from setjmp is
potentially ambiguous (unspecified). phi_opnd_phantom_redefinition()
discovers such redefinitions.
It does so by "recursively" (via a worklist of phis) walking the phi's
operands. So when we find an actual non-phi definition we already know
that it flows into the phi in the setjmp's basic block. We also need to
check that the redefinition is actually reachable from the setjmp and
that it is actually a *re*definition flowing into the setjmp bb via an
abnormal edge (what we actually check is that it flows into the abnormal
dispatcher).
[fp] Now back to the driver. In some cases such a redefinition may not
do any real harm: if the unspecified value is never used. We now walk
all ultimate uses of the phi in all_uses_reference_specified_values_p()
and check that all SSA values that reach a use (via phis) are not
unspecified in all_phi_args_are_specified_p(). This is where the
knowledge of the meaning of return values comes into play. In case of
the setjmp family and vfork, the return value indicates whether this is
the first (normal) or a subsequent (abnormal) return. And the value may
only be unspecified upon an abnormal return. So along with plain
reachability we check for compatibility of predicates and thus suppress
false positives on programs such as Wclobbered-pr21161-2.c.
I don't know how to check that a function is one of *setjmp or vfork
reliably, so I copied some code from special_function_p that uses string
comparison (see setjmp_or_vfork_p in the patch). One option would be to
have builtins for all kinds of setjmp and vfork and compare against
enum built_in_function values, but creating new builtins just for that
doesn't feel right.
Aside
=====
There's a rough analogy with loop analysis here. Let's consider all
loops that flow via the setjmp bb, reach the abnormal dispatcher by
arbitrary path, and go back to the setjmp bb. We are interested in
variables that are live in the dispatcher and modified in a loop (and
thus have a phi node in a setjmp bb). Such variables are candidates for
the warning, but we need to filter the set further. Namely, we want the
definition in the loop eventually used, and not under a predicate that
is executable only prior to going via the abnormal dispatcher.
Testing
=======
Bootstrapped with BOOT_CFLAGS="-O2 -Wclobbered", regtested on
x86_64-pc-linux-gnu, and successfully built emacs from git with
CFLAGS="-O -Wclobbered".
I would like to add a couple more test cases: PR 61118 (there's no
preprocessed version in the BZ), and internal_lisp_condition_case() from
emacs/src/eval.c. I don't yet have reduced versions, so let me do that
after the discussion of the main patch.
[*] About the terminology: "abnormal" is a GCC term. There are abnormal
edges and the ABNORMAL_DISPATHER internal function. Quite
intuitively, I call returns abnormal if they are reached at runtime
via the abnormal dispatcher. Linux man pages refer to them as
'"fake" returns' (in quotes). The C standard does not provide a
terminology apart from "direct invocation"/"return from a call to
the longjmp function".
gcc/
PR middle-end/21161
PR middle-end/65041
PR middle-end/48968
PR middle-end/54561
PR middle-end/61118
PR rtl-optimization/83162
* gcc/passes.def: Schedule the new pass.
* gcc/tree-pass.h (make_pass_early_warn_uninitialized): New declaration.
* gcc/tree-ssa-uninit.c: Include cfganal.h.
(struct pred_info): New constructors: from COND_STMT, trivial assignment
of all members, and the default.
(convert_control_dep_chain_into_preds): Use one of the above ctors (no
functional change).
(push_to_worklist): Ditto.
(get_pred_info_from_cmp): Ditto.
(edge_validator_fn): New typedef.
(nonzero_validator): New function.
(no_setjmp_validator): New function.
(dfs_direction): New enum.
(no_abnormal_dfs): New class.
(optional_location): New struct.
(phi_opnd_phantom_redefinition): New function.
(find_abnormal_dispatcher): New function.
(setjmp_or_vfork_p): New function.
(all_phi_args_are_specified_p): New function.
(all_uses_reference_specified_values_p): New function.
(warn_clobbered_setjmp): New function.
(pass_data_warn_clobbered): New struct variable.
(pass_warn_clobbered): New class.
(make_pass_warn_clobbered): New function.
testsuite/gcc.dg/
* Wclobbered-1.c: New test.
* Wclobbered-2.c: New test.
* Wclobbered-3.c: New test.
* Wclobbered-4.c: New test.
* Wclobbered-pr21161-1.c: New test.
* Wclobbered-pr21161-2.c: New test.
* Wclobbered-pr54561.c: New test.
* Wclobbered-pr65041.c: New test.
* Wclobbered-pr83162.c: New test.
testsuite/gcc.target/
* i386/attr-returns_twice-1.c: Match on arbitrary line and XFAIL.
---
gcc/passes.def | 1 +
gcc/testsuite/gcc.dg/Wclobbered-1.c | 26 +
gcc/testsuite/gcc.dg/Wclobbered-2.c | 42 ++
gcc/testsuite/gcc.dg/Wclobbered-3.c | 38 ++
gcc/testsuite/gcc.dg/Wclobbered-4.c | 32 +
gcc/testsuite/gcc.dg/Wclobbered-pr21161-1.c | 45 ++
gcc/testsuite/gcc.dg/Wclobbered-pr21161-2.c | 22 +
gcc/testsuite/gcc.dg/Wclobbered-pr54561.c | 68 +++
gcc/testsuite/gcc.dg/Wclobbered-pr65041.c | 46 ++
gcc/testsuite/gcc.dg/Wclobbered-pr83162.c | 36 ++
.../gcc.target/i386/attr-returns_twice-1.c | 2 +-
gcc/tree-pass.h | 1 +
gcc/tree-ssa-uninit.c | 556 +++++++++++++++++-
13 files changed, 897 insertions(+), 18 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/Wclobbered-1.c
create mode 100644 gcc/testsuite/gcc.dg/Wclobbered-2.c
create mode 100644 gcc/testsuite/gcc.dg/Wclobbered-3.c
create mode 100644 gcc/testsuite/gcc.dg/Wclobbered-4.c
create mode 100644 gcc/testsuite/gcc.dg/Wclobbered-pr21161-1.c
create mode 100644 gcc/testsuite/gcc.dg/Wclobbered-pr21161-2.c
create mode 100644 gcc/testsuite/gcc.dg/Wclobbered-pr54561.c
create mode 100644 gcc/testsuite/gcc.dg/Wclobbered-pr65041.c
create mode 100644 gcc/testsuite/gcc.dg/Wclobbered-pr83162.c
diff --git a/gcc/passes.def b/gcc/passes.def
index 1a7fd144f87..b3bcbe98807 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -339,6 +339,7 @@ along with GCC; see the file COPYING3. If not see
number of false positives from it. */
NEXT_PASS (pass_split_crit_edges);
NEXT_PASS (pass_late_warn_uninitialized);
+ NEXT_PASS (pass_warn_clobbered);
NEXT_PASS (pass_uncprop);
NEXT_PASS (pass_local_pure_const);
POP_INSERT_PASSES ()
diff --git a/gcc/testsuite/gcc.dg/Wclobbered-1.c b/gcc/testsuite/gcc.dg/Wclobbered-1.c
new file mode 100644
index 00000000000..c1961c4201e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wclobbered-1.c
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-O -Wclobbered" } */
+/* { dg-require-effective-target nonlocal_goto } */
+
+#include <setjmp.h>
+
+jmp_buf buf, buf2;
+int f;
+extern int bar ();
+extern void use (int);
+
+int foo()
+{
+ int i = bar();
+ setjmp (buf);
+ use (i);
+ if (f)
+ {
+ i = bar();
+ setjmp(buf2);
+ }
+ else
+ i = 0; /* { dg-warning "value.*.i..*clobbered" "" } */
+ use (i);
+ return bar();
+}
diff --git a/gcc/testsuite/gcc.dg/Wclobbered-2.c b/gcc/testsuite/gcc.dg/Wclobbered-2.c
new file mode 100644
index 00000000000..5a474e6838b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wclobbered-2.c
@@ -0,0 +1,42 @@
+/* { dg-do compile } */
+/* { dg-options "-O -Wclobbered" } */
+/* { dg-require-effective-target nonlocal_goto } */
+
+/* Derived from PR21161 (comment 4, mksh), used to produce false negatives. */
+
+#include <setjmp.h>
+#include <string.h>
+struct source {
+ int foo;
+ const char *str;
+} *source = NULL;
+jmp_buf jb;
+extern char *evalstr(const char *);
+extern struct source *pushs(void);
+extern void newenv(void);
+extern void quitenv(void);
+extern void bar();
+int hereinval(const char *content, int sub, char **resbuf) {
+ const char *ccp = evalstr ("sht");
+ struct source *s, *osource;
+
+ osource = source;
+ newenv();
+ if (setjmp(jb)) {
+ source = osource;
+ quitenv();
+ }
+ if (sub) {
+ s = pushs();
+ s->str = content;
+ source = s;
+ ccp = evalstr(ccp);
+ source = osource;
+ } else
+ ccp = content; /* { dg-warning "ccp.*clobbered" "" { target *-*-* } 0 } */
+ content = 0; /* { dg-warning "content.*clobbered" "" } */
+
+ *resbuf = strdup(ccp);
+ bar();
+ return (0);
+}
diff --git a/gcc/testsuite/gcc.dg/Wclobbered-3.c b/gcc/testsuite/gcc.dg/Wclobbered-3.c
new file mode 100644
index 00000000000..5922f3765a1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wclobbered-3.c
@@ -0,0 +1,38 @@
+/* { dg-do compile } */
+/* { dg-options "-O -Wclobbered" } */
+/* { dg-require-effective-target nonlocal_goto } */
+
+#include <setjmp.h>
+
+extern void bar(int);
+extern int foo();
+void f(int);
+void g(void);
+
+jmp_buf buf;
+int var;
+
+static int c = 0;
+static int accum = 0;
+
+/* { dg-bogus "ii.*clobbered" "" { target *-*-* } 0 } */
+
+void f(int ii)
+{
+ if (var) {
+ ii = var; // This change to ii is never done between sejmp and longjmp.
+ foo();
+ goto end;
+ }
+
+ if (setjmp(buf) != 0) {
+ accum = accum + ii;
+ c++;
+ if (c > 10)
+ __builtin_exit(0);
+ }
+ bar(ii);
+ g();
+ end:
+ ;
+}
diff --git a/gcc/testsuite/gcc.dg/Wclobbered-4.c b/gcc/testsuite/gcc.dg/Wclobbered-4.c
new file mode 100644
index 00000000000..5c56b74612c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wclobbered-4.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+/* { dg-options "-O -Wclobbered" } */
+/* { dg-require-effective-target nonlocal_goto } */
+
+#include <setjmp.h>
+
+extern void bar(int);
+void f(int);
+void g(void);
+
+jmp_buf buf;
+
+static int c = 0;
+static int accum = 0;
+
+void f(int i)
+{
+ if (setjmp(buf) != 0) {
+ accum = accum + i;
+ if (++c > 10)
+ __builtin_exit(0);
+ }
+ bar(i);
+ i = 2; /* { dg-warning "value.*.i..*clobbered" "" } */
+ g();
+}
+
+int main(void)
+{
+ f(99);
+ return accum;
+}
diff --git a/gcc/testsuite/gcc.dg/Wclobbered-pr21161-1.c b/gcc/testsuite/gcc.dg/Wclobbered-pr21161-1.c
new file mode 100644
index 00000000000..e0ecb6331aa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wclobbered-pr21161-1.c
@@ -0,0 +1,45 @@
+/* { dg-do compile } */
+/* { dg-options "-O -Wclobbered" } */
+/* { dg-require-effective-target nonlocal_goto } */
+
+/* From mksh (comment 4 in PR21161). */
+
+#include <setjmp.h>
+#include <string.h>
+struct source {
+ int foo;
+ const char *str;
+} *source = NULL;
+jmp_buf jb;
+extern char *evalstr(void);
+extern struct source *pushs(void);
+extern void newenv(void);
+extern void quitenv(void);
+
+/* { dg-bogus "ccp.*clobbered" "" { target *-*-* } 0 } */
+/* { dg-bogus "content.*clobbered" "" { target *-*-* } 0 } */
+
+int hereinval(const char *content, int sub, char **resbuf) {
+ const char *ccp;
+ struct source *s, *osource;
+
+ osource = source;
+ newenv();
+ if (setjmp(jb)) {
+ source = osource;
+ quitenv();
+ return (-2);
+ }
+ if (sub) {
+ s = pushs();
+ s->str = content;
+ source = s;
+ /* ... */
+ ccp = evalstr();
+ source = osource;
+ } else
+ ccp = content;
+
+ *resbuf = strdup(ccp);
+ return (0);
+}
diff --git a/gcc/testsuite/gcc.dg/Wclobbered-pr21161-2.c b/gcc/testsuite/gcc.dg/Wclobbered-pr21161-2.c
new file mode 100644
index 00000000000..1e291e4bb59
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wclobbered-pr21161-2.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-O -Wclobbered" } */
+/* { dg-require-effective-target nonlocal_goto } */
+
+/* Comment 14 in PR21161. */
+#include <setjmp.h>
+
+typedef struct { long a; } * b;
+b global_obj;
+
+void
+c (b obj) /* { dg-bogus "obj.*clobbered" "" { target *-*-* } 0 } */
+{
+ jmp_buf env;
+ if (setjmp(env) != 0)
+ // A warning here would be bogus, because the unspecified value (on the 2nd+
+ // return - any abnormal a.k.a. "fake" return) is not used; the variable is
+ // redefined before use.
+ obj = global_obj;
+ if (obj->a)
+ longjmp (env, 1);
+}
diff --git a/gcc/testsuite/gcc.dg/Wclobbered-pr54561.c b/gcc/testsuite/gcc.dg/Wclobbered-pr54561.c
new file mode 100644
index 00000000000..9749e8f05a7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wclobbered-pr54561.c
@@ -0,0 +1,68 @@
+/* { dg-do compile } */
+/* { dg-options "-fpreprocessed -O -Wclobbered" } */
+/* { dg-require-effective-target nonlocal_goto } */
+
+typedef struct FILE *FILE;
+typedef struct
+ {
+ unsigned long int __val[(1024 / (8 * sizeof (unsigned long int)))];
+ } __sigset_t;
+typedef long int __jmp_buf[8];
+struct __jmp_buf_tag
+ {
+ __jmp_buf __jmpbuf;
+ int __mask_was_saved;
+ __sigset_t __saved_mask;
+ };
+typedef struct __jmp_buf_tag jmp_buf[1];
+int setjmp (jmp_buf __env) __attribute__ ((__nothrow__));
+typedef struct png_struct_def png_struct;
+typedef struct png_info_def png_info;
+png_struct *random_png_struct_ptr (void);
+png_info *random_png_info_ptr (void);
+void png_set_random_slot (png_struct *, void *);
+void png_destroy_read_struct (png_struct **png_ptr_ptr, png_info **info_ptr_ptr, png_info **end_info_ptr_ptr) ;
+void png_read_info (png_struct *png_ptr, png_info *info_ptr);
+FILE *random_file (void);
+void fclose (FILE *);
+
+struct png_load_context
+{
+ png_struct *png_ptr;
+ png_info *info_ptr;
+ png_info *end_info;
+ FILE *fp;
+};
+
+int
+png_load_body (struct png_load_context *c)
+{
+ jmp_buf j;
+ png_info *info_ptr = 0, *end_info = 0;
+ FILE *fp = 0; /* { dg-bogus "fp.*clobbered" "" { target *-*-* } 0 } */
+ png_struct *png_ptr = random_png_struct_ptr ();
+ if (png_ptr)
+ {
+ fp = random_file ();
+ info_ptr = random_png_info_ptr (); /* { dg-bogus "info_ptr.*clobbered" "" { target *-*-* } 0 } */
+ end_info = random_png_info_ptr ();
+ }
+
+ c->png_ptr = png_ptr;
+ c->info_ptr = info_ptr;
+ c->end_info = end_info;
+ c->fp = fp;
+
+ if (setjmp (j))
+ {
+ if (c->png_ptr)
+ png_destroy_read_struct (&c->png_ptr, &c->info_ptr, &c->end_info);
+ if (c->fp)
+ fclose (c->fp);
+ return 0;
+ }
+
+ png_set_random_slot (png_ptr, fp);
+ png_read_info (png_ptr, info_ptr);
+ return 1;
+}
diff --git a/gcc/testsuite/gcc.dg/Wclobbered-pr65041.c b/gcc/testsuite/gcc.dg/Wclobbered-pr65041.c
new file mode 100644
index 00000000000..41593f1be63
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wclobbered-pr65041.c
@@ -0,0 +1,46 @@
+/* { dg-do compile } */
+/* { dg-options "-O -Wclobbered" } */
+/* { dg-require-effective-target nonlocal_goto } */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <setjmp.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+extern void somefunc(int x);
+extern void someotherfunc(int x);
+
+void
+testsetjmp(int a1)
+{
+ jmp_buf jbuf;
+ int fd;
+ int a2; /* { dg-bogus "a2.*clobbered" "" { target *-*-* } 0 } */
+
+ if (a1 > 0)
+ a2 = 42;
+ else
+ a2 = 43;
+
+ fd = open("mytmpfile", O_CREAT | O_RDWR | O_TRUNC, S_IRUSR | S_IWUSR);
+ if (fd < 0)
+ {
+ perror("mytmpfile");
+ exit(1);
+ }
+
+ if (setjmp(jbuf) == 0)
+ {
+ somefunc(a2);
+ close(fd);
+ fd = -1; /* { dg-warning "fd.*clobbered" "" } */
+ someotherfunc(a2);
+ }
+ else
+ {
+ /* we get here on longjmp out of somefunc or someotherfunc */
+ if (fd >= 0)
+ close(fd);
+ }
+}
diff --git a/gcc/testsuite/gcc.dg/Wclobbered-pr83162.c b/gcc/testsuite/gcc.dg/Wclobbered-pr83162.c
new file mode 100644
index 00000000000..9c0343da23a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wclobbered-pr83162.c
@@ -0,0 +1,36 @@
+/* { dg-do compile } */
+/* { dg-options "-fpreprocessed -O -Wclobbered" } */
+/* { dg-require-effective-target nonlocal_goto } */
+
+typedef struct
+{
+ unsigned long val[(1024 / (8 * sizeof (unsigned long)))];
+} sigset_t;
+typedef long __jmp_buf[8];
+struct __jmp_buf_tag
+ {
+ __jmp_buf __jmpbuf;
+ int __mask_was_saved;
+ sigset_t __saved_mask;
+ };
+typedef struct __jmp_buf_tag jmp_buf[1];
+int _setjmp (struct __jmp_buf_tag __env[1]) __attribute__ ((__nothrow__));
+
+typedef struct Lisp_Vector *Lisp_Object;
+struct Lisp_Vector { Lisp_Object contents[10]; };
+_Bool module_assertions;
+
+jmp_buf jb;
+
+void
+module_vec_set (Lisp_Object vec, long i, Lisp_Object val)
+{
+ if (_setjmp (jb))
+ return;
+
+ /* { dg-bogus "vec.*clobbered" "" { target *-*-* } 0 } */
+ /* { dg-bogus "val.*clobbered" "" { target *-*-* } 0 } */
+
+ ((Lisp_Object) __builtin_assume_aligned (module_assertions ? 0 : vec, 8))
+ ->contents[i] = module_assertions ? 0 : val;
+}
diff --git a/gcc/testsuite/gcc.target/i386/attr-returns_twice-1.c b/gcc/testsuite/gcc.target/i386/attr-returns_twice-1.c
index 17499adacd9..c8d16f0cfe9 100644
--- a/gcc/testsuite/gcc.target/i386/attr-returns_twice-1.c
+++ b/gcc/testsuite/gcc.target/i386/attr-returns_twice-1.c
@@ -7,7 +7,7 @@ void g(int);
int
main (void)
{
- register int reg asm ("esi") = 1; /* { dg-warning "might be clobbered" } */
+ register int reg asm ("esi") = 1; /* { dg-warning "might be clobbered" "" { xfail *-*-* } 0 } */
if (!newsetjmp ())
{
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index 1c8df3d0a71..ec87d694ef7 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -425,6 +425,7 @@ extern gimple_opt_pass *make_pass_post_ipa_warn (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_stdarg (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_early_warn_uninitialized (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_late_warn_uninitialized (gcc::context *ctxt);
+extern gimple_opt_pass *make_pass_warn_clobbered (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_cse_reciprocals (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_cse_sincos (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_optimize_bswap (gcc::context *ctxt);
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index 4d6f3773a87..8b7a37769e8 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see
#include "params.h"
#include "tree-cfg.h"
#include "cfghooks.h"
+#include "cfganal.h"
/* This implements the pass that does predicate aware warning on uses of
possibly uninitialized variables. The pass first collects the set of
@@ -612,6 +613,19 @@ compute_control_dep_chain (basic_block bb, basic_block dep_bb,
struct pred_info
{
+ /* Construct a predicate from COND_STMT followed by edge E. */
+ pred_info (const gimple *cond_stmt, const edge e)
+ {
+ pred_lhs = gimple_cond_lhs (cond_stmt);
+ pred_rhs = gimple_cond_rhs (cond_stmt);
+ cond_code = gimple_cond_code (cond_stmt);
+ invert = !!(e->flags & EDGE_FALSE_VALUE);
+ }
+ pred_info (tree lhs_, tree rhs_, enum tree_code cond_code_,
+ bool invert_ = false)
+ : pred_lhs (lhs_), pred_rhs (rhs_), cond_code (cond_code_), invert (invert_)
+ {}
+ pred_info () {};
tree pred_lhs;
tree pred_rhs;
enum tree_code cond_code;
@@ -704,10 +718,7 @@ convert_control_dep_chain_into_preds (vec<edge> *dep_chains,
}
if (gimple_code (cond_stmt) == GIMPLE_COND)
{
- one_pred.pred_lhs = gimple_cond_lhs (cond_stmt);
- one_pred.pred_rhs = gimple_cond_rhs (cond_stmt);
- one_pred.cond_code = gimple_cond_code (cond_stmt);
- one_pred.invert = !!(e->flags & EDGE_FALSE_VALUE);
+ one_pred = pred_info (cond_stmt, e);
t_chain.safe_push (one_pred);
has_valid_pred = true;
}
@@ -1988,13 +1999,7 @@ push_to_worklist (tree op, vec<pred_info, va_heap, vl_ptr> *work_list,
if (mark_set->contains (op))
return;
mark_set->add (op);
-
- pred_info arg_pred;
- arg_pred.pred_lhs = op;
- arg_pred.pred_rhs = integer_zero_node;
- arg_pred.cond_code = NE_EXPR;
- arg_pred.invert = false;
- work_list->safe_push (arg_pred);
+ work_list->safe_push (pred_info (op, integer_zero_node, NE_EXPR));
}
/* A helper that generates a pred_info from a gimple assignment
@@ -2003,12 +2008,9 @@ push_to_worklist (tree op, vec<pred_info, va_heap, vl_ptr> *work_list,
static pred_info
get_pred_info_from_cmp (gimple *cmp_assign)
{
- pred_info n_pred;
- n_pred.pred_lhs = gimple_assign_rhs1 (cmp_assign);
- n_pred.pred_rhs = gimple_assign_rhs2 (cmp_assign);
- n_pred.cond_code = gimple_assign_rhs_code (cmp_assign);
- n_pred.invert = false;
- return n_pred;
+ return pred_info (gimple_assign_rhs1 (cmp_assign),
+ gimple_assign_rhs2 (cmp_assign),
+ gimple_assign_rhs_code (cmp_assign));
}
/* Returns true if the PHI is a degenerated phi with
@@ -2619,6 +2621,483 @@ warn_uninitialized_phi (gphi *phi, vec<gphi *> *worklist,
uninit_use_stmt, loc);
}
+
+/* Type for callback functions for use with no_abnormal_dfs. EDGE is the edge
+ whose walkability is queried. Any auxiliary information the oracle might
+ need is passed in DATA. */
+
+typedef bool (*edge_validator_fn) (edge edge, void *data);
+
+/* This validator only accepts edges compatible with VAL != 0 predicate at
+ conditional branch points. */
+
+static bool
+nonzero_validator (edge e, void *data)
+{
+ gimple_stmt_iterator gsi;
+ gimple *cond_stmt;
+ tree val = (tree) data;
+ gsi = gsi_last_nondebug_bb (e->src);
+
+ if (gsi_end_p (gsi))
+ return true;
+
+ cond_stmt = gsi_stmt (gsi);
+
+ if (gimple_code (cond_stmt) == GIMPLE_COND)
+ {
+ /* The validator only admits edges compatible with val != 0, but the
+ predicate we create for this purpose is the inverse. */
+ pred_info val_zero_pred (val, integer_zero_node, EQ_EXPR);
+ pred_info edge_pred (cond_stmt, e);
+ return !is_pred_expr_subset_of (val_zero_pred, edge_pred);
+ }
+ if (gswitch *gs = dyn_cast <gswitch *> (cond_stmt))
+ {
+ if (operand_equal_p (val, gimple_switch_index (gs), 0))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "[nonzero_validator]: gswitch not handled yet\n");
+ }
+ /* This conservative handling may result in false positives. */
+ return true;
+ }
+ return true;
+}
+
+/* On the highest level, we want to know whether an abnormal return could happen
+ after a variable's redefinition. A straightforward way to check this is to
+ see whether a path exists from the redefinition to the setjmp call in
+ question via the abnormal dispatcher.
+
+ However, currently there is an edge in the IR from a setjmp calling block to
+ the abnormal dispatcher (same goes for a block calling any other function;
+ the knowledge that setjmp does not call longjmp is not used). This validator
+ takes care of skipping such edges, because such a path (redef ... -> setjmp
+ -> abnormal dispatcher -> setjmp) does not indicate that an abnormal return
+ can happen after the redefinition. */
+
+static bool
+no_setjmp_validator (edge e, void *data)
+{
+ basic_block abnormal_dispatcher = (basic_block) data;
+ /* This is used only with DFS_BACKWARD, so check src not dest. */
+ return !find_edge (abnormal_dispatcher, e->src);
+}
+
+namespace {
+
+enum dfs_direction
+{
+ DFS_FORWARD,
+ DFS_BACKWARD
+};
+
+class no_abnormal_dfs
+{
+public:
+ no_abnormal_dfs (function *fun,
+ bool skip_abnormal_edges_p = true,
+ dfs_direction direction = DFS_FORWARD,
+ edge_validator_fn edge_validator = NULL,
+ void *validator_data = NULL) :
+ m_visited_blocks (sbitmap_alloc (last_basic_block_for_fn (fun))),
+ m_skip_abnormal_edges_p (skip_abnormal_edges_p),
+ m_direction (direction),
+ m_edge_walkable_p (edge_validator),
+ m_validator_data (validator_data)
+ {
+ bitmap_clear (m_visited_blocks);
+ }
+ ~no_abnormal_dfs ()
+ {
+ sbitmap_free (m_visited_blocks);
+ }
+ bool visited_p (basic_block bb) const
+ {
+ return bitmap_bit_p (m_visited_blocks, bb->index);
+ }
+ void dfs (basic_block bb);
+
+private:
+ sbitmap m_visited_blocks;
+ bool m_skip_abnormal_edges_p;
+ dfs_direction m_direction;
+ edge_validator_fn m_edge_walkable_p;
+ void *m_validator_data;
+};
+
+void
+no_abnormal_dfs::dfs (basic_block bb)
+{
+ edge_iterator ei;
+ edge e;
+ basic_block adjacent;
+ vec<edge, va_gc> *edges;
+ bitmap_set_bit (m_visited_blocks, bb->index);
+ edges = m_direction == DFS_FORWARD ? bb->succs : bb->preds;
+ FOR_EACH_EDGE (e, ei, edges)
+ {
+ if (m_skip_abnormal_edges_p && e->flags & EDGE_ABNORMAL)
+ continue;
+ if (m_edge_walkable_p && !m_edge_walkable_p (e, m_validator_data))
+ continue;
+ adjacent = m_direction == DFS_FORWARD ? e->dest : e->src;
+ if (!visited_p (adjacent))
+ dfs (adjacent);
+ }
+}
+
+} // anon namespace for no_abnormal_dfs
+
+/* Poor man's std::optional<...>. */
+
+struct optional_location
+{
+ optional_location (location_t value_, basic_block bb_)
+ : value (value_), bb (bb_)
+ {}
+ optional_location ()
+ : value (UNKNOWN_LOCATION), bb (NULL)
+ {}
+ bool has_value ()
+ {
+ return bb != NULL;
+ };
+ location_t value;
+ basic_block bb;
+};
+
+/* Search for location of a variable redefinition on a non-abnormal path from
+ the setjmp-like call (the call is in PHI0->bb). The redefinition should also
+ ultimately flow into PHI0 abnormally; that would make the variable's value
+ after return from the setjmp-like call potentially ambiguous and therefore
+ unspecified.
+
+ DFS is for checking whether a bb is reachable from setjmp (PHI0->bb) via
+ non-abnormal edges. REACH_AB_DISP is for checking whether the abnormal
+ dispatcher is reachable from a bb. */
+
+static optional_location
+phi_opnd_phantom_redefinition (const no_abnormal_dfs &dfs,
+ const no_abnormal_dfs &reach_ab_disp,
+ gphi *phi0)
+{
+ vec<gphi *> phis = vNULL;
+ hash_set<tree> seen;
+ gphi *phi;
+ size_t n, i;
+ phis.safe_push (phi0);
+ while (phis.length () != 0)
+ {
+ phi = phis.pop ();
+ n = gimple_phi_num_args (phi);
+ for (i = 0; i < n; ++i)
+ {
+ tree op = gimple_phi_arg_def (phi, i);
+ if (seen.contains (op))
+ continue;
+ seen.add (op);
+
+ basic_block pred = gimple_phi_arg_edge (phi, i)->src;
+ if (TREE_CONSTANT (op)
+ && dfs.visited_p (pred)
+ && reach_ab_disp.visited_p (pred))
+ {
+ /* We've found a block where the variable is a constant. If there
+ is a path to that block from the setjmp() call, then there are
+ 2 options.
+
+ 1. The constant is assigned to the variable somewhere on the
+ path. Then we've found the redefinition we sought.
+
+ 2. ... not on the path (setjmp() call doesn't dominate pred).
+ But in this case pred should contain a phi for that variable
+ (because there's already a phi in the setjmp() caller: phi0).
+ That contradicts it being a constant there. So this situation
+ is not possible. */
+ return optional_location (gimple_phi_arg_location (phi, i), pred);
+ }
+ if (TREE_CODE (op) != SSA_NAME)
+ continue;
+
+ gimple *op_def_stmt = SSA_NAME_DEF_STMT (op);
+ if (gphi *op_phi = dyn_cast <gphi *> (op_def_stmt))
+ phis.safe_push (op_phi);
+ else
+ {
+ basic_block redef_bb = op_def_stmt->bb;
+ /* We are looking for a redefinition. Empty defs won't do,
+ neither will param values. */
+ if (gimple_nop_p (op_def_stmt))
+ continue;
+ if (dfs.visited_p (redef_bb)
+ && reach_ab_disp.visited_p (redef_bb))
+ return optional_location (op_def_stmt->location, redef_bb);
+ }
+ }
+ }
+ return optional_location (UNKNOWN_LOCATION, NULL);
+}
+
+static basic_block
+find_abnormal_dispatcher (function *fun)
+{
+ basic_block bb;
+ FOR_EACH_BB_FN (bb, fun)
+ {
+ gimple_stmt_iterator gsi = gsi_start_nondebug_after_labels_bb (bb);
+ gimple *g = gsi_stmt (gsi);
+ if (g && gimple_call_internal_p (g, IFN_ABNORMAL_DISPATCHER))
+ return bb;
+ }
+ return NULL;
+}
+
+/* See also special_function_p (in calls.c). */
+
+static bool
+setjmp_or_vfork_p (const_tree fndecl)
+{
+ tree name_decl = DECL_NAME (fndecl);
+
+ if (fndecl && name_decl
+ && (DECL_CONTEXT (fndecl) == NULL_TREE
+ || TREE_CODE (DECL_CONTEXT (fndecl)) == TRANSLATION_UNIT_DECL)
+ && TREE_PUBLIC (fndecl))
+ {
+ const char *name = IDENTIFIER_POINTER (name_decl);
+ const char *tname = name;
+
+ /* Disregard prefix _ or __. */
+ if (name[0] == '_')
+ {
+ if (name[1] == '_')
+ tname += 2;
+ else
+ tname += 1;
+ }
+
+ if (! strcmp (tname, "setjmp")
+ || ! strcmp (tname, "sigsetjmp")
+ || ! strcmp (name, "vfork"))
+ return true;
+ }
+ return false;
+}
+
+/* Returns TRUE iff no unspecified value can flow into PHI. Or equivalently,
+ iff the SSA value of PHI0's result never reaches PHI after an abnormal
+ return. AB_RET is a precomputed traversal that can be queried for
+ reachability from PHI0 after abnormal return. */
+
+static bool
+all_phi_args_are_specified_p (gphi *phi, gphi *phi0,
+ const no_abnormal_dfs &ab_ret)
+{
+ size_t n, i;
+ edge e;
+ tree op;
+
+ if (phi == phi0)
+ return false;
+
+ n = gimple_phi_num_args (phi);
+ for (i = 0; i < n; ++i)
+ {
+ op = gimple_phi_arg_def (phi, i);
+ e = gimple_phi_arg_edge (phi, i);
+
+ if (!ab_ret.visited_p (e->src))
+ continue;
+
+ if (TREE_CODE (op) != SSA_NAME)
+ gcc_unreachable ();
+
+ gimple *op_def_stmt = SSA_NAME_DEF_STMT (op);
+ if (gphi *op_phi = dyn_cast <gphi *> (op_def_stmt))
+ {
+ if (!all_phi_args_are_specified_p (op_phi, phi0, ab_ret))
+ return false;
+ }
+ /* If it's not a phi, then it's a redefinition; it's always specified. */
+ }
+ return true;
+}
+
+/* Returns TRUE iff all ultimate uses of PHI0 that happen after an abnormal
+ return can reference only specified values. AB_RET contains info about a
+ traversal from a setjmp-like call (PHI0->bb) via edges that can be taken
+ after an abnormal return.
+
+ Only paths that are realizable after an abnormal return from the setjmp-like
+ call are considered because clobbering never happens on normal return. */
+
+static bool
+all_uses_reference_specified_values_p (gphi *phi0, const no_abnormal_dfs &ab_ret)
+{
+ tree phi_result;
+ use_operand_p use_p;
+ imm_use_iterator iter;
+ gimple *use_stmt;
+ vec<gphi *> worklist = vNULL;
+ hash_set<gphi *> added_to_worklist;
+ gphi *use_phi;
+
+ worklist.safe_push (phi0);
+ added_to_worklist.add (phi0);
+
+ while (worklist.length () != 0)
+ {
+ gphi *phi = worklist.pop ();
+ phi_result = gimple_phi_result (phi);
+ FOR_EACH_IMM_USE_FAST (use_p, iter, phi_result)
+ {
+ use_stmt = USE_STMT (use_p);
+ if (is_gimple_debug (use_stmt))
+ continue;
+
+ if (!ab_ret.visited_p (use_stmt->bb))
+ continue;
+
+ if ((use_phi = dyn_cast <gphi *> (use_stmt))
+ && !added_to_worklist.contains (use_phi))
+ {
+ worklist.safe_push (use_phi);
+ added_to_worklist.add (use_phi);
+ }
+ else
+ {
+ if (!all_phi_args_are_specified_p (phi, phi0, ab_ret))
+ return false;
+ }
+ }
+ }
+ return true;
+}
+
+/* Scan function FUN with an abnormal dispatcher block ABNORMAL_DISPATCHER for
+ automatic variables that might be changed by longjmp (e.g. when the below
+ scenario can be realized) and report all such occurrences to the user.
+
+ The scenario that warrants a warning:
+
+ 1. There is an initial value that flows into the setjmp() bb.
+
+ 2. It gets overwritten (during normal flow of execution -- but the value it
+ is overwritten with flows - eventually - into the abnomral dispatcher,
+ abnormally, meaning there's a longjmp to follow at some point).
+
+ 3. longjmp transfers the control back to the setjmp() bb (abnormally).
+
+ 4. The value (which can supposedly be on a register) potentially gets
+ restored from the stash made at setjmp-like call losing the changes made
+ at step 2, rendering them "phantom" and the variable's value unspecified.
+
+ 5. The unspecified value is used after an abnormal return. (Unlike in step
+ 2, here we disregard execution paths realizable only after the initial
+ (normal) return from setjmp because it never clobbers anything.) */
+
+static void
+warn_clobbered_setjmp (function *fun, basic_block abnormal_dispatcher)
+{
+ edge_iterator ei;
+ edge e;
+ basic_block setjmp_bb;
+ gimple_stmt_iterator setjmp_call_gsi;
+ gimple *setjmp_call;
+ tree setjmp_decl;
+ tree retval;
+
+ if (dump_file)
+ fprintf (dump_file, "The abnormal dispatcher is bb %d.\n"
+ "Traversing the graph via reverse edges. (Abnormal edges are "
+ "walked, but blocks containing setjmp-like functions are not.)\n",
+ abnormal_dispatcher->index);
+ /* Compute bbs reachable from abnormal dispatcher via reverse edges. */
+ no_abnormal_dfs reach_ab_disp (fun, /*skip_abnormal_edges_p=*/false,
+ DFS_BACKWARD,
+ no_setjmp_validator,
+ abnormal_dispatcher);
+ reach_ab_disp.dfs (abnormal_dispatcher);
+ if (dump_file)
+ {
+ basic_block bb;
+ fprintf (dump_file, "REACHED FROM bb %d via reverse edges:\n",
+ abnormal_dispatcher->index);
+ FOR_EACH_BB_FN (bb, fun)
+ if (reach_ab_disp.visited_p (bb))
+ fprintf (dump_file, "%d ", bb->index);
+ fprintf (dump_file, ".\n");
+ }
+
+ FOR_EACH_EDGE (e, ei, abnormal_dispatcher->succs)
+ {
+ setjmp_bb = e->dest; /* Call site of setjmp or such. */
+ setjmp_call_gsi = gsi_start_nondebug_after_labels_bb (setjmp_bb);
+ setjmp_call = gsi_stmt (setjmp_call_gsi);
+ gcc_assert (gimple_call_flags (setjmp_call) & ECF_RETURNS_TWICE);
+ setjmp_decl = gimple_call_fndecl (setjmp_call);
+ /* Disregard return values of functions other than [_][sig]setjmp and
+ vfork because we don't know their meaning. */
+ retval = NULL_TREE;
+ if (setjmp_or_vfork_p (setjmp_decl))
+ retval = gimple_call_lhs (setjmp_call);
+
+ for (gphi_iterator gpi = gsi_start_nonvirtual_phis (setjmp_bb);
+ !gsi_end_p (gpi);
+ gsi_next_nonvirtual_phi (&gpi))
+ {
+ gphi *phi = gpi.phi ();
+
+ /* From the mere phi's existence it follows that
+
+ 1. The variable is live at the setjmp-like call site (no need for a
+ phi if its result is not used after the setjmp-like call).
+
+ 2. There are at least 2 different definitions of the variable at
+ the setjmp-like call site. (Though one of them might turn out to
+ be undef...)
+
+ So these are necessary - but not sufficient - conditions for a
+ warning. Now see if there's a redefinition we need to warn
+ about. */
+
+ optional_location maybe_redef_loc;
+ {
+ no_abnormal_dfs dfs (fun);
+ dfs.dfs (setjmp_bb);
+ maybe_redef_loc
+ = phi_opnd_phantom_redefinition (dfs, reach_ab_disp, phi);
+ }
+
+ if (maybe_redef_loc.has_value ())
+ {
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ fprintf (dump_file, "Found redefinition in bb %d\n",
+ maybe_redef_loc.bb->index);
+ if (retval)
+ {
+ no_abnormal_dfs ab_ret_compat (fun, true, DFS_FORWARD,
+ nonzero_validator, retval);
+ ab_ret_compat.dfs (setjmp_bb);
+ if (all_uses_reference_specified_values_p (phi, ab_ret_compat))
+ continue;
+ }
+ if (warning_at (maybe_redef_loc.value, OPT_Wclobbered,
+ "value assigned to %qE might be clobbered when "
+ "execution resumes",
+ phi->result))
+ inform (setjmp_call->location,
+ "after call to function %qF that may return twice:",
+ setjmp_decl);
+ }
+ }
+ }
+}
+
+
static bool
gate_warn_uninitialized (void)
{
@@ -2783,3 +3262,46 @@ make_pass_early_warn_uninitialized (gcc::context *ctxt)
{
return new pass_early_warn_uninitialized (ctxt);
}
+
+
+namespace {
+
+const pass_data pass_data_warn_clobbered =
+{
+ GIMPLE_PASS, /* type */
+ "clobbered", /* name */
+ OPTGROUP_NONE, /* optinfo_flags */
+ TV_NONE, /* tv_id */
+ PROP_ssa, /* properties_required */
+ 0, /* properties_provided */
+ 0, /* properties_destroyed */
+ 0, /* todo_flags_start */
+ 0, /* todo_flags_finish */
+};
+
+class pass_warn_clobbered : public gimple_opt_pass
+{
+public:
+ pass_warn_clobbered (gcc::context *ctxt)
+ : gimple_opt_pass (pass_data_warn_clobbered, ctxt)
+ {}
+
+ /* opt_pass methods: */
+ opt_pass *clone () { return new pass_warn_clobbered (m_ctxt); }
+ virtual bool gate (function *f) { return warn_clobbered && f->calls_setjmp; }
+ virtual unsigned int execute (function *fun)
+ {
+ basic_block abnormal_dispatcher;
+ if ((abnormal_dispatcher = find_abnormal_dispatcher (fun)))
+ warn_clobbered_setjmp (fun, abnormal_dispatcher);
+ return 0;
+ }
+}; // class pass_warn_clobbered
+
+} // anon namespace
+
+gimple_opt_pass *
+make_pass_warn_clobbered (gcc::context *ctxt)
+{
+ return new pass_warn_clobbered (ctxt);
+}
--
2.22.0
