[Bug c++/33407] New: C++ operator new and new expression do not change dynamic type

[Bug c++/33407] New: C++ operator new and new expression do not change dynamic type

[rguenth at gcc dot gnu dot org]

Nothing in the IL of the following testcase prevents the stores
to *q and *r in function doit from being reordered:

extern "C" void * malloc(__SIZE_TYPE__);
extern "C" void abort(void);

void *p;
void __attribute__((noinline)) init(void)
{
  p = malloc(4);
}

inline void *operator new(__SIZE_TYPE__)
{
  return p;
}

inline void operator delete (void*) {}

int * __attribute__((noinline)) doit(void)
{
  float *q = new float;
  *q = 1.0;
  delete q;
  int *r = new int;
  *r = 1;
  return r;
}

int main()
{
  if (*doit() != 1)
    abort();
  return 0;
}

from the first alias run results:

int* doit() ()
{
  void * D.1643;
  void * D.1643;
  void * D.1639;
  void * D.1639;
  int * r;
  float * q;

<bb 2>:
  # VUSE <p_14(D)>
  D.1639_7 = p;
  q_2 = (float *) D.1639_7;
  # SMT.6_16 = VDEF <SMT.6_15(D)>
  *q_2 = 1.0e+0;
  # VUSE <p_14(D)>
  D.1643_8 = p;
  r_4 = (int *) D.1643_8;
  # SMT.7_18 = VDEF <SMT.7_17(D)>
  *r_4 = 1;
  return r_4;

}

One way to fix this is to make sure that if operator new is inlined
we insert a CHANGE_DYNAMIC_TYPE_EXPR.


-- 
           Summary: C++ operator new and new expression do not change
                    dynamic type
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: wrong-code, alias
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rguenth at gcc dot gnu dot org


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

[Bug c++/33407] C++ operator new and new expression do not change dynamic type

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2007-09-12 10:00 -------
Related to PR29286.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|                            |29286


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

[Bug c++/33407] C++ operator new and new expression do not change dynamic type

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2007-09-12 10:13 -------
main should call init(), but it doesn't make a difference for the IL.  The
bug is wrong-IL for me only at the moment, but nothing prevents the two stores
from being reordered.

Here's one that abort()s at runtime on i686 with -O2 for me:  (same trick
as in PR29286, trick LIM into reordering the stores :))

extern "C" void * malloc(__SIZE_TYPE__);
extern "C" void abort(void);

void *p;
void __attribute__((noinline)) init(void)
{
  p = malloc(4);
}

inline void *operator new(__SIZE_TYPE__)
{
  return p;
}

inline void operator delete (void*) {}

int * __attribute__((noinline)) doit(int n)
{
  float *q;
  int *r;

  for (int i=0; i<n; ++i)
  {
    q = new float;
    *q = 1.0;
    delete q;
    r = new int;
    *r = 1;
  }

  return r;
}

int main()
{
  init();
  if (*doit(1) != 1)
    abort();
  return 0;
}


-- 


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2007-09-12 10:18 -------
4.2 works by luck as we weakened aliasing by the NONLOCAL stuff.  2.95 works
for whatever reason ;)  Even pre-tree-ssa we fail with -O2 (but it works with
-O).


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |3.3.6 3.4.6 4.0.4 4.1.2
                   |                            |4.3.0
      Known to work|                            |2.95.4 4.2.1
            Summary|C++ operator new and new    |[4.1/4.3 Regression] C++
                   |expression do not change    |operator new and new
                   |dynamic type                |expression do not change
                   |                            |dynamic type
   Target Milestone|---                         |4.3.0


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2007-09-12 10:20 -------
-O fails with -fstrict-aliasing as well.


-- 


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[jakub at gcc dot gnu dot org]

------- Comment #5 from jakub at gcc dot gnu dot org  2007-09-12 16:13 -------
Could we limit adding of the CHANGE_DYNAMIC_TYPE_EXPRs just to the case
where operator new or __attribute__((malloc)) marked FUNCTION_DECL is not
external?  That would be solid even for LTO, if you LTO and have say malloc
implemented among the stuff you read in, then we need to handle that carefully,
but if it is for the compiler just a black box in libc, there is no need
to pollute the IL.


-- 


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2007-09-13 09:36 -------
Yes, this was an idea I had as well.


-- 


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[mmitchel at gcc dot gnu dot org]

