diff mbox

[RFA,PR,16361] Add warnings for NULL pointer dereferences and such

Message ID 52E6AED6.6090704@redhat.com
State New
Headers show

Commit Message

Jeff Law Jan. 27, 2014, 7:09 p.m. UTC
[ Yes, 16351, it's that old. ]

First, as is often the case, this warning is not going to catch 
everything.  Dead code elimination, unreachable code elimination, etc 
can/will remove NULL pointer dereferences and if that happens we don't 
get a warning.  It also will not catch cases where a NULL value flows 
out of a PHI into uses in other blocks where it is then dereferenced or 
otherwise used erroneously.

-Wnull-dereference will warn for NULL pointer dereferences.  Both those 
which appear explicitly in the IL and those which occur if NULL value 
flows from a PHI to a dereference in the same block as the PHI.

-Wnull-attribute will warn if NULL is used as an argument to a function 
call where the callee's attributes have declared the argument must be 
non-NULL.  Similarly for NULL as a return value when the current 
function has an attribute declaring the function can never return NULL. 
  Similarly for NULL values flowing from a PHI into an argument/return 
in the same block as the PHI.

We utilize the analysis done for the erroneous-paths optimization.  The 
optimizations and warnings can be enabled/disabled independently.  The 
warnings are not enabled by -Wall.

The testsuite verifies basic operation by re-using the existing 
isolation tests.  Each test was split into two tests.  One which tests 
the warning (with the optimization explicitly disabled), the other which 
tests the optimization.

Earlier versions of this patch have been bootstrapped and regression 
tested.  The only difference between those earlier versions and this one 
are the docs & additions to the testsuite.

As with prior patches of mine that touched common.opt, particular 
attention to those changes would be appreciated.

OK, assuming it passes another round of bootstrapping and regression 
testing?

Jeff

Comments

Ian Lance Taylor Jan. 27, 2014, 9:02 p.m. UTC | #1
On Mon, Jan 27, 2014 at 11:09 AM, Jeff Law <law@redhat.com> wrote:
>
> First, as is often the case, this warning is not going to catch everything.
> Dead code elimination, unreachable code elimination, etc can/will remove
> NULL pointer dereferences and if that happens we don't get a warning.  It
> also will not catch cases where a NULL value flows out of a PHI into uses in
> other blocks where it is then dereferenced or otherwise used erroneously.
>
> -Wnull-dereference will warn for NULL pointer dereferences.  Both those
> which appear explicitly in the IL and those which occur if NULL value flows
> from a PHI to a dereference in the same block as the PHI.
>
> -Wnull-attribute will warn if NULL is used as an argument to a function call
> where the callee's attributes have declared the argument must be non-NULL.
> Similarly for NULL as a return value when the current function has an
> attribute declaring the function can never return NULL.  Similarly for NULL
> values flowing from a PHI into an argument/return in the same block as the
> PHI.
>
> We utilize the analysis done for the erroneous-paths optimization.  The
> optimizations and warnings can be enabled/disabled independently.  The
> warnings are not enabled by -Wall.

I want to raise the usual caution about warnings that are based on
optimizations.  This leads to different results in different GCC
releases, which makes it hard for other packages to use -Werror.

However, I admit that this is less of a concern when the warning is
not part of -Wall.

Ian
Jeff Law Jan. 27, 2014, 10:05 p.m. UTC | #2
On 01/27/14 14:02, Ian Lance Taylor wrote:
>> We utilize the analysis done for the erroneous-paths optimization.  The
>> optimizations and warnings can be enabled/disabled independently.  The
>> warnings are not enabled by -Wall.
>
> I want to raise the usual caution about warnings that are based on
> optimizations.  This leads to different results in different GCC
> releases, which makes it hard for other packages to use -Werror.
Absolutely.  It's an issue for any warnings that run after any of the 
optimization passes (as this one does).

There's nothing inherently difficult about running this code earlier, 
say just after entering SSA form.  You'll get a lot more false 
positives, but more stability from release to release.  There's still 
some things that would come and go due to the limited nature of 
following values out of PHI nodes in this code.

I've got the original analyzer here which was designed prior to the 
optimization work.  It actually uses the propagation engine and would 
provide much more warning stability from release to release if it were 
run just after going into SSA form.  It's something I hope to get back 
to during the next stage1 cycle.


>
> However, I admit that this is less of a concern when the warning is
> not part of -Wall.
Right.  I hadn't actually planned on even doing warnings at this stage, 
but it became pretty clear that something has needed based on the 
reactions to the erroneous-path optimizations.

jeff
Florian Weimer Feb. 4, 2014, 10:33 a.m. UTC | #3
On 01/27/2014 08:09 PM, Jeff Law wrote:

> +		      if (dereference)
> +			warning_at ((gimple_location (use_stmt)
> +				     ? gimple_location (use_stmt)
> +				     : gimple_phi_arg_location (phi, i)),
> +				    OPT_Wnull_dereference,
> +				    "Potential NULL pointer dereference");

I think warnings should start with a small letter (unless the first word 
is capitalized for other reasons).  Two more instances follow.

Instead of "Potential invalid NULL argument or return value", "potential 
NULL value used in non-NULL context" would be clearer.  It would be 
really nice if you could add a note that points to the declaration of 
non-NULL-ness for the context.

After the change, the third argument to infer_nonnull_range seems always 
true, including the call in tre-vrp.c:infer_value_range, so there's a 
minor cleanup opportunity.

Looking at infer_nonnull_range, there's an undocumented interaction with 
-fdelete-null-pointer-checks.
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 95a324c..8ffbc50 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,17 @@ 
+2014-01-27  Jeff Law  <law@redhat.com>
+
+	* common.opt (Wnull-dereference): New option.
+	(Wnull-attribute): Likewise.
+	* doc/invoke.texi: Document new warnings.
+	* gimple-ssa-isolate-paths.c: (find_implicit_erroneous_behaviour): Warn
+	for NULL dereferences and NULL values flowing into arguments or
+	return values where an attribute indicates that should never happen.
+	Do not optimize unless the optimization is enabled.
+	(find_explicit_erroneous_behaviour): Similarly.
+	(gate_isolate_erroneous_paths): Run if we want the optimization
+	or the warning.
+
+
 2014-01-27  Jakub Jelinek  <jakub@redhat.com>
 
 	PR bootstrap/59934
diff --git a/gcc/common.opt b/gcc/common.opt
index d334cf2..dcff512 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -576,6 +576,17 @@  Wlarger-than=
 Common RejectNegative Joined UInteger Warning
 -Wlarger-than=<number>	Warn if an object is larger than <number> bytes
 
+Wnull-dereference
+Common Var(warn_null_dereference) Warning
+Warn if the compiler detects paths which trigger erroneous or undefined
+behaviour due to dereferencing a NULL pointer.
+
+Wnull-attribute
+Common Var(warn_null_attribute) Warning
+Warn if the compiler detects paths which trigger erroneous or undefined
+behaviour due to passing a NULL value for an argument which must be non-NULL or
+if a function returns NULL when it has been declared as must not return NULL.
+
 Wunsafe-loop-optimizations
 Common Var(warn_unsafe_loop_optimizations) Warning
 Warn if the loop cannot be optimized due to nontrivial assumptions.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 8c620a5..81fb005 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -252,6 +252,7 @@  Objective-C and Objective-C++ Dialects}.
 -Wimplicit  -Wimplicit-function-declaration  -Wimplicit-int @gol
 -Winit-self  -Winline -Wmaybe-uninitialized @gol
 -Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
