Bug 33199 - [4.3 Regression] tr1/2_general_utilities/shared_ptr/assign/auto_ptr.cc
Summary: [4.3 Regression] tr1/2_general_utilities/shared_ptr/assign/auto_ptr.cc
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.3.0
: P1 normal
Target Milestone: 4.3.0
Assignee: Not yet assigned to anyone
URL:
Keywords: alias, wrong-code
Depends on:
Blocks:
 
Reported: 2007-08-26 21:57 UTC by H.J. Lu
Modified: 2007-09-07 14:03 UTC (History)
5 users (show)

See Also:
Host:
Target: i686-pc-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-08-28 19:23:20


Attachments
A testcase (28.11 KB, application/octet-stream)
2007-08-27 16:43 UTC, H.J. Lu
Details
reduced testcase (15.99 KB, text/plain)
2007-08-28 16:24 UTC, Richard Biener
Details
reduced testcase for the volatile diffs (863 bytes, text/plain)
2007-08-30 12:14 UTC, Richard Biener
Details

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2007-08-26 21:57:31 UTC
Revision 127763:

http://gcc.gnu.org/ml/gcc-cvs/2007-08/msg00657.html

caused

FAIL: tr1/2_general_utilities/shared_ptr/assign/auto_ptr.cc execution test

on Linux/i686.
Comment 1 H.J. Lu 2007-08-26 21:59:32 UTC
The patch is at

http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01374.html
Comment 2 Andrew Pinski 2007-08-26 22:12:49 UTC
I don't see the failure on i386-apple-darwin8.10 (with revision 127806) so this must be some target issue.
Comment 3 H.J. Lu 2007-08-27 01:27:53 UTC
Revision 127763 removed varargs_function_p call for C++. I tried this patch:

Index: tree-inline.c
===================================================================
--- tree-inline.c       (revision 127763)
+++ tree-inline.c       (working copy)
@@ -1843,6 +1843,18 @@ egress:
   return ret;
 }

+/* Does FUNCTION use a variable-length argument list?  */
+
+static int
+varargs_function_p (tree function)
+{
+  tree parm = TYPE_ARG_TYPES (TREE_TYPE (function));
+  for (; parm; parm = TREE_CHAIN (parm))
+    if (TREE_VALUE (parm) == void_type_node)
+      return 0;
+  return 1;
+}
+
 /* Returns nonzero if FN is a function that does not have any
    fundamental inline blocking properties.  */

@@ -1881,6 +1893,9 @@ inlinable_function_p (tree fn)
           && DECL_REPLACEABLE_P (fn))
     inlinable = false;

+  else if (varargs_function_p (fn))
+    inlinable = false;
+
   else if (!function_attribute_inlinable_p (fn))
     {
       if (do_warning)

and it fixes the regression.
Comment 4 Richard Biener 2007-08-27 07:31:08 UTC
The inliner can perfectly cope with varargs if they are unused.  See gcc.dg/inline-23.c:

/* { dg-do compile } */
/* { dg-options "-std=gnu89" } */
/* Make sure we can inline a varargs function whose variable arguments
   are not used.  See PR32493.  */
#include <stddef.h>
static inline __attribute__((always_inline)) void __check_printsym_format(const
char *fmt, ...)
{
}
static inline __attribute__((always_inline)) void print_symbol(const char *fmt,
ptrdiff_t addr)
{
 __check_printsym_format(fmt, "");
}
void do_initcalls(void **call)
{
   print_symbol(": %s()", (ptrdiff_t) *call);
}


That is, for C we didn't exclude varargs functions from inlining.  (Not
that I think this is particular wise, but we'd regress in the testcase
above)

I'll have a look.
Comment 5 Richard Biener 2007-08-27 15:27:23 UTC
Doesn't fail for me on x86_64 with -m32.
Comment 6 H.J. Lu 2007-08-27 16:11:31 UTC
I saw it on Linux/ia64 running RHEL4, on Linux/ia32 running RHEL4 and RHEL5
as well as with -m32 on Linux/x86-64 running F7, RHEL4 and RHEL5. If I
have to guess, some vararg functions may be only available on certain
platforms. I can provide a preprocessed testcase.

BTW, is that possible to create a C++ testcase to show the problem with
vararg functions?
Comment 7 rguenther@suse.de 2007-08-27 16:16:11 UTC
Subject: Re:  [4.3 Regression]
 tr1/2_general_utilities/shared_ptr/assign/auto_ptr.cc

On Mon, 27 Aug 2007, hjl at lucon dot org wrote:

> ------- Comment #6 from hjl at lucon dot org  2007-08-27 16:11 -------
> I saw it on Linux/ia64 running RHEL4, on Linux/ia32 running RHEL4 and RHEL5
> as well as with -m32 on Linux/x86-64 running F7, RHEL4 and RHEL5. If I
> have to guess, some vararg functions may be only available on certain
> platforms. I can provide a preprocessed testcase.
> 
> BTW, is that possible to create a C++ testcase to show the problem with
> vararg functions?

