diff mbox

Move Asan instrumentation to sanopt pass

Message ID 20140719093112.GO3003@laptop.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek July 19, 2014, 9:31 a.m. UTC 
On Fri, Jul 18, 2014 at 10:21:16PM +0400, Yuri Gribov wrote:
> On Fri, Jul 18, 2014 at 9:23 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Fri, Jul 18, 2014 at 08:42:35PM +0400, Yuri Gribov wrote:
> >> > Uh.  Can you please explain this?  That sounds weird.
> >>
> >> Sure, this caused maybe-uninitialized warnings with iterator during
> >> bootstrap. It turned out that *bb_seq_addr (bb) in gsi_start (bb)
> >> returned something like bb.flags & GIMPLE ? bb.il.gimple.seq : NULL
> >> and when GCC saw *NULL it rushed to generated uninitialized read. This
> >> assert silences compiler.
> >
> > Why it doesn't trip elsewhere?  Do you get the maybe-uninit warning
> > on asan.c or somewhere else without that?
> 
> Just in pass_sanopt::execute in asan.c.

Sounds like a bug, most likely in the lim pass.  If it for some reason
decides to load from possibly uninitialized memory before the loop, it
better should make sure TREE_NO_WARNING is set.  Can you please
file a PR for this, with preprocessed asan.c and minimal gcc options to
reproduce it?

As for the workaround, my strong preference would be to do something like
following incrmeental patch instead, just use different gsi in the two
loops.



	Jakub

Comments

Yuri Gribov July 19, 2014, 5:56 p.m. UTC | #1 
On Sat, Jul 19, 2014 at 1:31 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Jul 18, 2014 at 10:21:16PM +0400, Yuri Gribov wrote:
> Sounds like a bug, most likely in the lim pass.  If it for some reason
> decides to load from possibly uninitialized memory before the loop, it
> better should make sure TREE_NO_WARNING is set.  Can you please
> file a PR for this, with preprocessed asan.c and minimal gcc options to
> reproduce it?

Sure, I'll also check if I can submit a patch for this.

> As for the workaround, my strong preference would be to do something like
> following incrmeental patch instead, just use different gsi in the two
> loops.

Yeah, I also used this as a WAR originally.

-Y
diff mbox

Patch

--- gcc/asan.c.jj	2014-07-19 10:08:28.000000000 +0200
+++ gcc/asan.c	2014-07-19 11:22:38.137950281 +0200
@@ -2746,35 +2746,36 @@  unsigned int
 pass_sanopt::execute (function *fun)
 {
   basic_block bb;
-  gimple_stmt_iterator gsi;
 
   int asan_num_accesses = 0;
   FOR_EACH_BB_FN (bb, fun)
-    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-      {
- 	gimple stmt = gsi_stmt (gsi);
-	if (is_gimple_call (stmt) && gimple_call_internal_p (stmt))
-	  {
-	    enum internal_fn ifn = gimple_call_internal_fn (stmt);
-	    switch (ifn)
-	      {
-	      case IFN_ASAN_LOAD:
-	      case IFN_ASAN_STORE:
+    {
+      gimple_stmt_iterator gsi;
+      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+	{
+	  gimple stmt = gsi_stmt (gsi);
+	  if (is_gimple_call (stmt) && gimple_call_internal_p (stmt))
+	    {
+	      enum internal_fn ifn = gimple_call_internal_fn (stmt);
+	      switch (ifn)
 		{
+		case IFN_ASAN_LOAD:
+		case IFN_ASAN_STORE:
 		  ++asan_num_accesses;
 		  break;
+		default:
+		  break;
 		}
-	      default:
-		break;
-	      }
 	    }
         }
+    }
 
   bool use_calls = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
     && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
     
   FOR_EACH_BB_FN (bb, fun)
     {
+      gimple_stmt_iterator gsi;
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
 	{
 	  gimple stmt = gsi_stmt (gsi);