Re: [asan] Patch - fix an ICE in asan.c

[Tobias Burnus <burnus at net-b dot de>]

Tobias Burnus wrote:
> I spoke too early. With the updated patch, there is no ICE, but one 
> crashes for the following valid program with:

But with my original patch, it works.

To recap: My "if (gsi_end_p (i)) break;" (cf. [1]) fixes my original 
issue (ICE for fail31.ii; [1]) and gives the correct diagnostic at run 
time for strlen in the code [4] (both for correct and out-of-bounds 
programs).

While Jakub's "*iter = gsi_for_stmt (call);" (cf. [3]) fixes the ICE for 
my fail10.ii program [2]; I haven't tried to construct a run-time 
version for that code.

Updated patches attached (for the "asan" branch and for the trunk on top 
of Dodji's patches; I have only tested the latter).

Hopefully, the test suite will be working soon, it should help finding 
such issues.

Tobias

[1] fail31.ii (strlen ICE): 
http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00786.html
[2] fail10.ii (control flow in BB ICE): 
http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00791.html
[3] Jakub's patch: http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00801.html
[4] strlen run test: http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00809.html
(This patch is for the "asan" branch.)

2012-11-10  Tobias Burnus  <burnus@net-b.de>
	    Jakub Jelinek  <jakub@redhat.com>

        * asan.c (maybe_instrument_builtin_call): Set *iter
        to gsi for the call at the end.
	(transform_statements): Leave loop when gsi_end_p.

diff --git a/gcc/asan.c b/gcc/asan.c
index 155e84b..f5e357a 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1187,14 +1187,15 @@ maybe_instrument_builtin_call (gimple_stmt_iterator *iter)
 				      loc, /*is_store=*/false);
       if (source1 != NULL_TREE)
 	instrument_mem_region_access (source1, len, iter,
 				      loc, /*is_store=*/false);
       else if (dest != NULL_TREE)
 	instrument_mem_region_access (dest, len, iter,
 				      loc, /*is_store=*/true);
+      *iter = gsi_for_stmt (call);
       return true;
     }
   return false;
 }
 
 /*  Instrument the assignment statement ITER if it is subject to
     instrumentation.  */
@@ -1243,14 +1244,16 @@ transform_statements (void)
         {
 	  gimple s = gsi_stmt (i);
 
 	  if (gimple_assign_single_p (s))
 	    instrument_assignment (&i);
 	  else if (is_gimple_call (s))
 	    maybe_instrument_call (&i);
+         if (gsi_end_p (i))
+           break;
         }
     }
 }
 
 /* Build
    struct __asan_global
    {

(This patch is for the trunk after the "asan" patch has been applied.)

2012-11-10  Tobias Burnus  <burnus@net-b.de>
	    Jakub Jelinek  <jakub@redhat.com>

        * asan.c (maybe_instrument_builtin_call): Set *iter
        to gsi for the call at the end.
	(transform_statements): Leave loop when gsi_end_p.

--- gcc/asan.c.orig	2012-11-09 21:26:26.000000000 +0100
+++ gcc/asan.c	2012-11-10 19:23:33.000000000 +0100
@@ -1302,16 +1302,17 @@ instrument_builtin_call (gimple_stmt_ite
 	instrument_mem_region_access (source0, len, iter,
 				      loc, /*is_store=*/false);
       if (source1 != NULL_TREE)
 	instrument_mem_region_access (source1, len, iter,
 				      loc, /*is_store=*/false);
       else if (dest != NULL_TREE)
 	instrument_mem_region_access (dest, len, iter,
 				      loc, /*is_store=*/true);
+      *iter = gsi_for_stmt (call);
     }
 }
 
 /*  Instrument the assignment statement ITER if it is subject to
     instrumentation.  */
 
 static void
 instrument_assignment (gimple_stmt_iterator *iter)
@@ -1357,16 +1358,18 @@ transform_statements (void)
       for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
         {
 	  gimple s = gsi_stmt (i);
 
 	  if (gimple_assign_single_p (s))
 	    instrument_assignment (&i);
 	  else if (is_gimple_call (s))
 	    maybe_instrument_call (&i);
+	  if (gsi_end_p (i))
+	    break;
         }
     }
 }
 
 /* Build
    struct __asan_global
    {
      const void *__beg;