[Bug tree-optimization/30252] New: [4.2/4.3 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[Bug tree-optimization/30252] New: [4.2/4.3 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[belyshev at depni dot sinp dot msu dot ru]

gcc miscompiles this testcase (reduced from rtorrent) since r111639, compile
with -O1 -fstrict-aliasing:

#include <sigc++/bind.h>
#include <sigc++/slot.h>

static long dummy;

struct A
{
  static void *foo (void *p) { return p; }
  typedef sigc::slot <void *> C;
  C bar();
};

A::C A::bar ()
{
  return sigc::bind (sigc::ptr_fun (&A::foo), &dummy);
}

int main (void)
{
  A a;
  if (a.bar ()() != &dummy)
    abort ();
  return 0;
}


The program will crash in operator() because we store garbage instead of
function pointer inside A::bar():

--- O1  2006-12-18 18:44:34.000000000 +0300
+++ O1-fstrict-aliasing 2006-12-18 18:44:41.000000000 +0300
@@ -248,8 +248,8 @@
        movq    sigc::internal::typed_slot_rep<sigc::bind_functor<-1,
sigc::pointer_functor1<void*, void*>, long*, sigc::nil, sigc::nil, sigc::nil,
sigc::nil, sigc::nil, sigc::nil> >::dup(void*), 24(%rbx)
        movq    $0, 32(%rbx)
        movq    $0, 40(%rbx)
+       movq    %rbx, 64(%rbx)
        movq    $dummy, 72(%rbx)
-       movq    A::foo(void*), 64(%rbx)
        movq    %rbx, (%rsp)
        leaq    48(%rbx), %rsi
        movq    %rsp, %rdi

I will attach preprocessed and somewhat reduced testcase in a moment.

Caused by this patch:

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog       (revision 111638)
+++ gcc/ChangeLog       (revision 111639)
@@ -1,3 +1,8 @@
+2006-03-02  Richard Guenther  <rguenther@suse.de>
+
+       * tree-ssa-alias.c (find_used_portions): Consider taking
+       the address as making the variable not write-only.
+
 2006-03-02  Nick Clifton  <nickc@redhat.com>

        * config.gcc (default_use_cxa_atexit): Extend the description of
Index: gcc/tree-ssa-alias.c
===================================================================
--- gcc/tree-ssa-alias.c        (revision 111638)
+++ gcc/tree-ssa-alias.c        (revision 111639)
@@ -3071,6 +3071,8 @@ find_used_portions (tree *tp, int *walk_
            up->minused = 0;
            up->maxused = TREE_INT_CST_LOW (DECL_SIZE (var));
            up->implicit_uses = true;
+           if (!lhs_p)
+             up->write_only = false;

            up_insert (uid, up);
            *walk_subtrees = 0;


-- 
           Summary: [4.2/4.3 regression] miscompilation of sigc++-2.0 based
                    code with -fstrict-aliasing
           Product: gcc
           Version: 4.2.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: belyshev at depni dot sinp dot msu dot ru


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

[Bug tree-optimization/30252] [4.2/4.3 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[belyshev at depni dot sinp dot msu dot ru]

------- Comment #1 from belyshev at depni dot sinp dot msu dot ru  2006-12-18 15:57 -------
Created an attachment (id=12825)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12825&action=view)
preprocessed and somewhat reduced testcase


-- 


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

[Bug tree-optimization/30252] [4.2/4.3 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |blocker
   Target Milestone|---                         |4.2.0


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

[Bug tree-optimization/30252] [4.2/4.3 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[pinskia at gcc dot gnu dot org]

------- Comment #2 from pinskia at gcc dot gnu dot org  2006-12-18 19:04 -------
Hmm,
        typedef typed_slot_rep<T_functor> typed_slot;
        typed_slot *typed_rep = static_cast<typed_slot*>(rep);
        return (typed_rep->functor_)();

This code could violate C++ aliasing rules.


-- 


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[belyshev at depni dot sinp dot msu dot ru]

------- Comment #3 from belyshev at depni dot sinp dot msu dot ru  2006-12-19 11:11 -------
This does not fail on mainline since mem-ssa merge (r119760).


-- 

belyshev at depni dot sinp dot msu dot ru changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.3.0
            Summary|[4.2/4.3 regression]        |[4.2 regression]
                   |miscompilation of sigc++-2.0|miscompilation of sigc++-2.0
                   |based code with -fstrict-   |based code with -fstrict-
                   |aliasing                    |aliasing


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2006-12-27 16:18 -------
The patch referenced only could have uncovered a latent problem.  mem-ssa might
simply have hidden it again.

The reduced testcase doesn't link for me btw:

/tmp/ccGXFf9y.o: In function `A::bar()':
589.ii:(.text+0x2c): undefined reference to
`sigc::slot_base::slot_base(sigc::internal::slot_rep*)'
collect2: ld returned 1 exit status


-- 


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[belyshev at depni dot sinp dot msu dot ru]

------- Comment #5 from belyshev at depni dot sinp dot msu dot ru  2006-12-27 20:19 -------
(In reply to comment #4)
> The reduced testcase doesn't link for me btw:

sorry, forgot to mention: both tests require -lsigc-2.0


-- 


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[mmitchel at gcc dot gnu dot org]

------- Comment #6 from mmitchel at gcc dot gnu dot org  2007-02-19 20:52 -------
Before we can prioritize this problem, we need to establish that it doesn't
violate the aliasing rules.  As Andrew says in Comment #2, this looks likely to
be a problem in the input program.


-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[pinskia at gcc dot gnu dot org]

------- Comment #7 from pinskia at gcc dot gnu dot org  2007-04-29 08:01 -------
Can you try this again, I think this was related to PR 30567 which was just
fixed?


-- 


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[belyshev at depni dot sinp dot msu dot ru]

------- Comment #8 from belyshev at depni dot sinp dot msu dot ru  2007-04-29 09:58 -------
Created an attachment (id=13461)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13461&action=view)
self contained preprocessed testcase

(In reply to comment #7)
> Can you try this again, I think this was related to PR 30567 which was just
> fixed?

Tested r124272, bug is still there.

I'm attaching testcase which doesn't require -lsigc-2.0. Tested on i386, amd64,
alpha with same result.

Not sure about aliasing rules here. There is only two casts here, first one
(reinterpret_cast around line 19) looks innocent, the second (static_cast, line
222) is suspicious. Someone knowledgeable should re-examine it.


-- 

belyshev at depni dot sinp dot msu dot ru changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #12825|0                           |1
        is obsolete|                            |


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[bangerth at dealii dot org]

------- Comment #9 from bangerth at dealii dot org  2007-04-30 03:25 -------
I can't reproduce this with neither r124272 nor r124284. What flags exactly do
you use?

W.


-- 

bangerth at dealii dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bangerth at dealii dot org


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[pluto at agmk dot net]

------- Comment #10 from pluto at agmk dot net  2007-04-30 15:16 -------
i can reproduce testcase4 on 4.2.0-20070425@x86-64.

$ g++ 30252.cpp -O1 -fstrict-aliasing -g3 && ./a.out
zsh: segmentation fault  ./a.out

(gdb) bt
#0  0x00002adee263dbc0 in _rtld_local_ro () from /lib64/ld-linux-x86-64.so.2
#1  0x0000000000400654 in sigc::internal::slot_call0<sigc::bind_functor<-1,
sigc::pointer_functor1<void*, void*>, long*> >::call_it (
    rep=0x601010) at 30252.cpp:73
#2  0x000000000040062b in main () at 30252.cpp:238

it works with -O1 -fno-strict-aliasing.


-- 


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[pluto at agmk dot net]

------- Comment #11 from pluto at agmk dot net  2007-04-30 15:29 -------
--- 30252.cpp.099t.optimized.aliasing-OFF
+++ 30252.cpp.099t.optimized.aliasing-ON

 sigc::slot0 A::bar() (this)
 {
(...)
+  void * (*<T5b7>) (void *) SR.114;
(...)
+  this->functor_.D.2915.functor_.functor_.func_ptr_ = SR.114;
   this->functor_.bound1_.visited_ = &dummy;
-  this->functor_.D.2915.functor_.functor_.func_ptr_ = foo;
(...)

with strict aliasing the func_ptr_ is initialized with uninitialized
pointer (SR.114) instead of `foo'.


-- 


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[bangerth at dealii dot org]

------- Comment #12 from bangerth at dealii dot org  2007-04-30 15:42 -------
(In reply to comment #10)
> i can reproduce testcase4 on 4.2.0-20070425@x86-64.

I still can't reproduce on i686, sorry. Someone else will
have to look at this and try to further reduce the sources...

W.


-- 


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[giovannibajo at libero dot it]

------- Comment #13 from giovannibajo at libero dot it  2007-05-01 02:11 -------
(In reply to comment #2)

> Hmm,
>         typedef typed_slot_rep<T_functor> typed_slot;
>         typed_slot *typed_rep = static_cast<typed_slot*>(rep);
>         return (typed_rep->functor_)();
> 
> This code could violate C++ aliasing rules.

But:
    template <class T_functor>
    struct typed_slot_rep : public slot_rep

so it looks like it might be valid.


-- 


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[bangerth at math dot tamu dot edu]

------- Comment #14 from bangerth at math dot tamu dot edu  2007-05-01 02:39 -------
Subject: Re:  [4.2 regression] miscompilation
 of sigc++-2.0 based code with -fstrict-aliasing


> >         typedef typed_slot_rep<T_functor> typed_slot;
> >         typed_slot *typed_rep = static_cast<typed_slot*>(rep);
> >         return (typed_rep->functor_)();
> >
> > This code could violate C++ aliasing rules.
>
> But:
>     template <class T_functor>
>     struct typed_slot_rep : public slot_rep
>
> so it looks like it might be valid.

But only, of course, if the pointer actually points to a typed_slot_rep.
Maybe someone who can reproduce the bug can make the following test:
- add a virtual destructor to slot_rep
- change the static_cast to a dynamic_cast
If the object pointed to is not a typed_slot_rep (in which the static_cast
in the original code would have been in error), then the code should
probably segfault or so...

W.

-------------------------------------------------------------------------
Wolfgang Bangerth                email:            bangerth@math.tamu.edu
                                 www: http://www.math.tamu.edu/~bangerth/


-- 


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[pluto at agmk dot net]

------- Comment #15 from pluto at agmk dot net  2007-05-01 08:58 -------
(In reply to comment #14)

typed_rep points to:
N4sigc8internal14typed_slot_repINS_12bind_functorILin1ENS_16pointer_functor1IPvS4_EEPlEEEE


-- 


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #16 from rguenth at gcc dot gnu dot org  2007-05-01 13:35 -------
Created an attachment (id=13468)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13468&action=view)
slightly simplified testcase

I don't see anything wrong with the testcase.  I changed it to not take the
address of dummy, but pass zero instead.  This reduces the diff of initial
aliasing to

-  #   _A_b1_61 = V_MAY_DEF <_A_b1_54>;
-  #   SFT.70_62 = V_MAY_DEF <SFT.70_55>;
-  #   SFT.71_63 = V_MAY_DEF <SFT.71_56>;
-  #   SFT.72_64 = V_MAY_DEF <SFT.72_57>;
-  #   SFT.73_65 = V_MAY_DEF <SFT.73_58>;
-  #   NONLOCAL.79_66 = V_MAY_DEF <NONLOCAL.79_60>;
+  #   SFT.79_61 = V_MAY_DEF <SFT.79_57>;
   this_41->rep_ = rep_42;

but before DCE (which makes a mess out of this testcase) we have practically
indentical dumps for A::bar() - Note that A::bar() is the offending function
that gets mis-optimized.  We DCE the initialization of the function pointer so
we segfault at the indirect call to it.

@@ -78,21 +78,21 @@

 sigc::slot0 A::bar() (this)
 {
...
@@ -129,7 +129,7 @@
   _A_func_2 = foo;
   this_3 = (struct functor_base *) &D.2378;
   this_4 = this_3;
-  #   SFT.73_6 = V_MUST_DEF <SFT.73_5>;
+  #   SFT.80_6 = V_MUST_DEF <SFT.80_5>;
   D.2378.func_ptr_ = foo;
   _A_functor_7 = &D.2378;
   #   _A_b1_9 = V_MUST_DEF <_A_b1_8>;
@@ -151,33 +151,33 @@
   this_24 = (struct functor_base *) this_23;
   this_25 = this_24;
   #   D.2951_27 = V_MUST_DEF <D.2951_26>;
-  #   VUSE <SFT.73_6>;
+  #   VUSE <SFT.80_6>;
   D.2951 = D.2378;
-  #   SFT.70_50 = V_MAY_DEF <SFT.70_49>;
+  #   SFT.77_50 = V_MAY_DEF <SFT.77_49>;
   #   VUSE <D.2951_27>;
   this_20->functor_ = D.2951;
   this_28 = &D.2514.bound1_;
   this_29 = &D.2514.bound1_;
   _A_argument_30 = &_A_b1;
   D.2953_31 = 0B;
-  #   SFT.70_68 = V_MUST_DEF <SFT.70_50>;
+  #   SFT.77_63 = V_MUST_DEF <SFT.77_50>;
   D.2514.bound1_.visited_ = D.2953_31;
   functor_32 = &D.2514;
   #   _A_b1_54 = V_MAY_DEF <_A_b1_9>;
-  #   SFT.70_55 = V_MAY_DEF <SFT.70_68>;
-  #   SFT.71_56 = V_MAY_DEF <SFT.71_52>;
-  #   SFT.72_57 = V_MAY_DEF <SFT.72_45>;
-  #   SFT.73_58 = V_MAY_DEF <SFT.73_6>;
-  #   NONLOCAL.79_59 = V_MAY_DEF <NONLOCAL.79_53>;
-  D.3052_33 = operator new (20);
-  this_34 = (struct typed_slot_rep<sigc::bind_functor<-0x000000001,
sigc::pointer_functor1<void*, void*>, void*> > *) D.3052_33;
+  #   SFT.77_55 = V_MAY_DEF <SFT.77_63>;
+  #   SFT.78_56 = V_MAY_DEF <SFT.78_52>;
+  #   SFT.79_57 = V_MAY_DEF <SFT.79_45>;
+  #   SFT.80_58 = V_MAY_DEF <SFT.80_6>;
+  #   NONLOCAL.86_59 = V_MAY_DEF <NONLOCAL.86_53>;
+  D.3059_33 = operator new (20);
+  this_34 = (struct typed_slot_rep<sigc::bind_functor<-0x000000001,
sigc::pointer_functor1<void*, void*>, void*> > *) D.3059_33;
   this_35 = this_34;
   functor_36 = &D.2514;
   this_37 = &this_35->D.2649;
   this_38 = this_37;
-  #   NONLOCAL.79_60 = V_MAY_DEF <NONLOCAL.79_59>;
-  #   VUSE <SFT.70_55>;
-  #   VUSE <SFT.71_56>;
+  #   NONLOCAL.86_60 = V_MAY_DEF <NONLOCAL.86_59>;
+  #   VUSE <SFT.77_55>;
+  #   VUSE <SFT.78_56>;
   this_35->functor_ = D.2514;
   rep_39 = (struct slot_rep *) this_34;
   this_40 = &<retval>.D.2330;
@@ -185,14 +185,14 @@
   rep_42 = rep_39;
   this_43 = (struct functor_base *) &<retval>.D.2330;
   this_44 = this_43;
-  #   SFT.72_69 = V_MUST_DEF <SFT.72_57>;
+  #   SFT.79_64 = V_MUST_DEF <SFT.79_57>;
   <retval>.D.2330.rep_ = rep_42;
-  D.3058_46 = rep_39;
-  D.3060_47 = call_it;
-  D.3060_48 = call_it;
-  #   NONLOCAL.79_67 = V_MAY_DEF <NONLOCAL.79_60>;
-  D.3058_46->call_ = call_it;
-  #   VUSE <SFT.72_69>;
+  D.3065_46 = rep_39;
+  D.3067_47 = call_it;
+  D.3067_48 = call_it;
+  #   NONLOCAL.86_62 = V_MAY_DEF <NONLOCAL.86_60>;
+  D.3065_46->call_ = call_it;
+  #   VUSE <SFT.79_64>;
   return <retval>;

 }

Still the DCE diff contains

 <bb 2>:
-  #   SFT.73_6 = V_MUST_DEF <SFT.73_5>;
+  #   SFT.80_6 = V_MUST_DEF <SFT.80_5>;
   D.2378.func_ptr_ = foo;
   #   _A_b1_9 = V_MUST_DEF <_A_b1_8>;
   _A_b1 = 0B;
-  this_19 = &D.2514.D.2511.functor_;
-  this_20 = this_19;
-  #   D.2951_27 = V_MUST_DEF <D.2951_26>;
-  #   VUSE <SFT.73_6>;
-  D.2951 = D.2378;
-  #   SFT.70_50 = V_MAY_DEF <SFT.70_49>;
-  #   VUSE <D.2951_27>;
-  this_20->functor_ = D.2951;
   D.2953_31 = 0B;
-  #   SFT.70_68 = V_MUST_DEF <SFT.70_50>;
+  #   SFT.77_63 = V_MUST_DEF <SFT.77_49>;
   D.2514.bound1_.visited_ = D.2953_31;

where we removed the assignment

  D.2951 = D.2378;


-- 


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

[Bug tree-optimization/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #17 from rguenth at gcc dot gnu dot org  2007-05-01 14:00 -------
Without -fstrict-aliasing we mark this_20->functor_ = D.2946; obviously
necessary
because it is is_hidden_global_store ():

 ;; Function sigc::slot0 A::bar() (_ZN1A3barEv)

-Marking useful stmt: this_20->functor_ = D.2946;
-
-Marking useful stmt: D.3047_33 = operator new (20);
+Marking useful stmt: D.3054_33 = operator new (20);

 Marking useful stmt: this_35->functor_ = D.2514;

-Marking useful stmt: D.3053_46->call_ = call_it;
+Marking useful stmt: D.3060_46->call_ = call_it;

 Marking useful stmt: return <retval>;

without that we DCE too much as

  #   SFT.80_6 = V_MUST_DEF <SFT.80_5>;
  D.2378.func_ptr_ = foo;
...
  #   D.2951_27 = V_MUST_DEF <D.2951_26>;
  #   VUSE <SFT.80_6>;
  D.2951 = D.2378;
  #   SFT.77_50 = V_MAY_DEF <SFT.77_49>;
  #   VUSE <D.2951_27>;
  this_20->functor_ = D.2951;
  #   SFT.77_63 = V_MUST_DEF <SFT.77_50>;
  D.2514.bound1_.visited_ = D.2953_31;

the last store kills the assignment to this_20->functor_.  Note that while
we have uses for D.2951_27 and SFT.80_6 the structure assignment to
this_20->functor_ for some reason shares the SFT with the assignment
to D.2514.bound1_.visited_.  So this may be as well a frontend bug.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |alias
   Last reconfirmed|0000-00-00 00:00:00         |2007-05-01 14:00:47
               date|                            |


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #18 from rguenth at gcc dot gnu dot org  2007-05-01 14:05 -------
So my guess is this is related to PR22488 which is rotting since some time...


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at redhat dot com
  BugsThisDependsOn|                            |22488
          Component|tree-optimization           |c++


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[mmitchel at gcc dot gnu dot org]

------- Comment #19 from mmitchel at gcc dot gnu dot org  2007-05-02 21:59 -------
If this is a valid program, as we seem to think it is, then this is a serious
bug.  However, if it's the front-end bug that Richard suspects, there's not
much we can do about it in the near term.  The ball is firmly in Jason's court.


-- 

mmitchel at gcc dot gnu dot org changed:

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


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[ismail at pardus dot org dot tr]

------- Comment #22 from ismail at pardus dot org dot tr  2007-05-13 01:43 -------
This problem also breaks inkscape, it segfaults on startup when compiled with
gcc 4.2.0


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #23 from rguenth at gcc dot gnu dot org  2007-05-13 11:34 -------
There are no overlapping fields created from push_fields_onto_fieldstack, so my
pointing to PR22488 may be wrong.


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #24 from rguenth at gcc dot gnu dot org  2007-05-13 13:02 -------
That leaves aliasing and DCE.

The following statement connects to the initializing of the function pointer.

  #   SFT.78D.2952_55 = V_MAY_DEF <SFT.78D.2952_31>;
  #   VUSE <D.2838_29>;
  thisD.2828_22->functor_D.2399 = D.2838;

but,

  D.2839_30 = 0B;
  #   SFT.78D.2952_32 = V_MUST_DEF <SFT.78D.2952_55>;
  D.2831.bound1_D.2423 = D.2839_30;

kills it.  Still they look like they are accessing different fields
(propagating pointers for the first expression):

  #   SFT.78D.2952_55 = V_MAY_DEF <SFT.78D.2952_31>;
  #   VUSE <D.2838_29>;
  D.2831.D.2432.functor_D.2382.functor_D.2399 = D.2838;

so my hint at a frontend problem with type-layout.  Scheduling some more
early cleanup passes before the first DCE shows this (and fixes the
segfault!?):

  #   SFT.79D.2951_66 = V_MUST_DEF <SFT.79D.2951_49>;
  #   VUSE <D.2836_27>;
  D.2435.D.2432.functor_D.2382.functor_D.2399 = D.2836;
  D.2837_28 = 0B;
  #   SFT.78D.2950_30 = V_MUST_DEF <SFT.78D.2950_29>;
  D.2435.bound1_D.2423 = D.2837_28;

note how we no longer thing the second store kills the first one.  Which
makes it look like an aliasing problem again.  Scheduling another may_alias
pass before the first DCE also fixes the problem, as we then have

  #   SFT.88D.2960_48 = V_MAY_DEF <SFT.88D.2960_29>;
  #   SFT.89D.2961_61 = V_MAY_DEF <SFT.89D.2961_49>;
  #   VUSE <D.2836_27>;
  thisD.2827_20->functor_D.2399 = D.2836;
  D.2837_28 = 0B;
  #   SFT.88D.2960_30 = V_MUST_DEF <SFT.88D.2960_48>;
  D.2435.bound1_D.2423 = D.2837_28;

aliasing is fixed as long as the extra may_alias pass is after CCP, but that
CCP pass doesn't do too much interesting stuff to the above stmts:

   #   SFT.88D.2960_48 = V_MAY_DEF <SFT.88D.2960_29>;
   #   VUSE <D.2836_27>;
   thisD.2827_20->functor_D.2399 = D.2836;
   #   VUSE <_A_b1D.2819_9>;
-  D.2837_28 = *_A_bound1D.2820_11;
+  D.2837_28 = _A_b1D.2819;
   #   SFT.88D.2960_30 = V_MUST_DEF <SFT.88D.2960_48>;
   D.2435.bound1_D.2423 = D.2837_28;

and if you diff a may_alias pass before and after CCP you get

@@ -487,20 +489,20 @@
 _A_bound1_11 = { _A_b1 }
 this_12 = { D.2435 D.2435.bound1_ }
 D.2435 = { }
-D.2435.bound1_ = { NULL }
+D.2435.bound1_ = { NULL foo }
 this_13 = { D.2435 D.2435.bound1_ }
 _A_functor_14 = { D.2328 }
 this_15 = { D.2435 D.2435.bound1_ }
 this_16 = { D.2435 D.2435.bound1_ }
 this_17 = { D.2435 D.2435.bound1_ }
 this_18 = { D.2435 D.2435.bound1_ }
-this_19 = { D.2435.bound1_ }
-this_20 = { D.2435.bound1_ }
+this_19 = { D.2435 D.2435.bound1_ }
+this_20 = { D.2435 D.2435.bound1_ }
 _A_functor_21 = { D.2328 }
-this_22 = { D.2435.bound1_ }
-this_23 = { D.2435.bound1_ }
-this_24 = { D.2435.bound1_ }
-this_25 = { D.2435.bound1_ }
+this_22 = { D.2435 D.2435.bound1_ }
+this_23 = { D.2435 D.2435.bound1_ }
+this_24 = { D.2435 D.2435.bound1_ }
+this_25 = { D.2435 D.2435.bound1_ }
 D.2836 = { foo }
 D.2837_28 = { NULL }
 functor_31 = { D.2435 D.2435.bound1_ }
@@ -510,8 +512,6 @@
 functor_35 = { D.2435 D.2435.bound1_ }
 this_36 = { NULL NONLOCAL.105 foo call_it }
 this_37 = { NULL NONLOCAL.105 foo call_it }
-structcopydereftmp.127 = { NULL }
-structcopydereftmp.127.bound1_ = { NULL }
 rep_38 = { NULL NONLOCAL.105 foo call_it }
 this_39 = { D.2301 }
 D.2301 = { NULL NONLOCAL.105 foo call_it }
@@ -524,7 +524,7 @@
 call_it = { NULL NONLOCAL.105 foo call_it }
 D.2967_47 = { call_it }

which should be the most interesting parts of the diff (apart from the
extra vops that prevent the DCE).

Danny?  Any hints on what can go wrong here?


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dberlin at gcc dot gnu dot
                   |                            |org


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[dberlin at gcc dot gnu dot org]

------- Comment #25 from dberlin at gcc dot gnu dot org  2007-05-14 20:01 -------

> which should be the most interesting parts of the diff (apart from the
> extra vops that prevent the DCE).
> 
> Danny?  Any hints on what can go wrong here?
> 

The first pass is obviously generating wrong answers, unless someone changed
the subvars.  
It looks like it's getting confused about where to use the whole object or the
first field.

Is that a diff of the dump that includes -details?

I'm trying to figure out if the constraints are bad, or the solving is bad.


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[mmitchel at gcc dot gnu dot org]

------- Comment #26 from mmitchel at gcc dot gnu dot org  2007-05-14 22:25 -------
Will not be fixed in 4.2.0; retargeting at 4.2.1.


-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.2.0                       |4.2.1


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #27 from rguenth at gcc dot gnu dot org  2007-05-15 08:59 -------
Yes, that's a (part of!) a diff of a -details dump with the following patch

Index: passes.c
===================================================================
--- passes.c    (revision 124501)
+++ passes.c    (working copy)
@@ -496,7 +496,9 @@ init_optimization_passes (void)
   NEXT_PASS (pass_early_warn_uninitialized);

   /* Initial scalar cleanups.  */
+  NEXT_PASS (pass_may_alias);
   NEXT_PASS (pass_ccp);
+  NEXT_PASS (pass_may_alias);
   NEXT_PASS (pass_fre);
   NEXT_PASS (pass_dce);
   NEXT_PASS (pass_forwprop);

(so it's easy to reproduce the full dump)


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #28 from rguenth at gcc dot gnu dot org  2007-06-04 15:28 -------
solution_set_add () looks like the culprit (thx micha).  So, the following will
fix it in case we have offsets only from COMPONENT_REFs, not from regular
pointer arithmetic (which is not true):

Index: tree-ssa-structalias.c
===================================================================
--- tree-ssa-structalias.c      (revision 125310)
+++ tree-ssa-structalias.c      (working copy)
@@ -715,13 +715,10 @@ solution_set_add (bitmap set, unsigned H
         less than end.  Otherwise, it is globbed to a single
         variable.  */

-      if ((get_varinfo (i)->offset + offset) < get_varinfo (i)->fullsize)
+      if (get_varinfo (i)->offset + get_varinfo (i)->size - 1 >= offset
+         && get_varinfo (i)->offset + offset < get_varinfo (i)->fullsize)
        {
-         unsigned HOST_WIDE_INT fieldoffset = get_varinfo (i)->offset +
offset;
-         varinfo_t v = first_vi_for_offset (get_varinfo (i), fieldoffset);
-         if (!v)
-           continue;
-         bitmap_set_bit (result, v->id);
+         bitmap_set_bit (result, i);
        }
       else if (get_varinfo (i)->is_artificial_var
               || get_varinfo (i)->has_union


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #29 from rguenth at gcc dot gnu dot org  2007-06-04 15:45 -------
We can make it safe if we only really handle + in pointer arithmetic.  Like
with

@@ -3258,7 +3255,7 @@ handle_ptr_arith (VEC (ce_s, heap) *lhsc
   unsigned int i = 0;
   unsigned int j = 0;
   VEC (ce_s, heap) *temp = NULL;
-  unsigned int rhsoffset = 0;
+  unsigned HOST_WIDE_INT rhsoffset = 0;

   if (TREE_CODE (expr) != PLUS_EXPR
       && TREE_CODE (expr) != MINUS_EXPR)
@@ -3269,9 +3266,12 @@ handle_ptr_arith (VEC (ce_s, heap) *lhsc

   get_constraint_for (op0, &temp);
   if (POINTER_TYPE_P (TREE_TYPE (op0))
-      && TREE_CODE (op1) == INTEGER_CST
+      && host_integerp (op1, 1)
       && TREE_CODE (expr) == PLUS_EXPR)
     {
+      if ((TREE_INT_CST_LOW (op1) * BITS_PER_UNIT) / BITS_PER_UNIT
+         != TREE_INT_CST_LOW (op1))
+       return false;
       rhsoffset = TREE_INT_CST_LOW (op1) * BITS_PER_UNIT;
     }
   else


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #30 from rguenth at gcc dot gnu dot org  2007-06-05 09:27 -------
See this testcase (reduced from spec2k6 dealII by Micha):

  struct I {
    int ix;
    struct II {
      int j,i;
      void *ptr;
    } ii;
  };// inner;
struct O : public I {
//  int x;
  int y;
};

static int geti (struct O *o)
{
  return o->ii.i;
}
extern void init(struct O*);
extern void deinit(struct O*);
extern int getcond(void);
struct O::I *getinner (struct O*);
int reset (int cond)
{

  struct O cell;
  struct O *o;
  struct I::II *in;
  int *pi;
  o = &cell;
  init (o);
  in = &o->ii;
  while (getcond()) {
    if (o->y)
      cell.ii.i = -2;
    if (!getcond()) {
      in->i = -1;
      in->j = o->ii.i;
      break;
    }
  }
  pi = &in->j;
  pi++;
  deinit(&cell);
  return *pi;
}

and note how we compute the wrong points-to solution for in:

o_3 = { cell cell.y cell.ptr cell.i cell.j }
in_4 = { cell.y cell.ptr cell.j }

we are missing cell.i in the points-to set.  This is because with looking
simply for vars with oldoffset + offset is appearantly not enough here.

  cell + 64    ->  cell.j
  cell.y + 64  ->  off structure
  cell.ptr + 64 -> cell.y
  cell.i + 64  ->  cell.ptr (actually in the middle of it)
  cell.j + 64  ->  cell.ptr

so, there is nothing in the structure that, if added 64, will point to
cell.i.  But of course we are taking the address of the substructure and
so we are indeed pointing to cell.i.

So a different solution would be to add _each_ var starting with the
first one + offset.  Which would be simply "reversing" the logic like
I did with the patch (it needs some fixing still, as pruning at the
end is bogus in it as vars may be still reached from others).

The thing that fixes this (30252) bug is that with the original patch we
keep the parent var in the solution.  With something like

Index: tree-ssa-structalias.c
===================================================================
--- tree-ssa-structalias.c      (revision 125310)
+++ tree-ssa-structalias.c      (working copy)
@@ -708,6 +708,21 @@ solution_set_add (bitmap set, unsigned H
   bitmap result = BITMAP_ALLOC (&iteration_obstack);
   unsigned int i;
   bitmap_iterator bi;
+  unsigned HOST_WIDE_INT min = -1, max = 0;
+
+  /* Compute set of vars we can reach from set + offset.  */
+
+  EXECUTE_IF_SET_IN_BITMAP (set, 0, i, bi)
+    {
+      if (get_varinfo (i)->offset + offset < min)
+       min = get_varinfo (i)->offset + offset;
+      if (get_varinfo (i)->offset + get_varinfo (i)->size + offset > max)
+       {
+         max = get_varinfo (i)->offset + get_varinfo (i)->size + offset;
+         if (max > get_varinfo (i)->fullsize)
+           max = get_varinfo (i)->fullsize;
+       }
+    }

   EXECUTE_IF_SET_IN_BITMAP (set, 0, i, bi)
     {
@@ -715,13 +730,10 @@ solution_set_add (bitmap set, unsigned H
         less than end.  Otherwise, it is globbed to a single
         variable.  */

-      if ((get_varinfo (i)->offset + offset) < get_varinfo (i)->fullsize)
+      if (get_varinfo (i)->offset >= min
+         && get_varinfo (i)->offset < max)
        {
-         unsigned HOST_WIDE_INT fieldoffset = get_varinfo (i)->offset +
offset;
-         varinfo_t v = first_vi_for_offset (get_varinfo (i), fieldoffset);
-         if (!v)
-           continue;
-         bitmap_set_bit (result, v->id);
+         bitmap_set_bit (result, i);
        }
       else if (get_varinfo (i)->is_artificial_var
               || get_varinfo (i)->has_union

we don't retain it and this bug is not fixed, but the SPEC bug should be.


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #31 from rguenth at gcc dot gnu dot org  2007-06-05 12:05 -------
The problem in this PR is, that the structure we mess up points-to info has no
fields in its lower half, but only two varinfos (D.2860, offset 128 size 64
and D.2860.visited_, offset 192 size 64 -- the first one is actually
D.2860.func_ptr_, but we name the first after the decl always).  Now, there
_is_
a structure at offset zero, D.2857 (size 192), which contains D.2860.func_ptr_.

So we fail to have a varinfo for the offset the expression &D.2860.D.2857
points
to and we loose the first varinfo wrongly during the &this_18->functor_
addition of 64.

Now, either we need to generate fake varinfos for the 'holes' we end up with,
or just punt here.


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #32 from rguenth at gcc dot gnu dot org  2007-06-05 15:01 -------
Created an attachment (id=13657)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13657&action=view)
patch

Patch in testing.  It makes sure to create fields for all addressable
components
(such as empty bases) and adjusts solution_set_add to only "remove" no longer
necessary vars (iff we ever would need to add vars, we would be screwed
anyway).

Basically all pointer arithmetic (which includes COMPONENT_REFs) handling looks
_very_ fragile in the current setup.


-- 

rguenth at gcc dot gnu dot org changed:

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


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #33 from rguenth at gcc dot gnu dot org  2007-06-05 15:57 -------
Actually,

+  if (minoffset != 0)

must be changed to

+  if (minoffset != 0 && count != 0)


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[dberlin at dberlin dot org]

------- Comment #34 from dberlin at gcc dot gnu dot org  2007-06-05 16:20 -------
Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0 based code with
-fstrict-aliasing

On 5 Jun 2007 09:27:34 -0000, rguenth at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #30 from rguenth at gcc dot gnu dot org  2007-06-05 09:27 -------
> See this testcase (reduced from spec2k6 dealII by Micha):
>
>   struct I {
>     int ix;
>     struct II {
>       int j,i;
>       void *ptr;
>     } ii;
>   };// inner;
> struct O : public I {
> //  int x;
>   int y;
> };
>
> static int geti (struct O *o)
> {
>   return o->ii.i;
> }
> extern void init(struct O*);
> extern void deinit(struct O*);
> extern int getcond(void);
> struct O::I *getinner (struct O*);
> int reset (int cond)
> {
>
>   struct O cell;
>   struct O *o;
>   struct I::II *in;
>   int *pi;
>   o = &cell;
>   init (o);
>   in = &o->ii;
>   while (getcond()) {
>     if (o->y)
>       cell.ii.i = -2;
>     if (!getcond()) {
>       in->i = -1;
>       in->j = o->ii.i;
>       break;
>     }
>   }
>   pi = &in->j;
>   pi++;
>   deinit(&cell);
>   return *pi;
> }
>
> and note how we compute the wrong points-to solution for in:
>
> o_3 = { cell cell.y cell.ptr cell.i cell.j }
> in_4 = { cell.y cell.ptr cell.j }
>

> we are missing cell.i in the points-to set.  This is because with looking
> simply for vars with oldoffset + offset is appearantly not enough here.
>
>   cell + 64    ->  cell.j
>   cell.y + 64  ->  off structure
>   cell.ptr + 64 -> cell.y
>   cell.i + 64  ->  cell.ptr (actually in the middle of it)
>   cell.j + 64  ->  cell.ptr
>
> so, there is nothing in the structure that, if added 64, will point to
> cell.i.  But of course we are taking the address of the substructure and
> so we are indeed pointing to cell.i.
We are pointing to cell, and using that to access cell.i
Not quite the same thing.
But I guess in our world we have to make them the same thing.

> So a different solution would be to add _each_ var starting with the
> first one + offset.

You don't want to play in solution_set_add, which is common to a bunch of code.

The only cases where the above problem comes into play is in offseted
copy constraints, line 1520 (IE the last part of
do_complex_constraint).

In actuality, this is a bug in constraint building.

When we generate the constraints for in = &o->ii, we should be adding
an offsetted copy constraint for each offset of type *o.

This will fix the problem.


> I did with the patch (it needs some fixing still, as pruning at the
> end is bogus in it as vars may be still reached from others).
>

You can never get back to the parent structure from a pointer to the
substructure.
I asked this explicitly and was told it was undefined behavior.


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenther at suse dot de]

------- Comment #35 from rguenther at suse dot de  2007-06-05 16:30 -------
Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0
 based code with -fstrict-aliasing

On Tue, 5 Jun 2007, dberlin at dberlin dot org wrote:

> Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0 based code with
> -fstrict-aliasing
> 
> On 5 Jun 2007 09:27:34 -0000, rguenth at gcc dot gnu dot org
> <gcc-bugzilla@gcc.gnu.org> wrote:
> >
> >
> > ------- Comment #30 from rguenth at gcc dot gnu dot org  2007-06-05 09:27 -------
> > See this testcase (reduced from spec2k6 dealII by Micha):
> >
> >   struct I {
> >     int ix;
> >     struct II {
> >       int j,i;
> >       void *ptr;
> >     } ii;
> >   };// inner;
> > struct O : public I {
> > //  int x;
> >   int y;
> > };
> >
> > static int geti (struct O *o)
> > {
> >   return o->ii.i;
> > }
> > extern void init(struct O*);
> > extern void deinit(struct O*);
> > extern int getcond(void);
> > struct O::I *getinner (struct O*);
> > int reset (int cond)
> > {
> >
> >   struct O cell;
> >   struct O *o;
> >   struct I::II *in;
> >   int *pi;
> >   o = &cell;
> >   init (o);
> >   in = &o->ii;
> >   while (getcond()) {
> >     if (o->y)
> >       cell.ii.i = -2;
> >     if (!getcond()) {
> >       in->i = -1;
> >       in->j = o->ii.i;
> >       break;
> >     }
> >   }
> >   pi = &in->j;
> >   pi++;
> >   deinit(&cell);
> >   return *pi;
> > }
> >
> > and note how we compute the wrong points-to solution for in:
> >
> > o_3 = { cell cell.y cell.ptr cell.i cell.j }
> > in_4 = { cell.y cell.ptr cell.j }
> >
> 
> > we are missing cell.i in the points-to set.  This is because with looking
> > simply for vars with oldoffset + offset is appearantly not enough here.
> >
> >   cell + 64    ->  cell.j
> >   cell.y + 64  ->  off structure
> >   cell.ptr + 64 -> cell.y
> >   cell.i + 64  ->  cell.ptr (actually in the middle of it)
> >   cell.j + 64  ->  cell.ptr
> >
> > so, there is nothing in the structure that, if added 64, will point to
> > cell.i.  But of course we are taking the address of the substructure and
> > so we are indeed pointing to cell.i.
> We are pointing to cell, and using that to access cell.i
> Not quite the same thing.
> But I guess in our world we have to make them the same thing.
> 
> > So a different solution would be to add _each_ var starting with the
> > first one + offset.
> 
> You don't want to play in solution_set_add, which is common to a bunch of code.
> 
> The only cases where the above problem comes into play is in offseted
> copy constraints, line 1520 (IE the last part of
> do_complex_constraint).
> 
> In actuality, this is a bug in constraint building.
> 
> When we generate the constraints for in = &o->ii, we should be adding
> an offsetted copy constraint for each offset of type *o.
> 
> This will fix the problem.

How so?  The solution_set_add method is still wrong.

> > I did with the patch (it needs some fixing still, as pruning at the
> > end is bogus in it as vars may be still reached from others).
> >
> 
> You can never get back to the parent structure from a pointer to the
> substructure.
> I asked this explicitly and was told it was undefined behavior.

Yep, this is what I convinced myself aswell.

Richard.


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[dberlin at dberlin dot org]

------- Comment #36 from dberlin at gcc dot gnu dot org  2007-06-05 17:45 -------
Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0 based code with
-fstrict-aliasing

On 5 Jun 2007 16:30:32 -0000, rguenther at suse dot de
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #35 from rguenther at suse dot de  2007-06-05 16:30 -------
> Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0
>  based code with -fstrict-aliasing
>
> On Tue, 5 Jun 2007, dberlin at dberlin dot org wrote:
>
> > Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0 based code with
> > -fstrict-aliasing
> >
> > On 5 Jun 2007 09:27:34 -0000, rguenth at gcc dot gnu dot org
> > <gcc-bugzilla@gcc.gnu.org> wrote:
> > >
> > >
> > > ------- Comment #30 from rguenth at gcc dot gnu dot org  2007-06-05 09:27 -------
> > > See this testcase (reduced from spec2k6 dealII by Micha):
> > >
> > >   struct I {
> > >     int ix;
> > >     struct II {
> > >       int j,i;
> > >       void *ptr;
> > >     } ii;
> > >   };// inner;
> > > struct O : public I {
> > > //  int x;
> > >   int y;
> > > };
> > >
> > > static int geti (struct O *o)
> > > {
> > >   return o->ii.i;
> > > }
> > > extern void init(struct O*);
> > > extern void deinit(struct O*);
> > > extern int getcond(void);
> > > struct O::I *getinner (struct O*);
> > > int reset (int cond)
> > > {
> > >
> > >   struct O cell;
> > >   struct O *o;
> > >   struct I::II *in;
> > >   int *pi;
> > >   o = &cell;
> > >   init (o);
> > >   in = &o->ii;
> > >   while (getcond()) {
> > >     if (o->y)
> > >       cell.ii.i = -2;
> > >     if (!getcond()) {
> > >       in->i = -1;
> > >       in->j = o->ii.i;
> > >       break;
> > >     }
> > >   }
> > >   pi = &in->j;
> > >   pi++;
> > >   deinit(&cell);
> > >   return *pi;
> > > }
> > >
> > > and note how we compute the wrong points-to solution for in:
> > >
> > > o_3 = { cell cell.y cell.ptr cell.i cell.j }
> > > in_4 = { cell.y cell.ptr cell.j }
> > >
> >
> > > we are missing cell.i in the points-to set.  This is because with looking
> > > simply for vars with oldoffset + offset is appearantly not enough here.
> > >
> > >   cell + 64    ->  cell.j
> > >   cell.y + 64  ->  off structure
> > >   cell.ptr + 64 -> cell.y
> > >   cell.i + 64  ->  cell.ptr (actually in the middle of it)
> > >   cell.j + 64  ->  cell.ptr
> > >
> > > so, there is nothing in the structure that, if added 64, will point to
> > > cell.i.  But of course we are taking the address of the substructure and
> > > so we are indeed pointing to cell.i.
> > We are pointing to cell, and using that to access cell.i
> > Not quite the same thing.
> > But I guess in our world we have to make them the same thing.
> >
> > > So a different solution would be to add _each_ var starting with the
> > > first one + offset.
> >
> > You don't want to play in solution_set_add, which is common to a bunch of code.
> >
> > The only cases where the above problem comes into play is in offseted
> > copy constraints, line 1520 (IE the last part of
> > do_complex_constraint).
> >
> > In actuality, this is a bug in constraint building.
> >
> > When we generate the constraints for in = &o->ii, we should be adding
> > an offsetted copy constraint for each offset of type *o.
> >
> > This will fix the problem.
>
> How so?  The solution_set_add method is still wrong.

The constraints say to add this offset to the members of the points to
sets of these variables, and put the result over here.
As long as solution_set_add is doing that, it is not the problem.

If it does not generate correct results, it is the constraints that
are wrong, not solution_set_add


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[matz at gcc dot gnu dot org]

------- Comment #37 from matz at gcc dot gnu dot org  2007-06-05 18:24 -------
> We are pointing to cell, and using that to access cell.i

No.  We are pointing to cell.ii, and use that pointer to access cell.ii.i
(via in->i).  Hence of course the points-to set of 'in' needs to include
cell.in.i (or cell.i, as the point-to debug dump likes to call it).  At least
how the points-to solver is used.  Theoretically it would of course be more
sensible to only note that 'in' can point to 'cell.ii', and make the user
of that information recognize that this actually is a structure, and hence
over that pointer you can access all members of it.  But that's not
how the info is used, so we have to include cell.ii.i in the points to set
(which in our case rather is a can-access set).

Now, regarding where to fix it properly: if you say that solution_set_add()
is not the right place (and I don't buy that yet, because something is wrong
with it no matter what, e.g. looking for subfields starting from the things
in the pt set, which themself might be subfields feels wrong), then we must
not use pointer arithmetic (or at least '+' constraints) to represent pointing
to substructures.  And I'm not sure that's easily doable.  Because the insn
we see is not "in = &cell.in", but "in = &o->in", and only the pt solver
itself sees that 'o' points to cell.  So what constraints do you suggest
should be added instead of offseted constraints for this insn?

> You don't want to play in solution_set_add, which is common to a bunch of 
> code.

Well, it's common to exactly the code which handles offsetted constraints.
And I don't see why the problem we face here (with offseted copy constraints)
doesn't also occur with any other of the offseted constraints, e.g.
"*x = y+32", handled in do_ds_constraint.

> > You can never get back to the parent structure from a pointer to the
> > substructure.
> > I asked this explicitly and was told it was undefined behavior.
> 
> Yep, this is what I convinced myself aswell.

Well, it's perhaps illegal in C (I'm not sure about that actually), but
certainly occurs in C++, and even if it didn't occur there we are talking
about the middle-end, for which such invalidity shouldn't matter (except we
choose to define it as invalid).  To see why this is valid, consider this
C++ program:

struct A {int a;};
struct B {int b;};
struct C : public A, B { int c;};
C* b2c (B* b) { return (C*)b; }
C* get_c (void)
{
  static C c;
  B *pb;
  C *pc;
  pb = &c;
  pc = (C*)pb;
  return pc;
}

This is completely valid (suppose b2c is only ever called with pointers
to the B-in-C base of any C), and results in this insn:

  b.2_3 = (struct C *) b_2(D);
  D.2496_4 = b.2_3 - 4;

A better example might be the other function 'get_c', which also has the
subtraction:

  pb_3 = (struct CD.2478 *) &cD.2495.D.2482;
  pc_4 = pb_3 - 4;
  return pc_4;

This currently only works because points-to gives up on subtraction and 
adds INTEGER to the access set of pc_4 (additionally to c.b and c.c; c.a
or c itself are missing).


-- 

matz at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |matz at gcc dot gnu dot org


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[dberlin at dberlin dot org]

------- Comment #38 from dberlin at gcc dot gnu dot org  2007-06-05 19:07 -------
Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0 based code with
-fstrict-aliasing

On 5 Jun 2007 18:24:54 -0000, matz at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #37 from matz at gcc dot gnu dot org  2007-06-05 18:24 -------
> > We are pointing to cell, and using that to access cell.i
>
> No.  We are pointing to cell.ii, and use that pointer to access cell.ii.i
> (via in->i).  Hence of course the points-to set of 'in' needs to include
> cell.in.i (or cell.i, as the point-to debug dump likes to call it).  At least
> how the points-to solver is used.
>  Theoretically it would of course be more
> sensible to only note that 'in' can point to 'cell.ii', and make the user
> of that information recognize that this actually is a structure, and hence
> over that pointer you can access all members of it.  But that's not
> how the info is used, so we have to include cell.ii.i in the points to set
> (which in our case rather is a can-access set).

Or we have to fix how it is used to handle these issues there.  :)

Honestly, doing the second would result in much much saner points-to
constraint generation and solving.

Probably better results, too.
Now that we calculate the set of loaded/stored symbols on a
per-statement basis, it may be quite easy to do.

>
> Now, regarding where to fix it properly: if you say that solution_set_add()
> is not the right place (and I don't buy that yet, because something is wrong
> with it no matter what, e.g. looking for subfields starting from the things
> in the pt set, which themself might be subfields feels wrong)
You keep thinking the solver knows anything about types or structures
or fields or programming languages or fields.
It doesn't.
The only thing it knows is that constraint variable + 5 is some other
constraint variable.

Clear your mind of any relation to the original program variables.
The variables the solver is talking about need to be mapped back to
original program variables.  They are simply a unique set of
variables, some of which may be accessed by offseting something else.

All relations of the original variables need to be expressed to the
solver in constraints. The solver does not, and should not, know
anything about programming languages and their type systems :)

> not use pointer arithmetic (or at least '+' constraints) to represent pointing
> to substructures.  And I'm not sure that's easily doable.  Because the insn
> we see is not "in = &cell.in", but "in = &o->in", and only the pt solver
> itself sees that 'o' points to cell.

for in = &o->in, we should add

in = o + <offset> constraints

for every offset that can exist in the typeof("o->in").

And we should be doing this inside the various structure copy
constraint builders, but it's quite possible i fucked it up.

This would give you what you want, and make in point to cell.in.ii

Note that this is all a side-effect of trying to shoehorn may-alias
info directly into points-to sets, because our system thinks that a->i
is an access only to whatever a can point-to, instead of what a->i can
point to.

> > You don't want to play in solution_set_add, which is common to a bunch of
> > code.
>
> Well, it's common to exactly the code which handles offsetted constraints.
> And I don't see why the problem we face here (with offseted copy constraints)
> doesn't also occur with any other of the offseted constraints, e.g.
> "*x = y+32", handled in do_ds_constraint.

*x = y + 32 is an invalid constraint.

do_ds_constraint can't actually ever have a call where roffset is not
zero.  I've actually made this explicit on the pta-dev branch.

(you'll note do_sd_constraint also can't have loffset != zero).


>
> > > You can never get back to the parent structure from a pointer to the
> > > substructure.
> > > I asked this explicitly and was told it was undefined behavior.
> >
> > Yep, this is what I convinced myself aswell.
>
> Well, it's perhaps illegal in C (I'm not sure about that actually),

Joseph told me it was
>  but
> certainly occurs in C++, and even if it didn't occur there we are talking
> about the middle-end, for which such invalidity shouldn't matter (except we
> choose to define it as invalid).  To see why this is valid, consider this
> C++ program:
This is done through subtraction in C++, and we explicitly don't support it.

If we can't figure out what is going on, we collapse the whole thing
to a single variable and call it a day.


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenther at suse dot de]

------- Comment #39 from rguenther at suse dot de  2007-06-05 19:59 -------
Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0
 based code with -fstrict-aliasing

On Tue, 5 Jun 2007, dberlin at dberlin dot org wrote:

> ------- Comment #38 from dberlin at gcc dot gnu dot org  2007-06-05 19:07 -------
> Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0 based code with
> -fstrict-aliasing
> 
> On 5 Jun 2007 18:24:54 -0000, matz at gcc dot gnu dot org
> <gcc-bugzilla@gcc.gnu.org> wrote:
> >
> > Now, regarding where to fix it properly: if you say that solution_set_add()
> > is not the right place (and I don't buy that yet, because something is wrong
> > with it no matter what, e.g. looking for subfields starting from the things
> > in the pt set, which themself might be subfields feels wrong)
> You keep thinking the solver knows anything about types or structures
> or fields or programming languages or fields.
> It doesn't.
> The only thing it knows is that constraint variable + 5 is some other
> constraint variable.

But there are cases there is no constraint variable for p + 5.  Like
the problem with the empty bases that we take the address of but don't
generate constraint variables for.  Or consider

int p[2];
void *q = p;
int *p2;
q = q + 1;
q = q + 1;
q = q + 1;
q = q + 1;
p2 = q;
*p2 = 1;

which is a valid way to store into p[1].  We obviously don't have
constraint variables for q + 1 -- we have one for &p[0] and one
for &p[1], but we cannot represent q + 1 exactly.  Somehow it
doesn't cause problems to "fall back" to &p[0] for &p[0] + 1B,
but it would be nice to have some more confidence in that parts :)

Richard.


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[dberlin at dberlin dot org]

------- Comment #40 from dberlin at gcc dot gnu dot org  2007-06-05 22:44 -------
Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0 based code with
-fstrict-aliasing

On 5 Jun 2007 19:59:32 -0000, rguenther at suse dot de
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #39 from rguenther at suse dot de  2007-06-05 19:59 -------
> Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0
>  based code with -fstrict-aliasing
>
> On Tue, 5 Jun 2007, dberlin at dberlin dot org wrote:
>
> > ------- Comment #38 from dberlin at gcc dot gnu dot org  2007-06-05 19:07 -------
> > Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0 based code with
> > -fstrict-aliasing
> >
> > On 5 Jun 2007 18:24:54 -0000, matz at gcc dot gnu dot org
> > <gcc-bugzilla@gcc.gnu.org> wrote:
> > >
> > > Now, regarding where to fix it properly: if you say that solution_set_add()
> > > is not the right place (and I don't buy that yet, because something is wrong
> > > with it no matter what, e.g. looking for subfields starting from the things
> > > in the pt set, which themself might be subfields feels wrong)
> > You keep thinking the solver knows anything about types or structures
> > or fields or programming languages or fields.
> > It doesn't.
> > The only thing it knows is that constraint variable + 5 is some other
> > constraint variable.
>
> But there are cases there is no constraint variable for p + 5.  Like
> the problem with the empty bases that we take the address of but don't
> generate constraint variables for.  Or consider
>
> int p[2];
> void *q = p;
> int *p2;
> q = q + 1;
> q = q + 1;
> q = q + 1;
> q = q + 1;
> p2 = q;
> *p2 = 1;
>
> which is a valid way to store into p[1].  We obviously don't have
> constraint variables for q + 1 -- we have one for &p[0] and one
> for &p[1], but we cannot represent q + 1 exactly.

q_2 = q_1 + 1
q_3 = q_2 + 1
q_4 = q_3 + 1
q_5 = q_4 + 1
p2 = q_5

Will become the set of constraints
q_2 = q_1
q_2 = q_1 + 1
q_3 = q_2
q_3 = q_2 + 1
q_4 = q_3
q_4 = q_3 + 1
q_5 = q_4
q_5 = q_4 +1

Hmmm.
You are right, we should give up here and collapse the variable.
Sigh.

Well, we can always just make it give up on all direct pointer
arithmetic, rather than try to handle it at all :)

> Somehow it
> doesn't cause problems to "fall back" to &p[0] for &p[0] + 1B,
> but it would be nice to have some more confidence in that parts :)

TBQH, the only way i'll ever have confidence in these parts is if we
make it stop trying to come up with everything a pointer to a
structure could deref.
Note that this also prevents us from doing cool things.

In particular, because we say a pointer to struct p points to all it's
members, we can never simply propagate the object it does point to if
it points to a single object.


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenther at suse dot de]

------- Comment #41 from rguenther at suse dot de  2007-06-06 08:49 -------
Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0
 based code with -fstrict-aliasing

On Wed, 5 Jun 2007, dberlin at dberlin dot org wrote:

> q_2 = q_1 + 1
> q_3 = q_2 + 1
> q_4 = q_3 + 1
> q_5 = q_4 + 1
> p2 = q_5
> 
> Will become the set of constraints
> q_2 = q_1
> q_2 = q_1 + 1
> q_3 = q_2
> q_3 = q_2 + 1
> q_4 = q_3
> q_4 = q_3 + 1
> q_5 = q_4
> q_5 = q_4 +1
> 
> Hmmm.
> You are right, we should give up here and collapse the variable.
> Sigh.
> 
> Well, we can always just make it give up on all direct pointer
> arithmetic, rather than try to handle it at all :)
> 
> > Somehow it
> > doesn't cause problems to "fall back" to &p[0] for &p[0] + 1B,
> > but it would be nice to have some more confidence in that parts :)
> 
> TBQH, the only way i'll ever have confidence in these parts is if we
> make it stop trying to come up with everything a pointer to a
> structure could deref.

Now, this is of course only because of aliasing.  It should be
completely irrelevant for points-to.  So, if points-to comes up with
"only" &struct as a solution we would need some intermediate processing
to translate it to the correct alias set, possibly looking at the
type hierarchy again.

> Note that this also prevents us from doing cool things.

Too bad ;)

> In particular, because we say a pointer to struct p points to all it's
> members, we can never simply propagate the object it does point to if
> it points to a single object.

So we seem to agree the current solution is ugly and doesn't really
work.  Enough time to fix it for 4.3 then ;)

Btw, how does a patch to disable field sensitivity look like for the 4.2
branch?

Richard.


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[dberlin at dberlin dot org]

------- Comment #42 from dberlin at gcc dot gnu dot org  2007-06-06 15:52 -------
Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0 based code with
-fstrict-aliasing

On 6 Jun 2007 08:49:50 -0000, rguenther at suse dot de
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #41 from rguenther at suse dot de  2007-06-06 08:49 -------
> Subject: Re:  [4.2 regression] miscompilation of sigc++-2.0
>  based code with -fstrict-aliasing
>
> On Wed, 5 Jun 2007, dberlin at dberlin dot org wrote:
>
> > q_2 = q_1 + 1
> > q_3 = q_2 + 1
> > q_4 = q_3 + 1
> > q_5 = q_4 + 1
> > p2 = q_5
> >
> > Will become the set of constraints
> > q_2 = q_1
> > q_2 = q_1 + 1
> > q_3 = q_2
> > q_3 = q_2 + 1
> > q_4 = q_3
> > q_4 = q_3 + 1
> > q_5 = q_4
> > q_5 = q_4 +1
> >
> > Hmmm.
> > You are right, we should give up here and collapse the variable.
> > Sigh.
> >
> > Well, we can always just make it give up on all direct pointer
> > arithmetic, rather than try to handle it at all :)
> >
> > > Somehow it
> > > doesn't cause problems to "fall back" to &p[0] for &p[0] + 1B,
> > > but it would be nice to have some more confidence in that parts :)
> >
> > TBQH, the only way i'll ever have confidence in these parts is if we
> > make it stop trying to come up with everything a pointer to a
> > structure could deref.
>
> Now, this is of course only because of aliasing.  It should be
> completely irrelevant for points-to.  So, if points-to comes up with
> "only" &struct as a solution we would need some intermediate processing
> to translate it to the correct alias set, possibly looking at the
> type hierarchy again.
>

Well, actually, you don't.

At the point where you process it, we either have created SFT's or we
haven't (if we haven't than the deref is going to touch the one
variable we've created for the struct)
The rest is more or less equivalent to what access_can_touch_variable does :)
> So we seem to agree the current solution is ugly and doesn't really
> work.  Enough time to fix it for 4.3 then ;)
>
> Btw, how does a patch to disable field sensitivity look like for the 4.2
> branch?

The patch is trivial.  We have static bool use_field_sensitive = true
at the top of the file.  Just set it to false and away you go.


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #43 from rguenth at gcc dot gnu dot org  2007-06-19 09:24 -------
Subject: Bug 30252

Author: rguenth
Date: Tue Jun 19 09:24:35 2007
New Revision: 125844

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125844
Log:
2007-06-19  Richard Guenther  <rguenther@suse.de>
        Michael Matz  <matz@suse.de>

        PR tree-optimization/30252
        * tree-ssa-structalias.c (solution_set_add): Make sure to
        preserve all relevant vars.
        (handle_ptr_arith): Make sure to only handle positive
        offsets.
        (push_fields_onto_fieldstack): Create fields for empty
        bases.

        * g++.dg/opt/pr30252.C: New testcase.

Added:
    branches/gcc-4_2-branch/gcc/testsuite/g++.dg/opt/pr30252.C
Modified:
    branches/gcc-4_2-branch/gcc/ChangeLog
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_2-branch/gcc/tree-ssa-structalias.c


-- 


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

[Bug c++/30252] [4.2 regression] miscompilation of sigc++-2.0 based code with -fstrict-aliasing

[rguenth at gcc dot gnu dot org]

------- Comment #44 from rguenth at gcc dot gnu dot org  2007-06-19 09:45 -------
Fixed on the 4.2 branch.  Danny will fix this in a different way on the trunk.


-- 

rguenth at gcc dot gnu dot org changed:

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


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