[Bug c/18050] -Wsequence-point reports false positives

[Bug c/18050] -Wsequence-point reports false positives

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2006-03-22 12:19 -------
Another one:

int foo(int i)
{
  i = ++i;
  return i;
}

I think the point is we should not warn for pre-increment, only post-increment.
Or can someone come up with a testcase that has undefined evaluation order just
by using pre-increment?  One with two pre-increments:

int foo(void)
{
 int i = 1;
 i = (++i == 2) + ++i;
 return i;
}

This is certainly undefined.  But with one pre-increment only?


-- 


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

[Bug c/18050] -Wsequence-point reports false positives

[joseph at codesourcery dot com]

------- Comment #7 from joseph at codesourcery dot com  2006-03-22 12:26 -------
Subject: Re:  -Wsequence-point reports false positives

On Wed, 22 Mar 2006, rguenth at gcc dot gnu dot org wrote:

>   i = ++i;

Modified twice between sequence points, so undefined behavior.

> I think the point is we should not warn for pre-increment, only post-increment.
> Or can someone come up with a testcase that has undefined evaluation order just
> by using pre-increment?  One with two pre-increments:

It's undefined behavior, not undefined evaluation order.  Pre-increment 
returns the new value, but that doesn't mean the new value is stored until 
the following sequence point.


-- 


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

[Bug c/18050] -Wsequence-point reports false positives

[rguenth at gcc dot gnu dot org]

------- Comment #8 from rguenth at gcc dot gnu dot org  2006-03-22 12:53 -------
Sure - but this doesn't matter in this case.  And

6.5.3.1 tells you

 "The expression ++E is equivalent to (E+=1)."

6.5.16 says

 "The side effect of updating the stored value of the left operand shall
  occur between the previous and the next sequence point."

For i = ++i; this means we have

 i = (i += 1);

where for i += 1 the next sequence point is the i = ... assigment?

Of course for the particular testcase the ordering of the two stores
does not matter.  Would int i=0; i = ++i + 1; be able to result in
i == 1?  I don't think so as per 6.5.16.


-- 


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

[Bug c/18050] -Wsequence-point reports false positives

[schwab at suse dot de]