preprocessed source would be nice for a failing case.

Richard.
Comment 8 H.J. Lu 2007-08-27 16:43:33 UTC
Created attachment 14124 [details]
A testcase

It should be compiled with

-O2 -m32 -g -O2 -ffunction-sections -fdata-sections -fmessage-length=0 -g -O2 -Wl,--gc-sections
Comment 9 H.J. Lu 2007-08-27 20:39:53 UTC
This patch:

Index: decl.c
===================================================================
--- decl.c      (revision 127763)
+++ decl.c      (working copy)
@@ -11525,6 +11525,9 @@ finish_function (int flags)
       && !DECL_REPLACEABLE_P (fndecl))
     TREE_NOTHROW (fndecl) = 1;
 
+  if (varargs_function_p (fndecl))
+    DECL_UNINLINABLE (fndecl) = 1;
+
   /* This must come after expand_function_end because cleanups might
      have declarations (from inline functions) that need to go into
      this function's blocks.  */

also works on the testcase. I don't know if it is correct.
Comment 10 Jakub Jelinek 2007-08-27 21:42:17 UTC
This really can't be worked around, but has to be fixed properly.
for i in "" "-g"; do ./g++ -B ./ -O2 $i -m32 -o auto_ptr /tmp/auto_ptr.ii -L../x86_64-unknown-linux-gnu/32/libstdc++-v3/src/.libs/ -Wl,-rpath,../x86_64-unknown-linux-gnu/32/libstdc++-v3/src/.libs/; ./auto_ptr; echo $?; done
0
auto_ptr: /export/gnu/import/rrs/127763/src/libstdc++-v3/testsuite/tr1/2_general_utilities/shared_ptr/assign/auto_ptr.cc:72: int test01(): Assertion `A::dtor_count == 1' failed.
Aborted
134

This shows -g affects code generation in this case, which is a very severe problem.  Will look into this tomorrow unless somebody beats me to do that.
Comment 11 H.J. Lu 2007-08-27 21:57:30 UTC
build_new_1 has

  /* Now, check to see if this function is actually a placement
     allocation function.  This can happen even when PLACEMENT is NULL
     because we might have something like:

       struct S { void* operator new (size_t, int i = 0); };

     A call to `new S' will get this allocation function, even though
     there is no explicit placement argument.  If there is more than
     one argument, or there are variable arguments, then this is a
     placement allocation function.  */
  placement_allocation_fn_p
    = (type_num_arguments (TREE_TYPE (alloc_fn)) > 1
       || varargs_function_p (alloc_fn));

  /* Preevaluate the placement args so that we don't reevaluate them for a
     placement delete.  */
  if (placement_allocation_fn_p)
    {
      tree inits;
      stabilize_call (alloc_call, &inits);
      if (inits)
        alloc_expr = build2 (COMPOUND_EXPR, TREE_TYPE (alloc_expr), inits,
                             alloc_expr);
    }