+-Wnull-dereference -Wnull-attribute @gol
 -Winvalid-pch -Wlarger-than=@var{len}  -Wunsafe-loop-optimizations @gol
 -Wlogical-op -Wlong-long @gol
 -Wmain -Wmaybe-uninitialized -Wmissing-braces  -Wmissing-field-initializers @gol
@@ -3946,6 +3947,18 @@  In order to get a warning about an unused function parameter, you must
 either specify @option{-Wextra -Wunused} (note that @option{-Wall} implies
 @option{-Wunused}), or separately specify @option{-Wunused-parameter}.
 
+@item -Wnull-dereference
+@opindex Wnull-dereference
+@opindex Wno-null-dereference
+Warn if the compiler detects paths which trigger erroneous or undefined 
+behaviour due to dereferencing a NULL pointer.
+
+@item -Wnull-attribute
+@opindex Wnull-attribute
+@opindex Wno-null-attribute
+Warn if the compiler detects paths which trigger erroneous or undefined 
+behaviour due to passing a NULL value for an argument which must be non-NULL or if a function returns NULL when it has been declared as must not return NULL.
+
 @item -Wuninitialized
 @opindex Wuninitialized
 @opindex Wno-uninitialized
diff --git a/gcc/gimple-ssa-isolate-paths.c b/gcc/gimple-ssa-isolate-paths.c
index 56fcfc8..aae403b 100644
--- a/gcc/gimple-ssa-isolate-paths.c
+++ b/gcc/gimple-ssa-isolate-paths.c
@@ -42,6 +42,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "cfgloop.h"
 #include "tree-pass.h"
 #include "tree-cfg.h"