------- Comment #7 from mmitchel at gcc dot gnu dot org  2007-10-10 17:58 -------
We really need to fix this class of problems.  Every release of GCC over the
past couple of years has had serious aliasing issues that caused real-world
programs to fall over.  We can fix this by making the compiler smarter
(teaching it more about what can alias) or dumber (by making it do less with
aliasing information), but we need to do one or the other.


-- 

mmitchel at gcc dot gnu dot org changed:

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


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[rguenth at gcc dot gnu dot org]

------- Comment #8 from rguenth at gcc dot gnu dot org  2008-01-02 12:57 -------
Confirmed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2008-01-02 12:57:34
               date|                            |


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[rguenth at gcc dot gnu dot org]

------- Comment #9 from rguenth at gcc dot gnu dot org  2008-01-16 10:52 -------
Ian, is there any chance you can look at this?  Your fix for PR29286 could be
extended to handle this as well, right?


-- 


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[ian at airs dot com]

------- Comment #10 from ian at airs dot com  2008-01-16 22:49 -------
Created an attachment (id=14953)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14953&action=view)
Possible patch

This untested patch fixes the problem with the test case.


-- 


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[rguenth at gcc dot gnu dot org]

------- Comment #11 from rguenth at gcc dot gnu dot org  2008-01-18 12:43 -------
The patch should indeed work and I suggest we go forward with it for 4.3.

For 4.4, can we use this sort of flag (name it no_tbaa_for_result) to handle
both the operator new and the placement new case where for the latter we
at the moment do the CHANGE_DYNAMIC_TYPE_EXPR thing?  After all, the
placement new also gets inlined from its libstdc++ implementation.


-- 


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[ian at gcc dot gnu dot org]

------- Comment #12 from ian at gcc dot gnu dot org  2008-01-18 15:25 -------
Subject: Bug 33407

Author: ian
Date: Fri Jan 18 15:25:02 2008
New Revision: 131629

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131629
Log:
        PR c++/33407
./:
        * tree.h (DECL_IS_OPERATOR_NEW): Define.
        (struct tree_function_decl): Add new field operator_new_flag.
        * tree-inline.c (expand_call_inline): When inlining a call to
        operator new, force the return value to go into a variable, and
        set DECL_NO_TBAA_P on that variable.
        * c-decl.c (merge_decls): Merge DECL_IS_OPERATOR_NEW flag.
cp/:
        * decl.c (duplicate_decls): Copy DECL_IS_OPERATOR_NEW flag.
        (grok_op_properties): For NEW_EXPR and VEC_NEW_EXPR set
        DECL_IS_OPERATOR_NEW flag.
testsuite/:
        * g++.dg/init/new26.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/init/new26.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-decl.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-inline.c
    trunk/gcc/tree.h


-- 


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[ian at airs dot com]

------- Comment #13 from ian at airs dot com  2008-01-18 16:01 -------
I think you're right.  If the call to placement new is not inlined, and if we
don't know anything special about it (which we currently don't), then it seems
to me that everything is bound to work OK.  It is only the inlining that makes
a difference.

Pity we didn't realize that before.  Still, the heart of
CHANGE_DYNAMIC_TYPE_EXPR is compute_tbaa_pruning, and that will remain.  What
can be removed is the code in cp/init.c which creates CHANGE_DYNAMIC_TYPE_EXPR
and the code in find_func_aliases which sets the no_tbaa_pruning flag.

I have a vague memory that there was some weird test case in PR 29286 which we
would need to reconsider.  But I couldn't find it in a quick look, and I'm not
sure my memory is correct.


-- 


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[ian at airs dot com]

------- Comment #14 from ian at airs dot com  2008-01-18 16:17 -------
This is now fixed.


-- 

ian at airs dot com changed:

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


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

[Bug c++/33407] [4.1/4.3 Regression] C++ operator new and new expression do not change dynamic type

[ian at gcc dot gnu dot org]

------- Comment #15 from ian at gcc dot gnu dot org  2008-01-28 19:44 -------
Subject: Bug 33407

Author: ian
Date: Mon Jan 28 19:43:51 2008
New Revision: 131916

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131916
Log:
        PR c++/34862
        PR c++/33407
        * tree-ssa-copyrename.c (copy_rename_partition_coalesce): Don't
        coalesce pointers if they have different DECL_NO_TBAA_P values.
        * tree-ssa-copy.c (may_propagate_copy): Don't propagate copies
        between variables with different DECL_NO_TBAA_P values.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-copy.c
    trunk/gcc/tree-ssa-copyrename.c


-- 


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