Bug 34385 - [4.3 Regression] new miscompilation after PR34148 fix
Summary: [4.3 Regression] new miscompilation after PR34148 fix
Status: RESOLVED DUPLICATE of bug 21920
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: 4.3.0
Assignee: Not yet assigned to anyone
Keywords: wrong-code
: 34570 (view as bug list)
Depends on:
Reported: 2007-12-07 22:45 UTC by Dirk Mueller
Modified: 2008-12-04 16:34 UTC (History)
23 users (show)

See Also:
Known to work:
Known to fail:
Last reconfirmed:

testcase (1.26 KB, text/plain)
2007-12-07 22:46 UTC, Dirk Mueller

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Mueller 2007-12-07 22:45:35 UTC
the patch in PR34148 uncovered a new bug, avoidable with -fno-strict-aliasing
Comment 1 Dirk Mueller 2007-12-07 22:46:29 UTC
Created attachment 14710 [details]

compile with g++ -ansi -O2
Comment 2 Dirk Mueller 2007-12-07 22:47:08 UTC
diff between broken -fdump-tree-optimized and working one is really small:

--- out.cc.optimized.broken     2007-12-07 23:35:37.460943000 +0100
+++ out.cc.optimized.works      2007-12-07 23:35:53.641747000 +0100
@@ -40,6 +40,7 @@
   D.1626 = (struct QHashNode *) D.1625;
   D.1626->key.d = 0B;
   D.1626->key = *str;
+  D.1628.i = (struct Node *) D.1626;
   return &((struct QHashNode *) ((const struct const_iterator *) &D.1628)->i)->key;

Comment 3 Andrew Pinski 2007-12-07 22:52:44 UTC
This code does not look defined:

            i = reinterpret_cast < const const_iterator & >(o).i;

Where o is iterator&.
Comment 4 Richard Biener 2007-12-07 23:14:16 UTC
Right, that is invalid.

*** This bug has been marked as a duplicate of 21920 ***
Comment 5 Richard Biener 2007-12-07 23:47:01 UTC
We don't warn with -Wstrict-aliasing=3 first because we prune the points-to
sets based on TBAA which makes o.11_19 in

  o.11_19 = (const struct const_iterator *) &D.2360;
  # VUSE <D.2360_76>
  D.2362_20 = o.11_19->i;

point to anything.  If you "fix" that with

Index: tree-ssa-structalias.c
--- tree-ssa-structalias.c      (revision 130692)
+++ tree-ssa-structalias.c      (working copy)
@@ -5014,7 +5014,7 @@ find_what_p_points_to (tree p)
          set_uids_in_ptset (p, finished_solution, vi->solution,
-                            vi->no_tbaa_pruning);
+                            true);/*vi->no_tbaa_pruning);*/
          result = shared_bitmap_lookup (finished_solution);
          if (!result)

we don't warn either, possibly because the machinery in tree-ssa-alias-warnings.c does not handle pointers to locals(?)
Comment 6 Richard Biener 2007-12-07 23:57:42 UTC
First we hit

/* XXX: don't get into structures for now.  It brings much             complication
     and little benefit.  */
if (struct_class_union_p (ptr_type) || struct_class_union_p 
  return false;

and then if you fix that

  /* If they are not SSA names, but contain SSA names, drop the warning
     because it cannot be displayed well.
     Also drop it if they both contain artificials.
     XXX: this is a hack, must figure out a better way to display them.  */
  if (filter_artificials)
    if ((find_first_artificial_name (get_ssa_base (object1))
         && find_first_artificial_name (get_ssa_base (object2)))
        || (TREE_CODE (object1) != SSA_NAME
            && contains_node_type_p (object1, SSA_NAME))
        || (TREE_CODE (object2) != SSA_NAME
            && contains_node_type_p (object2, SSA_NAME)))
      return false;

Comment 7 Richard Biener 2007-12-08 00:03:24 UTC
Because the temporary object D.2360 is indeed artificial.  If you disable that
filtering you get (finally)

t.ii: In function 'const QString& staticQString(const QString&)':
t.ii:110: warning: likely type-punning may break strict-aliasing rules: object '*{unknown}' of main type 'QHash<QString, bool>::const_iterator' is referenced at or around t.ii:110 and may be aliased to object '{unknown}' of main type 'QHash<QString, bool>::iterator' which is referenced at or around t.ii:95.

For reference:

Index: tree-ssa-alias-warnings.c
--- tree-ssa-alias-warnings.c   (revision 130692)
+++ tree-ssa-alias-warnings.c   (working copy)
@@ -866,11 +866,6 @@ nonstandard_alias_p (tree ptr, tree alia
   if (var_ann (get_ssa_base (alias))->escape_mask != NO_ESCAPE)
     return false;
-  /* XXX: don't get into structures for now.  It brings much complication
-     and little benefit.  */
-  if (struct_class_union_p (ptr_type) || struct_class_union_p (alias_type))
-    return false;
   /* If they are both SSA names of artificials, let it go, the warning
      is too confusing.  */
   if (find_first_artificial_name (ptr) && find_first_artificial_name (alias))
@@ -923,7 +918,7 @@ dsa_named_for (tree ptr)
              if (nonstandard_alias_p (ptr, alias, false))
                strict_aliasing_warn (SSA_NAME_DEF_STMT (ptr),
-                                     ptr, true, alias, false, true);
+                                     ptr, true, alias, false, false);
Comment 8 Jakub Jelinek 2007-12-31 14:06:42 UTC
*** Bug 34570 has been marked as a duplicate of this bug. ***
Comment 9 Richard Biener 2008-12-04 16:34:06 UTC
Note that the trunk still doesn't warn because we transform the invalid access
to a valid one by doing

  node.11_19 = (struct Node *) D.2454_18;
  D.2456.i = node.11_19;
  D.2457_20 = VIEW_CONVERT_EXPR<const struct const_iterator>(D.2456).i;

instead.  Disabling forwprop yields

./cc1plus -quiet t.ii -m32 -Wall -O2
t.ii: In function 'const QString& staticQString(const QString&)':
t.ii:110: warning: dereferencing pointer 'o.10' does break strict-aliasing rules
t.ii:110: note: initialized from here