------- Comment #9 from schwab at suse dot de  2006-03-22 13:08 -------
(In reply to comment #8)
>  i = (i += 1);
> 
> where for i += 1 the next sequence point is the i = ... assigment?

The next sequence point is the semicolon.

> Of course for the particular testcase the ordering of the two stores
> does not matter.  Would int i=0; i = ++i + 1; be able to result in
> i == 1?

Yes, the side effects of = and ++ can be performed in either order.


-- 


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

[Bug c/18050] -Wsequence-point reports false positives

[pinskia at gcc dot gnu dot org]

------- Comment #10 from pinskia at gcc dot gnu dot org  2008-08-27 19:44 -------
*** Bug 37259 has been marked as a duplicate of this bug. ***


-- 

pinskia at gcc dot gnu dot org changed:

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


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

[Bug c/18050] -Wsequence-point reports false positives

[manu at gcc dot gnu dot org]

------- Comment #11 from manu at gcc dot gnu dot org  2008-08-28 01:18 -------
Andrew, your patch seems to work, so what is the problem?


-- 


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

[Bug c/18050] -Wsequence-point reports false positives

[pinskia at gcc dot gnu dot org]

------- Comment #12 from pinskia at gcc dot gnu dot org  2008-08-28 01:22 -------
(In reply to comment #11)
> Andrew, your patch seems to work, so what is the problem?

I think we are still warning in too many places but I can't remember now, it
was almost 4 years ago and many stuff has changed.


-- 


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

[Bug c/18050] -Wsequence-point reports false positives

[manu at gcc dot gnu dot org]

------- Comment #13 from manu at gcc dot gnu dot org  2008-08-28 01:30 -------
(In reply to comment #12)
> 
> I think we are still warning in too many places but I can't remember now, it
> was almost 4 years ago and many stuff has changed.

Do you mind if I test it and try to make it work? For the tests in this PR it
seems to work correctly. If you can think of other tests, please post them
here.


-- 


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

[Bug c/18050] -Wsequence-point reports false positives

[manu at gcc dot gnu dot org]

------- Comment #14 from manu at gcc dot gnu dot org  2008-08-29 00:07 -------
Subject: Bug 18050

Author: manu
Date: Fri Aug 29 00:06:19 2008
New Revision: 139742

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=139742
Log:
2008-08-28  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
            Andrew Pinski  <pinskia@gcc.gnu.org>

        PR 18050
        * c-common.c (verify_tree): Fix handling of ADDR_EXPR.
testsuite/
        * gcc.dg/Wsequence-point-pr18050.c: New.
        * g++.dg/warn/Wsequence-point-pr18050.C: New.

Added:
    trunk/gcc/testsuite/g++.dg/warn/Wsequence-point-pr18050.C
    trunk/gcc/testsuite/gcc.dg/Wsequence-point-pr18050.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-common.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug c/18050] -Wsequence-point reports false positives

[manu at gcc dot gnu dot org]

------- Comment #15 from manu at gcc dot gnu dot org  2008-08-29 00:08 -------
Fixed in GCC 4.4

Thanks for the report.


-- 

manu at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.4.0


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

[Bug c/18050] New: -Wsequence-point reports false positives

[mitr at volny dot cz]

void f(int a, int *b) { *b = a; } void g(void) { int a = 5; f(++a, &a); }

with -Wall reports
t.c:1: warning: operation on `a' may be undefined.

This is with the following gcc versions:
[I realize they are all Red Hat versions, not official releases, but
I think that this happening on three different release branches
provides sufficient argument that this is very likely an upstream bug.]

Reading specs from /usr/lib/gcc-lib/i386-redhat-linux/3.3.3/specs
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
--infodir=/usr/share/info --enable-shared --enable-threads=posix
--disable-checking --disable-libunwind-exceptions --with-system-zlib
--enable-__cxa_atexit --host=i386-redhat-linux
Thread model: posix
gcc version 3.3.3 20040412 (Red Hat Linux 3.3.3-7)

and

Reading specs from /usr/lib/gcc/i386-redhat-linux/3.4.1/specs
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
--infodir=/usr/share/info --enable-shared --enable-threads=posix
--disable-checking --with-system-zlib --enable-__cxa_atexit
--disable-libunwind-exceptions --enable-java-awt=gtk --host=i386-redhat-linux
Thread model: posix
gcc version 3.4.1 20040831 (Red Hat 3.4.1-10)

and

Reading specs from /usr/lib/gcc/i386-redhat-linux/3.5.0/specs
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
--infodir=/usr/share/info --enable-shared --enable-threads=posix
--disable-checking --with-system-zlib --enable-__cxa_atexit
--disable-libunwind-exceptions --with-gxx-include-dir=/usr/include/c++/3.4.1
--enable-languages=c,c++,f99 --disable-libgcj --host=i386-redhat-linux
Thread model: posix
gcc version 3.5.0 20040818 (Red Hat 3.5.0-0.9)

-- 
           Summary: -Wsequence-point reports false positives
           Product: gcc
           Version: 3.3.3
            Status: UNCONFIRMED
          Severity: minor
          Priority: P2
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: mitr at volny dot cz
                CC: gcc-bugs at gcc dot gnu dot org


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

[Bug c/18050] -Wsequence-point reports false positives

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-10-18 22:01 -------
No the warning is correct. ++a could come before or after taking the address of a which is why this is 
undefined.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID


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

[Bug c/18050] -Wsequence-point reports false positives

[giovannibajo at libero dot it]

------- Additional Comments From giovannibajo at libero dot it  2004-10-18 22:15 -------
Uh? How can an increment operation change the address of a variable?

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|INVALID                     |


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

[Bug c/18050] -Wsequence-point reports false positives

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-10-18 22:25 -------
Oh, you are right I need to look at the test more closely.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
           Keywords|                            |diagnostic
   Last reconfirmed|0000-00-00 00:00:00         |2004-10-18 22:25:30
               date|                            |


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

[Bug c/18050] -Wsequence-point reports false positives

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-10-18 22:35 -------
We cannot just ignore ADDR_EXPR outright though (this is undefined):
struct x
{
  int i;
};
void g(struct x*, int *);
void f(struct x *y)
{
  g(y++, &y->i);
}

I think I have a fix will test the fix.

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


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

[Bug c/18050] -Wsequence-point reports false positives

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-10-18 22:45 -------
Actually the patch will not work when we start warning about full expression, here is the patch just to 
give an example of what the final patch would look like:
Index: c-common.c
===============================================================
====
RCS file: /cvs/gcc/gcc/gcc/c-common.c,v
retrieving revision 1.578
diff -u -p -r1.578 c-common.c
--- c-common.c	16 Oct 2004 22:58:45 -0000	1.578
+++ c-common.c	18 Oct 2004 22:42:30 -0000
@@ -1358,6 +1358,16 @@ verify_tree (tree x, struct tlist **pbef
 	add_tlist (pno_sp, t->cache_after_sp, NULL_TREE, 1);
 	return;
       }
+    
+    case ADDR_EXPR:
+      {
+	x = TREE_OPERAND (x, 0);
+	if (DECL_P (x))
+	  return;
+	writer = 0;
+	goto restart;
+      }
+      return;
 
     default:
       /* For other expressions, simply recurse on their operands.


Here is the test which I was taking about:
struct x
{
  int i;
};
void g(int, int *);
void f(struct x *y)
{
  g(y->i++, &y->i); /* {dg-bogus "undefined" } */
}

Here is another where we should warn:
struct x
{
  int i;
};
void g(int, int *);
void f(struct x *y)
{
  g((y++)->i, &y->i); /* { dg-warning "undefined" } */
}

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


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