Bug 70143 - [6 Regression] false strict-aliasing warning
Summary: [6 Regression] false strict-aliasing warning
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: 6.0
Assignee: Richard Biener
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2016-03-08 20:44 UTC by Jan Kratochvil
Modified: 2016-03-09 15:57 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-03-09 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Kratochvil 2016-03-08 20:44:52 UTC
struct a {
  int i;
};
struct b {
  struct a a;
  int j;
};
int main(void) {
  static struct b b;
  struct a *ap=(struct a *)&b;
  return ((struct b *)&ap->i)->j;
}
-------------------------------------------------------------------------------
-Wall -O2
aliasing.c: In function ‘main’:
aliasing.c:11:19: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
   return ((struct b *)&ap->i)->j;
                   ^
FAIL:
gcc (GCC) 6.0.0 20160308 (experimental)
gcc-6.0.0-0.15.fc24
gcc-6.0.0-0.15.fc25
PASS:
gcc-6.0.0-0.14.fc24
gcc-5.3.1-2.fc23.x86_64

Pedro Alves said it is a GCC Bug so I am filing it here.
https://sourceware.org/ml/binutils/2016-03/msg00120.html
Comment 1 Jakub Jelinek 2016-03-09 07:13:47 UTC
Started with r233961.
Comment 2 Richard Biener 2016-03-09 10:00:30 UTC
The warning is "correct".  You are accessing object *ap (a struct a) via a
pointer to struct b.  That ap really points to a b is something the FE code
doesn't know at this point.

I'll see if we can somehow make the FE code "smarter".
Comment 3 Jakub Jelinek 2016-03-09 10:12:53 UTC
Note even GCC 5.x and earlier warn here with -Wstrict-aliasing=2 or -Wstrict-aliasing=1, it is just whether we warn about this with the most common one -Wstrict-aliasing=3.
Comment 4 Pedro Alves 2016-03-09 10:16:28 UTC
> The warning is "correct".  You are accessing object *ap (a struct a) via a
> pointer to struct b.  

I'd think that instead, we are accessing object "*&ap->i", an int, via a pointer 
to struct b, and I'd imagine that the problem is that the frontend doesn't know 
that struct *b and int * can alias, because the first field of 'struct a', which 
in turn is the first field of 'struct b', is an int.

So I don't see how you can call this correct?

Is this really just a warning problem, or does the compiler really think that struct *b and int * cannot alias?

> I'll see if we can somehow make the FE code "smarter".

I imagine that the frontend misses recursing through the first field of the first field of a struct, when adding valid-alias types.
Comment 5 rguenther@suse.de 2016-03-09 10:31:02 UTC
On Wed, 9 Mar 2016, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70143
> 
> --- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Note even GCC 5.x and earlier warn here with -Wstrict-aliasing=2 or
> -Wstrict-aliasing=1, it is just whether we warn about this with the most common
> one -Wstrict-aliasing=3.

Sure.  I can trivially re-instantiate previous behavior with

Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c     (revision 234025)
+++ gcc/c-family/c-common.c     (working copy)
@@ -1568,7 +1568,9 @@ strict_aliasing_warning (tree otype, tre
           alias_set_type set2 = get_alias_set (TREE_TYPE (type));
 
           if (set1 != set2 && set2 != 0
-             && (set1 == 0 || !alias_set_subset_of (set2, set1)))
+             && (set1 == 0
+                 || (!alias_set_subset_of (set2, set1)
+                     && !alias_set_subset_of (set1, set2))))
            {
              warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
                       "pointer will break strict-aliasing rules");

or with the suggested && !alias_sets_conflict_p (set1, set2).

Though that will FAIL the gcc.dg/Wstrict-aliasing-struct-member.c
I added.

Note that we then also don't warn for

struct a {
  int i;
  int k;
};
struct b {
  struct a a;
  int j;
};
int main(void) {
  static struct b b;
  struct a *ap=(struct a *)&b;
  return ((struct b *)&ap->k)->j;
}

