Bug 39121 - strange behavior in chained operations
Summary: strange behavior in chained operations
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: unknown
: P3 major
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-06 19:39 UTC by llpamies
Modified: 2015-08-10 03:38 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description llpamies 2009-02-06 19:39:51 UTC
#include <stdio.h>

/*
  Why the first swap operation works as expected but
  it does not happen the same with the second one ?
  I guess that it can be due operations within temp values,
  but IMHO swap operation should work in both cases.

  Thanks !
 */

void swap(int *a, int *b) {
    *a ^= *b ^= *a ^= *b;
}

int main() {
    int a = 5;
    int b = 8;
    printf("%d, %d\n", a, b);
    a ^= b ^= a ^= b;
    printf("%d, %d\n", a, b);
    swap(&a, &b);
    printf("%d, %d\n", a, b);
}
Comment 1 Andrew Pinski 2009-02-06 20:09:52 UTC
This is undefined code as you are modifying *a twice without a sequence point inbetween the modifies.

*** This bug has been marked as a duplicate of 15145 ***
Comment 2 llpamies 2009-02-06 21:07:27 UTC
Is not the same bug as #15145. I agree with you that there is just one sequence point, but the operation is not undefined.

void swap(int *a, int *b) {
    *a ^= *b ^= *a ^= *b;
}

This code should be compiled to:

*a = *a ^ *b;
*b = *b ^ *a;
*a = *a ^ *b;

And not to something like (I think that is what happens):

int tmp;
tmp = *a ^ *b;
*b = *b ^ tmp;
//On that point *a should contain 5^8 instead of the original value 5.
//This happens because the temp variable generated by the compiler.
*a = *a ^ *b;  

I think that the compiler is not translating properly what was written in the source code. Summarizing, I think that in:

y = 1;
x = (y += 1);

The execution order should be:

volatile_register <--- y + 1
y                 <--- volatile_register
x                 <--- volatile_register

instead of:

volatile_register <--- y + 1
x                 <--- volatile_register
y                 <--- volatile_register
Comment 3 Richard Biener 2009-02-06 21:21:58 UTC
Evaluation order is undefined if there is no sequence point.
Comment 4 joe.carnuccio 2015-06-25 18:49:01 UTC
I have found the following:

This works: c ^= d ^= c ^= d  (where c and d are not pointers)

This fails: *a ^= *b ^= *a ^= *b  (where a and b are pointers)

When compiling using -Os then the failed case now works.
Comment 5 joe.carnuccio 2015-06-25 18:51:30 UTC
Since using gcc -Os causes the correct execution, then "sequence point" does not have anything to do with it.
Comment 6 Andrew Pinski 2015-06-25 19:10:10 UTC
(In reply to joe.carnuccio from comment #5)
> Since using gcc -Os causes the correct execution, then "sequence point" does
> not have anything to do with it.

And you are wrong about that.  -Os causes what you think is the correct execution but there are multiple interpretations of the code because there are not sequence points there.
Comment 7 joe.carnuccio 2015-06-25 19:40:05 UTC
Ok, the sequence points are at each of the assignment operators.

The crux of this is that doing the xor chain with dereferenced pointers fails (incorrect execution), whereas doing it with variables works...

i.e. *a and *b are being treated differently than a and b;


a ^= b ^= a ^= b is supposed to do the following:

a = a ^ (b = b ^ (a = a ^ b))

from right-to-left each assignment is done in sequence (and has been verified to work correctly);


*a ^= *b ^= *a ^= *b should work the same way, but it does not (unless you compile with -Os).
Comment 8 Manuel López-Ibáñez 2015-06-25 20:08:16 UTC
(In reply to joe.carnuccio from comment #7)

> *a ^= *b ^= *a ^= *b should work the same way, but it does not (unless you
> compile with -Os).

https://gcc.gnu.org/wiki/FAQ#undefinedbut
Comment 9 frankhb1989 2015-08-10 03:38:43 UTC
This should work since C++11 because the rules of builtin assignment were modified (CWG 222; see also CWG 637). However, it is still undefined in C11, even if the new "sequenced before" wording has been copied from C++11 (WG21/N1944).
Not sure if any diagnostics should be changed.