...
    /* If any part of the object initialization terminates by throwing an
         exception and a suitable deallocation function can be found, the
         deallocation function is called to free the memory in which the
         object was being constructed, after which the exception continues
         to propagate in the context of the new-expression. If no
         unambiguous matching deallocation function can be found,
         propagating the exception does not cause the object's memory to be
         freed.  */
      if (flag_exceptions && ! use_java_new)
        {
          enum tree_code dcode = array_p ? VEC_DELETE_EXPR : DELETE_EXPR;
          tree cleanup;

          /* The Standard is unclear here, but the right thing to do
             is to use the same method for finding deallocation
             functions that we use for finding allocation functions.  */
          cleanup = build_op_delete_call (dcode, alloc_node, size,
                                          globally_qualified_p,
                                          (placement_allocation_fn_p
                                           ? alloc_call : NULL_TREE),
                                          alloc_fn);

I think it is related.
Comment 12 Richard Biener 2007-08-28 08:52:14 UTC
Note I think it is perfectly reasonable and wanted that we can inline

   template<_Lock_policy _Lp>
     inline void
     __enable_shared_from_this_helper(const __shared_count<_Lp>&, ...)
     { }

and thus eliminate the call.  We should figure out why we miscompile things
if we do so though (and only if using -g, which is even more embarrassing).
Comment 13 Paolo Carlini 2007-08-28 08:56:08 UTC
(In reply to comment #12)
Totally agreed. In any case, I think we all agree that: 1- Miscompilations are always very bad; 2- It is legal C++; 3- Any user, not just library authors, can write such kind of code.

Comment 14 Richard Biener 2007-08-28 11:05:31 UTC
I'm reducing the testcase now.
Comment 15 Richard Biener 2007-08-28 13:54:23 UTC
The first difference (between -g and non--g build) is after DSE where for the -g
build it doesn't delete a redundant store.  Which is because we have different
alias information with/without -g (in fact this effect looks like a dup of
PR32624):

-  D.10781_25 = &D.10780_24->D.10279;
-  # SMT.491_337 = VDEF <SMT.491_331(ab)>
-  D.10780_24->D.10279._vptr._Sp_counted_base = &_ZTVNSt3tr116_Sp_counted_baseILN9__gnu_cxx12_Lock_policyE1EEE[2];
-  # SMT.491_339 = VDEF <SMT.491_337>
-  D.10780_24->D.10279._M_use_count = 1;
-  # SMT.491_341 = VDEF <SMT.491_339>
-  D.10780_24->D.10279._M_weak_count = 1;
-  # SMT.491_342 = VDEF <SMT.491_341>
-  D.10780_24->D.10279._vptr._Sp_counted_base = &_ZTVNSt3tr121_Sp_counted_base_implIP1BNS_11_Sp_deleterIS1_EELN9__gnu_cxx12_Lock_policyE1EEE[2];
-  # SMT.491_343 = VDEF <SMT.491_342>
-  D.10780_24->_M_ptr = D.9911_4;
-  # VUSE <__d_335>
-  # SMT.491_344 = VDEF <SMT.491_343>
-  D.10780_24->_M_del = __d;
+  D.10923_25 = &D.10922_24->D.10280;
+  # SMT.487_370 = VDEF <SMT.487_353(ab)>
+  # SMT.488_360 = VDEF <SMT.488_354(ab)>
+  D.10922_24->D.10280._vptr._Sp_counted_base = &_ZTVNSt3tr116_Sp_counted_baseILN9__gnu_cxx12_Lock_policyE1EEE[2];
+  # SMT.487_26 = VDEF <SMT.487_370>
+  # SMT.488_361 = VDEF <SMT.488_360>
+  D.10922_24->D.10280._M_use_count = 1;
+  # SMT.487_345 = VDEF <SMT.487_26>
+  # SMT.488_362 = VDEF <SMT.488_361>
+  D.10922_24->D.10280._M_weak_count = 1;
+  # SMT.487_363 = VDEF <SMT.487_345>
+  # SMT.488_364 = VDEF <SMT.488_362>
+  D.10922_24->D.10280._vptr._Sp_counted_base = &_ZTVNSt3tr121_Sp_counted_base_implIP1BNS_11_Sp_deleterIS1_EELN9__gnu_cxx12_Lock_policyE1EEE[2];
+  # SMT.487_365 = VDEF <SMT.487_363>
+  # SMT.488_366 = VDEF <SMT.488_364>
+  D.10922_24->_M_ptr = D.9912_4;
+  # VUSE <__d_359>
+  # SMT.487_367 = VDEF <SMT.487_365>
+  # SMT.488_368 = VDEF <SMT.488_366>
+  D.10922_24->_M_del = __d;


note how we have two SMTs per store for -g but only one without -g.


Comment 16 Richard Biener 2007-08-28 15:56:59 UTC
The differences in alias info are due to different memory partitioning that happens (regardless of same IL, same number of referenced vars and same BB
frequencies) as we have different "Memory symbol references before partitioning:"

For example one difference is

read frequency:      0, write frequency:      0, direct reads:  34, direct writes:   3, indirect reads:    0, indirect writes:   26, symbol: SMT.486, tags: NIL

vs.

read frequency:  20000, write frequency:  30000, direct reads:  34, direct writes:   3, indirect reads:    2, indirect writes:   29, symbol: SMT.487, tags: { SMT.486 }

where the thing to investigate is why the "tags:" part is different (that part
causes the number of indirect reads/writes to be different)
Comment 17 Richard Biener 2007-08-28 16:24:41 UTC
Created attachment 14128 [details]
reduced testcase

Reduced testcase.  Note that even removing an unused class (struct counter) makes
the testcase "work".
Comment 18 Richard Biener 2007-08-28 19:23:20 UTC
Just to add, the reduced testcase fails with r127848.  I suppose the failure
might vanish with the inliner change I put in after that.
Comment 19 Richard Biener 2007-08-29 10:32:09 UTC
MTAG_ALIASES of the SMTs are different:

SMT.469 no aliases
SMT.470 no aliases
SMT.464{ ctor_count ctor_count dtor_count SMT.465 SMT.470 }
SMT.465{ ctor_count ctor_count dtor_count SMT.470 }
SMT.466{ ctor_count ctor_count dtor_count SMT.469 }
SMT.468{ ctor_count ctor_count dtor_count SMT.469 SMT.470 }
SMT.467{ ctor_count ctor_count dtor_count }

SMT.464D.9338, UID D.9338, _Atomic_wordD.5734, is addressable, is global, is volatile, direct reads: 0, direct writes: 9, indirect reads: 0, indirect writes: 26, read frequency: 0, write frequency: 0, call clobbered (passed to call, is global var), may aliases: { ctor_countD.7347 ctor_countD.7372 dtor_countD.7373 SMT.465D.9339 SMT.470D.9344 }
SMT.465D.9339, UID D.9339, struct _Sp_counted_baseD.6722, is addressable, is global, direct reads: 34, direct writes: 3, indirect reads: 0, indirect writes: 35, read frequency: 0, write frequency: 401, call clobbered (stored in global, passed to call, is global var), may aliases: { ctor_countD.7347 ctor_countD.7372 dtor_countD.7373 SMT.470D.9344 }
SMT.469D.9343, UID D.9343, struct AD.7343, is addressable, is global, direct reads: 0, direct writes: 0, indirect reads: 2, indirect writes: 27, read frequency: 7108, write frequency: 10000, call clobbered (stored in global, passed to call, is global var)
SMT.470D.9344, UID D.9344, struct _Sp_counted_base_implD.8267, is addressable, is global, direct reads: 2, direct writes: 3, indirect reads: 34, indirect writes: 38, read frequency: 47544, write frequency: 30401, call clobbered (stored in global, passed to call, is global var)
SMT.466D.9340, UID D.9340, struct BD.7365, is addressable, is global, direct reads: 2, direct writes: 1, indirect reads: 0, indirect writes: 26, read frequency: 0, write frequency: 0, call clobbered (stored in global, passed to call, is global var), may aliases: { ctor_countD.7347 ctor_countD.7372 dtor_countD.7373 SMT.469D.9343 }, belongs to partition: MPT.471D.9345
SMT.468D.9342, UID D.9342, , is addressable, is global, direct reads: 0, direct writes: 0, indirect reads: 0, indirect writes: 26, read frequency: 0, write frequency: 0, call clobbered (passed to call, is global var), may aliases: { ctor_countD.7347 ctor_countD.7372 dtor_countD.7373 SMT.469D.9343 SMT.470D.9344 }, belongs to partition: MPT.471D.9345
SMT.467D.9341, UID D.9341, intD.2 (*__vtbl_ptr_typeD.1557) (void), is addressable, is global, direct reads: 10, direct writes: 0, indirect reads: 0, indirect writes: 26, read frequency: 0, write frequency: 0, call clobbered (is global var), may aliases: { ctor_countD.7347 ctor_countD.7372 dtor_countD.7373 }, belongs to partition: MPT.471D.9345

vs.

SMT.464{ ctor_count ctor_count dtor_count SMT.465 SMT.466 }
SMT.465{ ctor_count ctor_count dtor_count SMT.466 }
SMT.466{ ctor_count ctor_count dtor_count }
SMT.467{ ctor_count ctor_count dtor_count }
SMT.468{ ctor_count ctor_count dtor_count SMT.469 SMT.470 }
SMT.469{ SMT.470 }
SMT.470 no aliases

SMT.464D.9522, UID D.9522, struct _Sp_counted_base_implD.8268, is addressable, is global, direct reads: 2, direct writes: 3, indirect reads: 0, indirect writes: 26, read frequency: 0, write frequency: 0, call clobbered (is global var), may aliases: { ctor_countD.7348 ctor_countD.7373 dtor_countD.7374 SMT.465D.9523 SMT.466D.9524 }, belongs to partition: MPT.471D.9529
SMT.465D.9523, UID D.9523, struct _Sp_counted_baseD.6723, is addressable, is global, direct reads: 34, direct writes: 3, indirect reads: 2, indirect writes: 29, read frequency: 20000, write frequency: 30000, call clobbered (stored in global, passed to call, is global var), may aliases: { ctor_countD.7348 ctor_countD.7373 dtor_countD.7374 SMT.466D.9524 }
SMT.466D.9524, UID D.9524, _Atomic_wordD.5735, is addressable, is global, is volatile, direct reads: 0, direct writes: 9, indirect reads: 36, indirect writes: 32, read frequency: 67544, write frequency: 60000, call clobbered (stored in global, passed to call, is global var), may aliases: { ctor_countD.7348 ctor_countD.7373 dtor_countD.7374 }
SMT.467D.9525, UID D.9525, intD.2 (*__vtbl_ptr_typeD.1558) (void), is addressable, is global, direct reads: 10, direct writes: 0, indirect reads: 0, indirect writes: 26, read frequency: 0, write frequency: 0, call clobbered (is global var), may aliases: { ctor_countD.7348 ctor_countD.7373 dtor_countD.7374 }
SMT.468D.9526, UID D.9526, , is addressable, is global, direct reads: 0, direct writes: 0, indirect reads: 0, indirect writes: 26, read frequency: 0, write frequency: 0, call clobbered (passed to call, is global var), may aliases: { ctor_countD.7348 ctor_countD.7373 dtor_countD.7374 SMT.470D.9528 MPT.471D.9529 }, belongs to partition: MPT.471D.9529
SMT.469D.9527, UID D.9527, struct BD.7366, is addressable, is global, direct reads: 2, direct writes: 1, indirect reads: 0, indirect writes: 26, read frequency: 0, write frequency: 0, call clobbered (stored in global, passed to call, is global var), may aliases: { SMT.470D.9528 }, belongs to partition: MPT.471D.9529
SMT.470D.9528, UID D.9528, struct AD.7344, is addressable, is global, direct reads: 0, direct writes: 0, indirect reads: 2, indirect writes: 27, read frequency: 7108, write frequency: 10000, call clobbered (stored in global, passed to call, is global var)
Comment 20 Richard Biener 2007-08-29 11:58:30 UTC
Due to the alias differences with -g compared to without -g we have the following
difference after tree-level optimization:

--- -   2007-08-29 13:52:02.567822000 +0200
+++ b/auto_ptr.min.ii.116t.optimized    2007-08-29 13:51:05.000000000 +0200
@@ -505,6 +505,7 @@ int test01() ()
   D.8890 = (struct _Sp_counted_base_impl *) D.8889;
   __d = __d.79;
   D.8891 = &D.8890->D.8307;
+  D.8890->D.8307._vptr._Sp_counted_base = &_ZTVNSt3tr116_Sp_counted_baseILN9__gnu_cxx12_Lock_policyE1EEE[2];
   D.8890->D.8307._M_use_count = 1;
   D.8890->D.8307._M_weak_count = 1;
   D.8890->D.8307._vptr._Sp_counted_base = &_ZTVNSt3tr121_Sp_counted_base_implIP1BNS_11_Sp_deleterIS1_EELN9__gnu_cxx12_Lock_policyE1EEE[2];
@@ -529,9 +530,9 @@ Invalid sum of incoming frequencies 8500
   goto <bb 9>;
 
 <bb 8>:
-  D.8898 = D.8890->D.8307._M_use_count + 1;
-  *D.8895 ={v} D.8898;
   __result = D.8890->D.8307._M_use_count;
+  D.8898 = __result + 1;
+  *D.8895 ={v} D.8898;
   D.9413 = __result + -1;
   *D.8895 ={v} D.9413;
   goto <bb 11>;

that is, we have one dead store removed by DSE and PRE removes one load of
_M_use_count:

 int test01() ()
 {
+  _Atomic_word prephitmp.476;
+  _Atomic_word pretmp.475;
   int storetmp.473;
   volatile _Atomic_word * __mem.22;
   _Atomic_word __result;
@@ -3806,9 +3836,11 @@ Invalid sum of incoming frequencies 8415
   goto <bb 13>;
 
 <bb 29>:
+  pretmp.475_164 = D.8890_24->D.8307._M_use_count;
 
 <bb 12>:
-  __result_86 = D.8890_24->D.8307._M_use_count;
+  # prephitmp.476_144 = PHI <pretmp.475_164(29), D.8897_31(8)>
+  __result_86 = prephitmp.476_144;
   D.9413_88 = __result_86 + -1;
   *D.8895_29 ={v} D.9413_88;
 
before PRE this looks like

<bb 6>:
Invalid sum of incoming frequencies 8500, should be 7225
  D.8895_29 = &D.8890_24->D.8307._M_use_count;
...

<bb 8>:
  # VUSE <SMT.465_361, SMT.466_362>
  D.8897_31 = D.8890_24->D.8307._M_use_count;
  D.8898_32 = D.8897_31 + 1;
  # ctor_count_373 = VDEF <ctor_count_344(ab)>
  # ctor_count_374 = VDEF <ctor_count_345(ab)>
  # dtor_count_375 = VDEF <dtor_count_346(ab)>
  *D.8895_29 ={v} D.8898_32;
  goto <bb 12>;

...

<bb 29>:

<bb 12>:
  # MPT.471_203 = PHI <MPT.471_280(29), MPT.471_351(ab)(8)>
  # SMT.470_61 = PHI <SMT.470_260(29), SMT.470_350(ab)(8)>
  # SMT.467_59 = PHI <SMT.467_240(29), SMT.467_349(ab)(8)>
  # SMT.466_58 = PHI <SMT.466_220(29), SMT.466_362(8)>
  # SMT.465_54 = PHI <SMT.465_200(29), SMT.465_361(8)>
  # dtor_count_53 = PHI <dtor_count_180(29), dtor_count_375(8)>
  # ctor_count_52 = PHI <ctor_count_160(29), ctor_count_374(8)>
  # ctor_count_48 = PHI <ctor_count_140(29), ctor_count_373(8)>
  # VUSE <SMT.465_54, SMT.466_58>
  __result_86 = D.8890_24->D.8307._M_use_count;
  D.9413_88 = __result_86 + -1;
  # ctor_count_449 = VDEF <ctor_count_48>
  # ctor_count_450 = VDEF <ctor_count_52>
  # dtor_count_451 = VDEF <dtor_count_53>
  *D.8895_29 ={v} D.9413_88;


forwprop doesn't propagate the address expr &D.8890_24->D.8307._M_use_count
to the dereference because the dereference site has volatile ops.  But
you can see from <bb 8> that we have wrong alias info there - the load
does not use the ctor_count and dtor_count syms and the store does not
def the two SMTs.
Comment 21 Richard Biener 2007-08-29 15:19:09 UTC
I wonder why D.9380_64, defined as

  D.9380_64 = &D.8894_34->_M_use_count;

points to anything and NULL:

  D.9380_64, is dereferenced, points-to anything, points-to NULL

where the single dereference site looks like

  # ctor_count_403 = VDEF <ctor_count_140>
  # ctor_count_404 = VDEF <ctor_count_160>
  # dtor_count_405 = VDEF <dtor_count_180>
  *D.9380_64 ={v} D.9383_69;

of course because of the constraints:

  D.9380_64 = { NULL }

possibly because

  # VUSE <SFT.433_337>
  D.8894_34 = D.8885._M_refcount._M_pi;

which also

  D.8894_34, is dereferenced, its value escapes, points-to anything, points-to NULL

which is because

  D.8885._M_pi = &NULL

but (!?) we have

...
  D.7990_3 ={v} operator new (4);

<bb 4>:
  D.7950_4 = (struct B *) D.7990_3;
...
  # SMT.470_328 = VDEF <SMT.470_325(ab)>
  D.7950_4->_vptr.B = &_ZTV1B[2];
...
  # SFT.432_331 = VDEF <SFT.432_330(D)>
  __ref.80._M_ptr = D.7950_4;
  # VUSE <SFT.432_331>
  __ref$_M_ptr_14 = __ref.80._M_ptr;
  # SFT.425_333 = VDEF <SFT.425_332(D)>
  b._M_ptr = __ref$_M_ptr_14;
  # VUSE <SFT.425_333>
  D.8873_20 = b._M_ptr;
  D.8884_21 = (struct A *) D.8873_20;
  # SFT.434_335 = VDEF <SFT.434_334(D)>
  D.8885._M_ptr = D.8884_21;

so it is at most non-null, because we dereference the pointer.

Note we miss(?) a constraint for D.7990_3 but only have

D.7950_4 = D.7990_3
__ref.80 = D.7950_4
__ref$_M_ptr_14 = __ref.80
b = __ref$_M_ptr_14
D.8873_20 = b
D.8884_21 = D.8873_20
D.8885 = D.8884_21
(and then directly)
D.8885._M_pi = &NULL

shouldn't we have

D.7990_3 = &ANYTHING

?

In find_func_aliases we don't create a constraint for the lhs of a call
at all:

  else if (((TREE_CODE (t) == GIMPLE_MODIFY_STMT
             && TREE_CODE (GIMPLE_STMT_OPERAND (t, 1)) == CALL_EXPR
             && !(call_expr_flags (GIMPLE_STMT_OPERAND (t, 1))
                  & (ECF_MALLOC | ECF_MAY_BE_ALLOCA)))
            || (TREE_CODE (t) == CALL_EXPR
                && !(call_expr_flags (t)
                     & (ECF_MALLOC | ECF_MAY_BE_ALLOCA)))))
    {
      if (!in_ipa_mode)
        {
          if (TREE_CODE (t) == GIMPLE_MODIFY_STMT)
            handle_rhs_call (GIMPLE_STMT_OPERAND (t, 1));
          else
            handle_rhs_call (t);
        }

So the following adds this constraint:

Index: tree-ssa-structalias.c
===================================================================
--- tree-ssa-structalias.c      (revision 127848)
+++ tree-ssa-structalias.c      (working copy)
@@ -3726,7 +3726,23 @@ find_func_aliases (tree origt)
       if (!in_ipa_mode)
        {
          if (TREE_CODE (t) == GIMPLE_MODIFY_STMT)
-           handle_rhs_call (GIMPLE_STMT_OPERAND (t, 1));
+           {
+             handle_rhs_call (GIMPLE_STMT_OPERAND (t, 1));
+             if (POINTER_TYPE_P (TREE_TYPE (GIMPLE_STMT_OPERAND (t, 1))))
+               {
+                 VEC(ce_s, heap) *lhsc = NULL;
+                 struct constraint_expr rhsc;
+                 unsigned int j;
+                 struct constraint_expr *lhsp;
+                 rhsc.var = anything_id;
+                 rhsc.offset = 0;
+                 rhsc.type = ADDRESSOF;
+                 get_constraint_for (GIMPLE_STMT_OPERAND (t, 0), &lhsc);
+                  for (j = 0; VEC_iterate (ce_s, lhsc, j, lhsp); j++)
+                    process_constraint_1 (new_constraint (*lhsp, rhsc), true);
+                 VEC_free (ce_s, heap, lhsc);
+               }
+           }
          else
            handle_rhs_call (t);
        }

but still we end up with

D.8885 = D.8884_21
D.8885._M_pi = &NULL

!?

hm, we have

  # SFT.433_314 = VDEF <SFT.433_313(D)>
  D.8885._M_refcount._M_pi = 0B;

so that might be ok.  The above patch fixes the failure for me, but
this might be pure luck given the fragile aliasing machinery.  So, does
the patch look anywhere sane?
Comment 22 Daniel Berlin 2007-08-29 18:30:33 UTC
Subject: Re:  [4.3 Regression] tr1/2_general_utilities/shared_ptr/assign/auto_ptr.cc

On 29 Aug 2007 15:19:10 -0000, rguenth at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #21 from rguenth at gcc dot gnu dot org  2007-08-29 15:19 -------
> I wonder why D.9380_64, defined as
>
>   D.9380_64 = &D.8894_34->_M_use_count;
>
> points to anything and NULL:
>
>   D.9380_64, is dereferenced, points-to anything, points-to NULL
>
> where the single dereference site looks like
>
>   # ctor_count_403 = VDEF <ctor_count_140>
>   # ctor_count_404 = VDEF <ctor_count_160>
>   # dtor_count_405 = VDEF <dtor_count_180>
>   *D.9380_64 ={v} D.9383_69;
>
> of course because of the constraints:
>
>   D.9380_64 = { NULL }
>
> possibly because
>
>   # VUSE <SFT.433_337>
>   D.8894_34 = D.8885._M_refcount._M_pi;
>
> which also
>
>   D.8894_34, is dereferenced, its value escapes, points-to anything, points-to
> NULL
>
> which is because
>
>   D.8885._M_pi = &NULL
>
> but (!?) we have
>
> ...
>   D.7990_3 ={v} operator new (4);
>
> <bb 4>:
>   D.7950_4 = (struct B *) D.7990_3;
> ...
>   # SMT.470_328 = VDEF <SMT.470_325(ab)>
>   D.7950_4->_vptr.B = &_ZTV1B[2];
> ...
>   # SFT.432_331 = VDEF <SFT.432_330(D)>
>   __ref.80._M_ptr = D.7950_4;
>   # VUSE <SFT.432_331>
>   __ref$_M_ptr_14 = __ref.80._M_ptr;
>   # SFT.425_333 = VDEF <SFT.425_332(D)>
>   b._M_ptr = __ref$_M_ptr_14;
>   # VUSE <SFT.425_333>
>   D.8873_20 = b._M_ptr;
>   D.8884_21 = (struct A *) D.8873_20;
>   # SFT.434_335 = VDEF <SFT.434_334(D)>
>   D.8885._M_ptr = D.8884_21;
>
> so it is at most non-null, because we dereference the pointer.
>
> Note we miss(?) a constraint for D.7990_3 but only have
>
> D.7950_4 = D.7990_3
> __ref.80 = D.7950_4
> __ref$_M_ptr_14 = __ref.80
> b = __ref$_M_ptr_14
> D.8873_20 = b
> D.8884_21 = D.8873_20
> D.8885 = D.8884_21
> (and then directly)
> D.8885._M_pi = &NULL
>
> shouldn't we have
>
> D.7990_3 = &ANYTHING
>
> ?
>
> In find_func_aliases we don't create a constraint for the lhs of a call
> at all:
>
>   else if (((TREE_CODE (t) == GIMPLE_MODIFY_STMT
>              && TREE_CODE (GIMPLE_STMT_OPERAND (t, 1)) == CALL_EXPR
>              && !(call_expr_flags (GIMPLE_STMT_OPERAND (t, 1))
>                   & (ECF_MALLOC | ECF_MAY_BE_ALLOCA)))
>             || (TREE_CODE (t) == CALL_EXPR
>                 && !(call_expr_flags (t)
>                      & (ECF_MALLOC | ECF_MAY_BE_ALLOCA)))))
>     {
>       if (!in_ipa_mode)
>         {
>           if (TREE_CODE (t) == GIMPLE_MODIFY_STMT)
>             handle_rhs_call (GIMPLE_STMT_OPERAND (t, 1));
>           else
>             handle_rhs_call (t);
>         }
>
> So the following adds this constraint:
>
> Index: tree-ssa-structalias.c
> ===================================================================
> --- tree-ssa-structalias.c      (revision 127848)
> +++ tree-ssa-structalias.c      (working copy)
> @@ -3726,7 +3726,23 @@ find_func_aliases (tree origt)
>        if (!in_ipa_mode)
>         {
>           if (TREE_CODE (t) == GIMPLE_MODIFY_STMT)
> -           handle_rhs_call (GIMPLE_STMT_OPERAND (t, 1));
> +           {
> +             handle_rhs_call (GIMPLE_STMT_OPERAND (t, 1));
> +             if (POINTER_TYPE_P (TREE_TYPE (GIMPLE_STMT_OPERAND (t, 1))))
> +               {
> +                 VEC(ce_s, heap) *lhsc = NULL;
> +                 struct constraint_expr rhsc;
> +                 unsigned int j;
> +                 struct constraint_expr *lhsp;
> +                 rhsc.var = anything_id;
> +                 rhsc.offset = 0;
> +                 rhsc.type = ADDRESSOF;
> +                 get_constraint_for (GIMPLE_STMT_OPERAND (t, 0), &lhsc);
> +                  for (j = 0; VEC_iterate (ce_s, lhsc, j, lhsp); j++)
> +                    process_constraint_1 (new_constraint (*lhsp, rhsc), true);
> +                 VEC_free (ce_s, heap, lhsc);
> +               }
> +           }


This is right for !in_ipa_mode.  You may want to encapsulate it into a
new "handle_lhs_of_call" though :)
Comment 23 Richard Biener 2007-08-30 09:07:37 UTC
I don't think the patch fixes anything.  Can you elaborate on

"D.8892_26 is a non-pointer variable, eliminating edges."

but D.8892 _is_ a pointer.  Or is it just that D.8892_26 is ultimately copied
from a pointer that doesn't have a constraint?  (because it is initialized
from a call, this is what the patch would "fix" - it creates a lot more NMTs
and (by luck?) sees that now, even if with and without the patch D.8895_29
and __tmp_27 point to anything, they may point to the same thing)
Comment 24 Richard Biener 2007-08-30 09:49:27 UTC
Note that even with the proposed patch we generate different code dependent on
if -g is enabled or not.  Starting with the first alias pass there are differences
in the has_volatile_ops annotations!

@@ -3818,9 +3823,9 @@ int test01() ()
   D.8890_24 = (struct _Sp_counted_base_impl *) D.8889_23;
   __d = __d.79;
   D.8891_25 = &D.8890_24->D.8307;
-  D.8891_25->_vptr._Sp_counted_base ={v} &_ZTVNSt3tr116_Sp_counted_baseILN9__gnu_cxx12_Lock_policyE1EEE[2];
-  D.8891_25->_M_use_count ={v} 1;
-  D.8891_25->_M_weak_count ={v} 1;
+  D.8891_25->_vptr._Sp_counted_base = &_ZTVNSt3tr116_Sp_counted_baseILN9__gnu_cxx12_Lock_policyE1EEE[2];
+  D.8891_25->_M_use_count = 1;
+  D.8891_25->_M_weak_count = 1;
   D.8890_24->D.8307._vptr._Sp_counted_base = &_ZTVNSt3tr121_Sp_counted_base_implIP1BNS_11_Sp_deleterIS1_EELN9__gnu_cxx12_Lock_policyE1EEE[2];
   D.8890_24->_M_ptr = D.8855_22;
   D.8890_24->_M_del = __d;
@@ -3840,7 +3845,7 @@ int test01() ()
     goto <bb 10>;
 
 <bb 7>:
-  D.8895_29 ={v} &__tmp_27->_M_use_count;
+  D.8895_29 = &__tmp_27->_M_use_count;
   if (__gthrw_pthread_cancel != 0B)
     goto <bb 8>;
   else
...

etc.

I'm trying to reduce the testcase further based on such differences.
Comment 25 Richard Biener 2007-08-30 12:14:06 UTC
Created attachment 14142 [details]
reduced testcase for the volatile diffs

against same svn revision + the proposed patch
Comment 26 Richard Biener 2007-08-30 14:52:52 UTC
Subject: Bug 33199

Author: rguenth
Date: Thu Aug 30 14:52:28 2007
New Revision: 127927

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127927
Log:
2007-08-30  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/33199
	* tree-ssa-structalias.c (handle_lhs_call): New function.
	(find_func_aliases): In non-IPA mode make sure that for
	calls that return a pointer we add a constraint for the
	result to point to anything.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-structalias.c

Comment 27 Daniel Berlin 2007-08-30 14:56:56 UTC
(In reply to comment #23)
> I don't think the patch fixes anything.  

Uh, sure it does.

Before we were ignoring the pointer results from calls.

They should point to anything.

> Can you elaborate on
> 
> "D.8892_26 is a non-pointer variable, eliminating edges."
> 
This really means that it was never used.  The only variables that are unused must come from things are not really pointers (IE don't point to anything).

I can change the wording :)

> but D.8892 _is_ a pointer.  Or is it just that D.8892_26 is ultimately copied
> from a pointer that doesn't have a constraint?  

Right.
> (because it is initialized

> from a call, this is what the patch would "fix" - it creates a lot more NMTs
> and (by luck?) sees that now, even if with and without the patch D.8895_29
> and __tmp_27 point to anything, they may point to the same thing)


For sure the patch is the right thing to do.  Whether it fixes this bug, is the real cause of this bug, etc, i can't say.

I can only say that we should be assigning &ANYTHING to the LHS of a call in !in_ipa_mode, and not doing so was a dumb oversight on my part when i fixed the call handling for !in_ipa_mode.  I actually thought we were already doing this, which is why i approved this so quickly.

> 

Comment 28 Richard Biener 2007-09-07 14:03:27 UTC
I'm closing this as fixed as I don't see the failure any more.