then either.  Doing this kind of warnings strictly based on alias
sets only is going to be "broken" - though I consider our strict-aliasing
warnings broken anyway (and I don't think we can ever implement something
sensible :/).
Comment 6 Jakub Jelinek 2016-03-09 10:41:21 UTC
(In reply to rguenther@suse.de from comment #5)
> Though that will FAIL the gcc.dg/Wstrict-aliasing-struct-member.c
> I added.
> 
> Note that we then also don't warn for
> 
> struct a {
>   int i;
>   int k;
> };
> struct b {
>   struct a a;
>   int j;
> };
> int main(void) {
>   static struct b b;
>   struct a *ap=(struct a *)&b;
>   return ((struct b *)&ap->k)->j;
> }
> 
> then either.  Doing this kind of warnings strictly based on alias
> sets only is going to be "broken" - though I consider our strict-aliasing
> warnings broken anyway (and I don't think we can ever implement something
> sensible :/).

The point of -Wstrict-aliasing=3 is to give very few false positives and still catch lots of likely bugs.  So, if there is INDIRECT_REF or *MEM_REF involved and you can't analyze what it points to, we should err on the side that it might be valid, people can still use -Wstrict-aliasing=2 that will warn even about these.
Comment 7 rguenther@suse.de 2016-03-09 10:44:58 UTC
On Wed, 9 Mar 2016, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70143
> 
> --- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to rguenther@suse.de from comment #5)
> > Though that will FAIL the gcc.dg/Wstrict-aliasing-struct-member.c
> > I added.
> > 
> > Note that we then also don't warn for
> > 
> > struct a {
> >   int i;
> >   int k;
> > };
> > struct b {
> >   struct a a;
> >   int j;
> > };
> > int main(void) {
> >   static struct b b;
> >   struct a *ap=(struct a *)&b;
> >   return ((struct b *)&ap->k)->j;
> > }
> > 
> > then either.  Doing this kind of warnings strictly based on alias
> > sets only is going to be "broken" - though I consider our strict-aliasing
> > warnings broken anyway (and I don't think we can ever implement something
> > sensible :/).
> 
> The point of -Wstrict-aliasing=3 is to give very few false positives and still
> catch lots of likely bugs.  So, if there is INDIRECT_REF or *MEM_REF involved
> and you can't analyze what it points to, we should err on the side that it
> might be valid, people can still use -Wstrict-aliasing=2 that will warn even
> about these.

Ok, I'll remove the new test and test

Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c     (revision 234025)
+++ gcc/c-family/c-common.c     (working copy)
@@ -1568,7 +1568,9 @@ strict_aliasing_warning (tree otype, tre
           alias_set_type set2 = get_alias_set (TREE_TYPE (type));
 
           if (set1 != set2 && set2 != 0
-             && (set1 == 0 || !alias_set_subset_of (set2, set1)))
+             && (set1 == 0
+                 || (!alias_set_subset_of (set2, set1)
+                     && !alias_sets_conflict_p (set1, set2))))
            {
              warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
                       "pointer will break strict-aliasing rules");

then.
Comment 8 Pedro Alves 2016-03-09 10:49:12 UTC
> Note that we then also don't warn for
(...)
> struct a { int i; int k; };
> return ((struct b *)&ap->k)->j;
(...)
> then either

I see it the same as:

 int *ip = &ap->k;
 ((struct b *)ip)->j;

Fine from type alias perspective, but undefined for other reasons.

> Doing this kind of warnings strictly based on alias
> sets only is going to be "broken"

Agreed -- a warning for this case should probably be based on
layout / determining that k is not the initial field of struct a,
and thus this is undefined behavior.
Comment 9 Jakub Jelinek 2016-03-09 10:53:47 UTC
(In reply to Pedro Alves from comment #8)
> I see it the same as:
> 
>  int *ip = &ap->k;
>  ((struct b *)ip)->j;

That is certainly not fine from aliasing perspective, aliasing is not just about the type of the field you access, but the whole access path, so if you
use ((struct b *)ip)->j then ip should point to an object with effective type of struct b.
But, I'm afraid we can't warn about this for -Wstrict-aliasing=3, because that would lead to too many false positives.
Comment 10 rguenther@suse.de 2016-03-09 10:57:33 UTC
On Wed, 9 Mar 2016, palves at redhat dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70143
> 
> --- Comment #4 from Pedro Alves <palves at redhat dot com> ---
> > The warning is "correct".  You are accessing object *ap (a struct a) via a
> > pointer to struct b.  
> 
> I'd think that instead, we are accessing object "*&ap->i", an int, via a
> pointer 
> to struct b, and I'd imagine that the problem is that the frontend doesn't know 
> that struct *b and int * can alias, because the first field of 'struct a',
> which 
> in turn is the first field of 'struct b', is an int.
> 
> So I don't see how you can call this correct?
> 
> Is this really just a warning problem, or does the compiler really think that
> struct *b and int * cannot alias?

They can alias.  But 'struct *b' and an 'int' object can't.  Well.  At
least if the dynamic type of the 'int' object is still 'int'.  See
tree-ssa-alias.c:indirect_ref_may_alias_decl_p

  /* When we are trying to disambiguate an access with a pointer 
dereference
     as base versus one with a decl as base we can use both the size
     of the decl and its dynamic type for extra disambiguation.
     ???  We do not know anything about the dynamic type of the decl
     other than that its alias-set contains base2_alias_set as a subset
     which does not help us here.  */
  /* As we know nothing useful about the dynamic type of the decl just
     use the usual conflict check rather than a subset test.
     ???  We could introduce -fvery-strict-aliasing when the language
     does not allow decls to have a dynamic type that differs from their
     static type.  Then we can check
     !alias_set_subset_of (base1_alias_set, base2_alias_set) instead.  */
  if (base1_alias_set != base2_alias_set
      && !alias_sets_conflict_p (base1_alias_set, base2_alias_set))
    return false;
Comment 11 Pedro Alves 2016-03-09 11:38:10 UTC
> That is certainly not fine from aliasing perspective, aliasing is not just 
> about the type of the field you access, but the whole access path, so if you
> use ((struct b *)ip)->j then ip should point to an object with effective type 
> of struct b.

Sorry, bad choice of words (and I'm not that familiar with gcc internals, as you'll have guessed).  I meant "pointer type conversion" perspective.  IOW, this particular particular ip can never point to a struct b, but not because 
the pointers' static types are incompatible.

> use ((struct b *)ip)->j then ip should point to an object with effective type of struct b.

> But, I'm afraid we can't warn about this for -Wstrict-aliasing=3, because that
> would lead to too many false positives.

I see three distinct cases:

#1 - 'ip' has dynamic type 'int *' (can also be some "struct *" that has int as first field)

 extern int *ip;
 ((struct b *)ip)->j;

#2 - 'ip' has dynamic type "final" 'int *'

 static struct b b;
 struct a *ap=(struct a *)&b;
 int *ip = &ap->k;
 ((struct b *)ip)->j;

#3 - 'ip' has dynamic type "final" 'struct b *'.

 static struct b b;
 struct a *ap=(struct a *)&b;
 int *ip = &ap->i;
 ((struct b *)ip)->j;

Sounds like gcc doesn't distinguish #1 from #2.  IOW, it's missing some kind of tracking whether the dynamic type is "final"?
Comment 12 Richard Biener 2016-03-09 14:01:47 UTC
Author: rguenth
Date: Wed Mar  9 14:01:16 2016
New Revision: 234084

URL: https://gcc.gnu.org/viewcvs?rev=234084&root=gcc&view=rev
Log:
2016-03-09  Richard Biener  <rguenther@suse.de>

	c-family/
	PR c/70143
	* c-common.c (strict_aliasing_warning): Add back
	alias_sets_conflict_p check.

	* gcc.dg/Wstrict-aliasing-bogus-upcast.c: New testcase.
	* gcc.dg/Wstrict-aliasing-struct-with-char-member.c: Likewise.
	* gcc.dg/Wstrict-aliasing-struct-member.c: Remove again.

Added:
    trunk/gcc/testsuite/gcc.dg/Wstrict-aliasing-bogus-upcast.c
    trunk/gcc/testsuite/gcc.dg/Wstrict-aliasing-struct-with-char-member.c
Removed:
    trunk/gcc/testsuite/gcc.dg/Wstrict-aliasing-struct-member.c
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/testsuite/ChangeLog
Comment 13 Richard Biener 2016-03-09 15:57:37 UTC
Fixed.