+#include "diagnostic.h"
 
 
 static bool cfg_altered;
@@ -275,11 +276,34 @@  find_implicit_erroneous_behaviour (void)
 		  if (gimple_bb (use_stmt) != bb)
 		    continue;
 
-		  if (infer_nonnull_range (use_stmt, lhs,
-					   flag_isolate_erroneous_paths_dereference,
-					   flag_isolate_erroneous_paths_attribute))
+		  if (infer_nonnull_range (use_stmt, lhs, true, true))
 
 		    {
+		      /* By calling infer_nonnull_range again, we can determine
+			 if it was a dereference or attribute which gave us the
+			 NULL range and appropriately customize the warning.  */
+		      bool dereference = infer_nonnull_range (use_stmt, lhs,
+							  true, false);
+
+		      if (dereference)
+			warning_at ((gimple_location (use_stmt)
+				     ? gimple_location (use_stmt)
+				     : gimple_phi_arg_location (phi, i)),
+				    OPT_Wnull_dereference,
+				    "Potential NULL pointer dereference");
+		      else
+			warning_at ((gimple_location (use_stmt)
+				     ? gimple_location (use_stmt)
+				     : gimple_phi_arg_location (phi, i)),
+				    OPT_Wnull_attribute,
+				    "Potential invalid NULL argument or return value");
+
+		      if ((!flag_isolate_erroneous_paths_dereference
+			   && dereference)
+			  || (!flag_isolate_erroneous_paths_attribute
+			       && !dereference))
+			continue;
+
 		      duplicate = isolate_path (bb, duplicate,
 						e, use_stmt, lhs);
 
@@ -328,10 +352,27 @@  find_explicit_erroneous_behaviour (void)
 	  /* By passing null_pointer_node, we can use infer_nonnull_range
 	     to detect explicit NULL pointer dereferences and other uses
 	     where a non-NULL value is required.  */
-	  if (infer_nonnull_range (stmt, null_pointer_node,
-				   flag_isolate_erroneous_paths_dereference,
-				   flag_isolate_erroneous_paths_attribute))
+	  if (infer_nonnull_range (stmt, null_pointer_node, true, true))
 	    {
+	      /* By calling infer_nonnull_range again, we can determine
+		 if it was a dereference or attribute which gave us the
+		 NULL range.  */
+	      bool dereference = infer_nonnull_range (stmt, null_pointer_node,
+						      true, false);
+
+	      if (dereference)
+		warning_at (gimple_location (stmt),
+			    OPT_Wnull_dereference,
+			    "NULL pointer dereference");
+	      else
+		warning_at (gimple_location (stmt),
+			    OPT_Wnull_attribute,
+			    "Invalid NULL argument or return value");
+
+	      if ((!flag_isolate_erroneous_paths_dereference && dereference)
+		  || (!flag_isolate_erroneous_paths_attribute && !dereference))
+		continue;
+
 	      insert_trap_and_remove_trailing_statements (&si,
 							  null_pointer_node);
 
@@ -417,10 +458,10 @@  gimple_ssa_isolate_erroneous_paths (void)
 static bool
 gate_isolate_erroneous_paths (void)
 {
-  /* If we do not have a suitable builtin function for the trap statement,
-     then do not perform the optimization.  */
   return (flag_isolate_erroneous_paths_dereference != 0
-	  || flag_isolate_erroneous_paths_attribute != 0);
+	  || flag_isolate_erroneous_paths_attribute != 0
+	  || warn_null_dereference
+	  || warn_null_attribute);
 }
 
 namespace {
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index fa61d5c..a9c5654 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,16 @@ 
+2014-01-27  Jeff Law  <law@redhat.com>
+
+	* gcc.dg/nullwarn.c: New test.
+	* gcc.dg/tree-ssa/isolate-1.c: Split into two tests.  
+	isolate-1.c, and isolate-1a.c.  Add appropriate warning
+	markers for the former.  Disable the optimization, but
+	enable the warnings for the former test.  The latter
+	test just checks the optimization.
+	* gcc.dg/tree-ssa-isolate-2.c: Similarly.
+	* gcc.dg/tree-ssa-isolate-3.c: Similarly.
+	* gcc.dg/tree-ssa-isolate-4.c: Similarly.
+	* gcc.dg/tree-ssa-isolate-5.c: Similarly.
+	
 2014-01-27  Christian Bruel  <christian.bruel@st.com>
 
 	* gcc.target/sh/torture/strncmp.c: New tests.
diff --git a/gcc/testsuite/gcc.dg/nullwarn.c b/gcc/testsuite/gcc.dg/nullwarn.c
new file mode 100644
index 0000000..a496aaf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/nullwarn.c
@@ -0,0 +1,45 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wnull-dereference" } */
+
+#include <stdio.h>
+
+struct t
+{
+	int bar;
+};
+
+void test1 ()
+{
+	struct t *s = NULL;
+	s->bar = 1;  /* { dg-warning "NULL pointer dereference" } */
+}
+
+void test2 (struct t *s)
+{
+	if (s == NULL && s->bar > 2)  /* { dg-warning "NULL pointer dereference" } */
+		return;
+
+	s->bar = 3;
+}
+
+void test3 (struct t *s)
+{
+	if (s != NULL || s->bar > 2)  /* { dg-warning "NULL pointer dereference" } */
+		return;
+
+	s->bar = 3;  /* { dg-warning "NULL pointer dereference" } */
+}
+
+void test4 (struct t *s)
+{
+	if (s != NULL && s->bar > 2)  /* { dg-bogus "NULL pointer dereference" } */
+		return;
+}
+
+void test5 (struct t *s)
+{
+	if (s == NULL || s->bar > 2)  /* { dg-bogus "NULL pointer dereference" } */
+		return;
+
+}
+
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c
index f1f3101..fabf016 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c
@@ -1,6 +1,6 @@ 
 
 /* { dg-do compile } */ 
-/* { dg-options "-O2 -fdump-tree-isolate-paths" } */
+/* { dg-options "-O2 -fno-isolate-erroneous-paths-dereference -Wnull-dereference" } */
 
 
 struct demangle_component
@@ -38,23 +38,7 @@  d_type (struct d_info *di)
 {
    struct demangle_component *ret;
    ret = d_make_empty (di);
-   ret->type = 42;
-   ret->zzz = -1;
+   ret->type = 42;		/* { dg-warning "NULL pointer dereference" } */
+   ret->zzz = -1;		/* { dg-warning "NULL pointer dereference" } */
    return ret;
 }
-
-/* We're testing three aspects of isolation here.  First that isolation
-   occurs, second that if we have two null dereferences in a block that
-   that we delete everything from the first dereferece to the end of the
-   block, regardless of which comes first in the immediate use iterator
-   and finally that we set the RHS of the store to zero.  */
-/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "isolate-paths"} } */
-/* { dg-final { scan-tree-dump-times "->type = 42" 1 "isolate-paths"} } */
-/* { dg-final { scan-tree-dump-times "->type ={v} 0" 1 "isolate-paths"} } */
-/* { dg-final { scan-tree-dump-times "->zzz" 1 "isolate-paths"} } */
-/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
-
-
-
-
-
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-1a.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-1a.c
new file mode 100644
index 0000000..629f051
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-1a.c
@@ -0,0 +1,21 @@ 
+
+/* { dg-do compile } */ 
+/* { dg-options "-O2 -fdump-tree-isolate-paths" } */
+
+#include "isolate-1.c"
+
+/* We're testing three aspects of isolation here.  First that isolation
+   occurs, second that if we have two null dereferences in a block that
+   that we delete everything from the first dereferece to the end of the
+   block, regardless of which comes first in the immediate use iterator
+   and finally that we set the RHS of the store to zero.  */
+/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "isolate-paths"} } */
+/* { dg-final { scan-tree-dump-times "->type = 42" 1 "isolate-paths"} } */
+/* { dg-final { scan-tree-dump-times "->type ={v} 0" 1 "isolate-paths"} } */
+/* { dg-final { scan-tree-dump-times "->zzz" 1 "isolate-paths"} } */
+/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
+
+
+
+
+
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c
index bfcaa2b..247da04 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */ 
-/* { dg-options "-O2 -fisolate-erroneous-paths-attribute -fdump-tree-isolate-paths -fdump-tree-phicprop1" } */
+/* { dg-options "-O2 -fno-isolate-erroneous-paths-attribute -Wnull-attribute" } */
 
 
 int z;
