[Bug c++/47541] New: For integer pointers, the value of ++*p is not written back to memory

[Bug c++/47541] New: For integer pointers, the value of ++*p is not written back to memory

[veksler at il dot ibm.com]

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

           Summary: For integer pointers, the value of ++*p  is not
                    written back to memory
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: veksler@il.ibm.com


The problem occurs in a copy constructor both for 386 and for X86-64. This is a
copy constructor of a reference counting object which uses int* to keep 
track of the number of copies of an "A" object. The copy constructor messes up,
it calculates the new count (in some cases), but forgets to write it back to
the int*.

The attached test case is a bit complicated, yet it is a simplification of real
code. There are two options to see the issue:
  $ g++ -O2 -g refcount.cpp && ./a.out
  Segmentation fault (core dumped)

  $ g++ -O2 -DTRACE -g refcount.cpp && ./a.out
  A::A()
  RefCount()
  count=1
  RefCount(const RefCount&)
  count=1
  ~RefCount()
  A::~A
  count=0
  Segmentation fault (core dumped)

As you can see, the reference is not incremented when it should be.

$ g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/4.5.1/lto-wrapper
Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
--infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla
--enable-bootstrap --enable-shared --enable-threads=posix
--enable-checking=release --with-system-zlib --enable-__cxa_atexit
--disable-libunwind-exceptions --enable-gnu-unique-object
--enable-linker-build-id
--enable-languages=c,c++,objc,obj-c++,java,fortran,ada,lto --enable-plugin
--enable-java-awt=gtk --disable-dssi
--with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre
--enable-libgcj-multifile --enable-java-maintainer-mode
--with-ecj-jar=/usr/share/java/eclipse-ecj.jar --disable-libjava-multilib
--with-ppl --with-cloog --with-tune=generic --with-arch_32=i686
--build=x86_64-redhat-linux
Thread model: posix
gcc version 4.5.1 20100924 (Red Hat 4.5.1-4) (GCC)

[Bug c++/47541] For integer pointers, the value of ++*p is not written back to memory

[schwab@linux-m68k.org]

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

Andreas Schwab <schwab@linux-m68k.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2011.01.30 09:44:37
     Ever Confirmed|0                           |1

--- Comment #2 from Andreas Schwab <schwab@linux-m68k.org> 2011-01-30 09:44:37 UTC ---
There is no attachment.

[Bug c++/47541] For integer pointers, the value of ++*p is not written back to memory

[veksler at il dot ibm.com]

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

Michael Veksler <veksler at il dot ibm.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|4.5.1                       |4.6.0

--- Comment #1 from Michael Veksler <veksler at il dot ibm.com> 2011-01-30 09:39:50 UTC ---
I recreated this bug, with the same attachment, for 4.6.0 20110129
(experimental):

Target: x86_64-unknown-linux-gnu
Configured with: ../gcc-4.6-20110129/configure--enable-languages=c++
Thread model: posix
gcc version 4.6.0 20110129 (experimental) (GCC) 


Updating the version accordingly. 

Also, it seems to me that this worked fine with the gcc-4.4 series (I was able
to work with that code just fine with fc13 which, as far as I can remember,
comes with gcc-4.4). In that case, this is a regression.

[Bug c++/47541] For integer pointers, the value of ++*p is not written back to memory

[veksler at il dot ibm.com]

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

--- Comment #3 from Michael Veksler <veksler at il dot ibm.com> 2011-01-30 12:39:04 UTC ---
Created attachment 23168
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23168
This is a minimal file to reproduce this error

Strange, it got lost somehow. Here it is, again.

[Bug c++/47541] [4.5/4.6 Regression] For integer pointers, the value of ++*p is not written back to memory

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW
                 CC|                            |hubicka at gcc dot gnu.org
   Target Milestone|---                         |4.5.3
            Summary|For integer pointers, the   |[4.5/4.6 Regression] For
                   |value of ++*p  is not       |integer pointers, the value
                   |written back to memory      |of ++*p  is not written
                   |                            |back to memory

--- Comment #4 from H.J. Lu <hjl.tools at gmail dot com> 2011-01-30 16:03:30 UTC ---
It is caused by revision 152520:

http://gcc.gnu.org/ml/gcc-cvs/2009-10/msg00169.html

[Bug c++/47541] [4.5/4.6 Regression] For integer pointers, the value of ++*p is not written back to memory

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-31 14:26:39 UTC ---
It is tree dse1 that removes the needed increment (the only dse_optimize_stmt
optimized store in the whole testcase).

[Bug c++/47541] [4.5/4.6 Regression] For integer pointers, the value of ++*p is not written back to memory

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-01-31 15:10:34 UTC ---
Mine.

[Bug c++/47541] [4.5/4.6 Regression] For integer pointers, the value of ++*p is not written back to memory

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-31 15:57:28 UTC ---
Created attachment 23180
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23180
pr47541.ii

For Richard:
x86_64-linux, trunk from today, ./cc1plus -O2 -fdump-tree-alias
-fdump-tree-dse-all pr47541.ii
;; Function virtual void B::Push(Wrapper) (_ZN1B4PushE7Wrapper)
...
Deleted dead store '*D.10321_19 = D.10324_22;
'
and the delete is invalid.
*.alias has:
ptr = &PARM_NOALIAS.204.64+64
D.10321_19 = *ptr + 128
...
D.10321_19 = { }
...
D.10321_19, points-to vars: { }

[Bug c++/47541] [4.5/4.6 Regression] For integer pointers, the value of ++*p is not written back to memory

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-01-31 16:28:19 UTC ---
Hmm.  We have

  D.10321_19 = MEM[(const struct RefCount &)ptr_3(D) + 8].count_;
  MEM[(struct RefCount *)D.9717_8 + 8B].count_ = D.10321_19;
  D.10322_20 = MEM[(struct RefCount *)D.9717_8 + 8B].count_;
