Bug 53952 - [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test
Summary: [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libmudflap (show other bugs)
Version: 4.8.0
: P1 normal
Target Milestone: 4.8.0
Assignee: Alexandre Oliva
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-13 13:35 UTC by Richard Biener
Modified: 2012-12-15 10:30 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-11-29 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2012-07-13 13:35:15 UTC
I see since quite some time

FAIL: libmudflap.c++/pass55-frag.cxx ( -O) execution test
FAIL: libmudflap.c++/pass55-frag.cxx (-O2) execution test
FAIL: libmudflap.c++/pass55-frag.cxx (-O3) execution test

with -O0 it does not fail.

I suspect stricter variable lifetime handling but did not try to confirm.
Comment 1 Alexandre Oliva 2012-11-29 00:41:36 UTC
The problem is that SRA replaces a copy of an iterator type to a variable that was not addressable with a copy of the pointer held in the iterator.  To that end, it takes the address of the iterator.  This takes place after mudflap1, so that the address range for the variable is never registered, but before mudflap2, that introduces the access checks for addressed variables and marking them addressable.  Oops.

(it's the iterator passed to _M_insert_aux, in the copy of push_back inlined into main)

This ugly patch fixes it, but I don't really like it.  Any other suggestions?

--- a/gcc/tree-mudflap.c
+++ b/gcc/tree-mudflap.c
@@ -933,6 +933,12 @@ mf_xform_derefs_1 (gimple_stmt_iterator *iter, tree *tp,
       return;
     }
 
+  /* SRA takes the address of variables that were not addressable.
+     Don't check them.  */
+  if (TREE_CODE (addr) == ADDR_EXPR
+      && !TREE_ADDRESSABLE (TREE_OPERAND (addr, 0)))
+    return;
+
   mf_build_check_statement_for (base, limit, iter, location, dirflag);
 }
 /* Transform
Comment 2 Jakub Jelinek 2012-11-29 07:57:12 UTC
I'd say it is the right thing, just should be done at a different spot, instead of doing it for everything just check it for MEM_REF.  I think only MEM_REF can have ADDR_EXPR of a non-TREE_ADDRESSABLE DECL_P as argument, look e.g. at
mem_ref_refers_to_non_mem_p in expr.c (which checks more than that), or
opf_non_addressable and opf_not_non_addressable handling in tree-ssa-operands.c.
So, MEM_REF with ADDR_EXPR of a non-TREE_ADDRESSABLE DECL_P IMHO doesn't need to be instrumented (seems I should modify asan/tsan too).
Comment 3 rguenther@suse.de 2012-11-29 09:26:31 UTC
On Thu, 29 Nov 2012, jakub at gcc dot gnu.org wrote:

> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53952
> 
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org
> 
> --- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-11-29 07:57:12 UTC ---
> I'd say it is the right thing, just should be done at a different spot, instead
> of doing it for everything just check it for MEM_REF.  I think only MEM_REF can
> have ADDR_EXPR of a non-TREE_ADDRESSABLE DECL_P as argument, look e.g. at
> mem_ref_refers_to_non_mem_p in expr.c (which checks more than that), or
> opf_non_addressable and opf_not_non_addressable handling in
> tree-ssa-operands.c.
> So, MEM_REF with ADDR_EXPR of a non-TREE_ADDRESSABLE DECL_P IMHO doesn't need
> to be instrumented (seems I should modify asan/tsan too).

Yes, that's true.  Only MEM_REF and TARGET_MEM_REF can have ADDR_EXRPs
of sth non-addressable.  And yes, no instrumentation necessary.

Richard.
Comment 4 Alexandre Oliva 2012-12-06 21:26:47 UTC
Mine.  Updated patch posted here:
http://gcc.gnu.org/ml/gcc-patches/2012-12/msg00371.html
Comment 5 Alexandre Oliva 2012-12-15 10:25:24 UTC
Author: aoliva
Date: Sat Dec 15 10:25:15 2012
New Revision: 194519

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=194519
Log:
PR libmudflap/53952
* expr.c (mem_ref_refers_to_non_mem_p): Factor out
implementation into...
(addr_expr_of_non_mem_decl_p_1): ... this new function.
(addr_expr_of_non_mem_decl_p): New.
* tree.h (addr_expr_of_non_mem_decl_p): Declare.
* tree-mudflap.c (mf_xform_derefs_1): Don't change MEM_REFs
and TARGET_MEM_REFs that have an ADDR_EXPR of a non-mem DECL
as base operand.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/expr.c
    trunk/gcc/tree-mudflap.c
    trunk/gcc/tree.h
Comment 6 Alexandre Oliva 2012-12-15 10:30:04 UTC
Fixed