@@ -17,7 +17,7 @@  foo(int a)
       case 0:
         return &z;
       default:
-        return (int *)0;
+        return (int *)0; /* { dg-warning "NULL argument or return value" } */
     }
 }
 
@@ -25,19 +25,5 @@  foo(int a)
 int *
 bar (void)
 {
-  return 0;
+  return 0;		/* { dg-warning "NULL argument or return value" } */
 }
-
-/* We testing that the path isolation code can take advantage of the
-   returns non-null attribute to isolate a path where NULL flows into
-   a return statement.  We test this twice, once where the NULL flows
-   from a PHI, the second with an explicit return 0 in the IL.
-
-   We also verify that after isolation phi-cprop simplifies the
-   return statement so that it returns &z directly.
-/* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "isolate-paths"} } */
-/* { dg-final { scan-tree-dump-times "return &z;" 1 "phicprop1"} } */
-/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
-/* { dg-final { cleanup-tree-dump "phicprop1" } } */
-
-
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-2a.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-2a.c
new file mode 100644
index 0000000..4798550
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-2a.c
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */ 
+/* { dg-options "-O2 -fisolate-erroneous-paths-attribute -fdump-tree-isolate-paths -fdump-tree-phicprop1" } */
+
+#include "isolate-2.c"
+
+/* We testing that the path isolation code can take advantage of the
+   returns non-null attribute to isolate a path where NULL flows into
+   a return statement.  We test this twice, once where the NULL flows
+   from a PHI, the second with an explicit return 0 in the IL.
+
+   We also verify that after isolation phi-cprop simplifies the
+   return statement so that it returns &z directly.
+/* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "isolate-paths"} } */
+/* { dg-final { scan-tree-dump-times "return &z;" 1 "phicprop1"} } */
+/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
+/* { dg-final { cleanup-tree-dump "phicprop1" } } */
+
+
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c
index 7dddd80..7a56548 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */ 
-/* { dg-options "-O2 -fdump-tree-isolate-paths" } */
+/* { dg-options "-O2 -fno-isolate-erroneous-paths-dereference -Wnull-dereference" } */
 
 
 typedef long unsigned int size_t;
