[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] -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

[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: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

[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: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