[Bug libmudflap/53952] New: [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test

[Bug libmudflap/53952] New: [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53952

             Bug #: 53952
           Summary: [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx
                    ( -O[123]) execution test
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libmudflap
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: rguenth@gcc.gnu.org


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.

[Bug libmudflap/53952] [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53952

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.0

[Bug libmudflap/53952] [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53952

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug libmudflap/53952] [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test

[aoliva at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53952

Alexandre Oliva <aoliva at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-11-29
                 CC|                            |aoliva at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #1 from Alexandre Oliva <aoliva at gcc dot gnu.org> 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

[Bug libmudflap/53952] [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test

[jakub at gcc dot gnu.org]

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).

[Bug libmudflap/53952] [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53952

--- Comment #3 from rguenther at suse dot de <rguenther at suse dot 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.

[Bug libmudflap/53952] [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test

[aoliva at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53952

Alexandre Oliva <aoliva at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |aoliva at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #4 from Alexandre Oliva <aoliva at gcc dot gnu.org> 2012-12-06 21:26:47 UTC ---
Mine.  Updated patch posted here:
http://gcc.gnu.org/ml/gcc-patches/2012-12/msg00371.html

[Bug libmudflap/53952] [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test

[aoliva at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53952

--- Comment #5 from Alexandre Oliva <aoliva at gcc dot gnu.org> 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

[Bug libmudflap/53952] [4.8 Regression] FAIL: libmudflap.c++/pass55-frag.cxx ( -O[123]) execution test

[aoliva at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53952

Alexandre Oliva <aoliva at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #6 from Alexandre Oliva <aoliva at gcc dot gnu.org> 2012-12-15 10:30:04 UTC ---
Fixed