@@ -28,7 +28,7 @@  static __inline__ void
 VEC_rtx_gc_safe_grow (VEC_rtx_gc ** vec_, int size_, const char *file_,
                       unsigned line_, const char *function_)
 {
-  ((*vec_) ? &(*vec_)->base : 0)->num = size_;
+  ((*vec_) ? &(*vec_)->base : 0)->num = size_; /* { dg-warning "NULL pointer dereference" } */
 } 
 
 static __inline__ void
@@ -50,16 +50,3 @@  init_alias_analysis (void)
    (&(reg_base_value), maxreg, "../../../gcc-4.6.0/gcc/alias.c", 2755,
     __FUNCTION__, arf ()));
 }
-
-
-
-/* This is an example of how a NULL pointer dereference can show up
-   without a PHI.  Note VEC_rtx_gcc_safe_grow.  If an earlier pass
-   (such as VRP) isolates the NULL path for some reason or another
-   we end up with an explicit NULL dereference in the IL.  Yes, it
-   started with a PHI, but by the time the path isolation code runs
-   its explicit in the IL.  */
-/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "isolate-paths"} } */
-/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
-
-
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-3a.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-3a.c
new file mode 100644
index 0000000..fdfafff
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-3a.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile } */ 
+/* { dg-options "-O2 -fdump-tree-isolate-paths" } */
+
+#include "isolate-3.c"
+
+/* This is an example of how a NULL pointer dereference can show up
+   without a PHI.  Note VEC_rtx_gcc_safe_grow.  If an earlier pass
+   (such as VRP) isolates the NULL path for some reason or another
+   we end up with an explicit NULL dereference in the IL.  Yes, it
+   started with a PHI, but by the time the path isolation code runs
+   its explicit in the IL.  */
+/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "isolate-paths"} } */
+/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
+
+
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c
index c9c074d..ed48e5f 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */ 
-/* { dg-options "-O2 -fisolate-erroneous-paths-attribute -fdump-tree-isolate-paths -fdump-tree-phicprop1" } */
+/* { dg-options "-O2 -fno-isolate-erroneous-paths-attribute -Wnull-attribute" } */
 
 
 extern void foo(void *) __attribute__ ((__nonnull__ (1)));