...
  MEM[(int *)D.10322_20] = D.10324_22;

so _19 isn't used at this point (as in, dereferenced) directly.

Constraints:

PARM_NOALIAS.204.64+64 = NONLOCAL
PARM_NOALIAS.204.128+64 = NONLOCAL
ptr = &PARM_NOALIAS.204.64+64
D.10321_19 = *ptr + 128
*D.9717_8 + 128 = D.10321_19
D.10322_20 = *D.9717_8 + 128
D.10323_21 = *D.10322_20

Note the funny fact that no constraint is there for
the subvar starting at 0.  And we end up with

  # PT = nonlocal escaped
  D.10320_18 = MEM[(const struct RefCount &)ptr_3(D) + 8].ptr_;
  MEM[(struct RefCount *)D.9717_8 + 8B].ptr_ = D.10320_18;
  # PT =
  D.10321_19 = MEM[(const struct RefCount &)ptr_3(D) + 8].count_;
  MEM[(struct RefCount *)D.9717_8 + 8B].count_ = D.10321_19;
  # PT = nonlocal escaped
  D.10322_20 = MEM[(struct RefCount *)D.9717_8 + 8B].count_;

which means that we think we access PARM_NOALIAS.204.196+64 when
loading from count_.  This looks like a type issue to me, related to
pass-by-reference + restrict handling.  Eventually related to
how inheritance is represented.

[Bug c++/47541] [4.5/4.6 Regression] For integer pointers, the value of ++*p is not written back to memory

[rguenth at gcc dot gnu.org]

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

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-01-31 16:40:11 UTC ---
Testcase:

struct Dummy {};
struct RefCount : public Dummy {
    ~RefCount(); /* Has to be non-pod.  */
    int *a;
    int *b;
};
RefCount::~RefCount(){}
struct Wrapper : public Dummy { RefCount ref; };
void __attribute__((noinline,noclone))
Push(Wrapper ptr)
{
  *ptr.ref.b = 0;
}
extern "C" void abort (void);
int main()
{
  int a = 1, b = 1;
  Wrapper x;
  x.ref.a = &a;
  x.ref.b = &b;
  Push(x);
  if (b != 0)
    abort ();
  return 0;
}

[Bug c++/47541] [4.5/4.6 Regression] For integer pointers, the value of ++*p is not written back to memory

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-01-31 17:07:19 UTC ---
So, type Wrapper as seen by PTA has

Wrapper
  offset 64:  RefCount

RefCount
  offset 0: a
  offset 64: b

and the issue is that the ptr parameter gets the address of a as initial
value by PTA.  Oops.  It really needs to get offset zero as initial address!
We should always keep a subvar for offset zero.

I have a patch.

[Bug c++/47541] [4.5/4.6 Regression] For integer pointers, the value of ++*p is not written back to memory

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-02-01 09:47:26 UTC ---
Author: rguenth
Date: Tue Feb  1 09:47:21 2011
New Revision: 169468

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169468
Log:
2011-02-01  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/47541
    * tree-ssa-structalias.c (push_fields_onto_fieldstack): Make
    sure to have a field at offset zero.

    * g++.dg/torture/pr47541.C: New testcase.

Added:
    trunk/gcc/testsuite/g++.dg/torture/pr47541.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-structalias.c

[Bug c++/47541] [4.5 Regression] For integer pointers, the value of ++*p is not written back to memory

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.4.4, 4.6.0
            Summary|[4.5/4.6 Regression] For    |[4.5 Regression] For
                   |integer pointers, the value |integer pointers, the value
                   |of ++*p  is not written     |of ++*p  is not written
                   |back to memory              |back to memory
      Known to fail|                            |4.5.2

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-02-01 11:15:50 UTC ---
Fixed on the trunk so far.

[Bug c++/47541] [4.5 Regression] For integer pointers, the value of ++*p is not written back to memory

[rguenth at gcc dot gnu.org]

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

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-02-01 11:27:07 UTC ---
Author: rguenth
Date: Tue Feb  1 11:27:04 2011
New Revision: 169472

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169472
Log:
2011-02-01  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/47541
    * tree-ssa-structalias.c (push_fields_onto_fieldstack): Make
    sure to have a field at offset zero.

    * g++.dg/torture/pr47541.C: New testcase.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/torture/pr47541.C
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_5-branch/gcc/tree-ssa-structalias.c

[Bug c++/47541] [4.5 Regression] For integer pointers, the value of ++*p is not written back to memory

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |4.5.3
         Resolution|                            |FIXED

--- Comment #14 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-02-01 11:29:26 UTC ---
Fixed.

[Bug c++/47541] [4.5 Regression] For integer pointers, the value of ++*p is not written back to memory

[dnovillo at gcc dot gnu.org]

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

--- Comment #15 from Diego Novillo <dnovillo at gcc dot gnu.org> 2011-02-02 18:14:44 UTC ---
Author: dnovillo
Date: Wed Feb  2 18:14:39 2011
New Revision: 169740

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169740
Log:
2011-02-01  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/47541
    * tree-ssa-structalias.c (push_fields_onto_fieldstack): Make
    sure to have a field at offset zero.

    * g++.dg/torture/pr47541.C: New testcase.

Added:
    branches/google/integration/gcc/testsuite/g++.dg/torture/pr47541.C
Modified:
    branches/google/integration/gcc/ChangeLog
    branches/google/integration/gcc/testsuite/ChangeLog
    branches/google/integration/gcc/tree-ssa-structalias.c