@@ -9,24 +9,11 @@  int z;
 void
 com (int a)
 {
-    foo (a == 42 ? &z  : (void *) 0);
+    foo (a == 42 ? &z  : (void *) 0); /* { dg-warning "NULL argument or return value" } */
 }
 
 void
 bar (void)
 {
-  foo ((void *)0);
+  foo ((void *)0); /* { dg-warning "NULL argument or return value" } */
 }
-
-/* We testing that the path isolation code can take advantage of the
-   returns non-null attribute to isolate a path where NULL flows into
-   a return statement.
-
-   We also verify that after isolation phi-cprop simplifies the
-   return statement so that it returns &z directly.
-/* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "isolate-paths"} } */
-/* { dg-final { scan-tree-dump-times "foo .&z.;" 1 "phicprop1"} } */
-/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
-/* { dg-final { cleanup-tree-dump "phicprop1" } } */
-
-
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-4a.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-4a.c
new file mode 100644
index 0000000..85007ae
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-4a.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */ 
+/* { dg-options "-O2 -fisolate-erroneous-paths-attribute -fdump-tree-isolate-paths -fdump-tree-phicprop1" } */
+
+#include "isolate-4.c"
+
+/* We testing that the path isolation code can take advantage of the
+   returns non-null attribute to isolate a path where NULL flows into
+   a return statement.
+
+   We also verify that after isolation phi-cprop simplifies the
+   return statement so that it returns &z directly.
+/* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "isolate-paths"} } */
+/* { dg-final { scan-tree-dump-times "foo .&z.;" 1 "phicprop1"} } */
+/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
+/* { dg-final { cleanup-tree-dump "phicprop1" } } */
+
+
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c
index 4d01d5c..89afc3a 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */ 
-/* { dg-options "-O2 -fdump-tree-isolate-paths -fdump-tree-optimized" } */
+/* { dg-options "-O2 -fno-isolate-erroneous-paths-dereference -Wnull-dereference" } */
 
 struct demangle_component
 {
@@ -32,21 +32,7 @@  d_type (struct d_info *di)
 {
    struct demangle_component *ret;
    ret = d_make_empty (di);
-   foo (ret->type);
-   bar (ret->zzz);
+   foo (ret->type); /* { dg-warning "NULL pointer dereference" } */
+   bar (ret->zzz); /* { dg-warning "NULL pointer dereference" } */
    return ret;
 }
-
-/* We're testing two aspects of isolation here.  First that isolation
-   occurs, second that if we have two null dereferences in a block that
-   that we delete everything from the first dereferece to the end of the
-   block, regardless of which comes first in the immediate use iterator.
-
-   We leave the 0->type in the IL, so expect to see ->type twice.  */
-/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "isolate-paths"} } */
-/* { dg-final { scan-tree-dump-times "->type" 2 "isolate-paths"} } */
-/* { dg-final { scan-tree-dump-times "->type" 1 "optimized"} } */
-/* { dg-final { scan-tree-dump-times "\\.type" 1 "optimized"} } */
-/* { dg-final { scan-tree-dump-times "->zzz" 1 "isolate-paths"} } */
-/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
-/* { dg-final { cleanup-tree-dump "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-5a.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-5a.c
new file mode 100644
index 0000000..0402be9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-5a.c
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */ 
+/* { dg-options "-O2 -fdump-tree-isolate-paths -fdump-tree-optimized" } */
+
+#include "isolate-5.c"
+
+/* We're testing two aspects of isolation here.  First that isolation
+   occurs, second that if we have two null dereferences in a block that
+   that we delete everything from the first dereferece to the end of the
+   block, regardless of which comes first in the immediate use iterator.
+
+   We leave the 0->type in the IL, so expect to see ->type twice.  */
+/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "isolate-paths"} } */
+/* { dg-final { scan-tree-dump-times "->type" 2 "isolate-paths"} } */
+/* { dg-final { scan-tree-dump-times "->type" 1 "optimized"} } */
+/* { dg-final { scan-tree-dump-times "\\.type" 1 "optimized"} } */
+/* { dg-final { scan-tree-dump-times "->zzz" 1 "isolate-paths"} } */
+/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */