11

I've been reading up on the strict aliasing rules over the last week or so and ran into this article: Understanding C/C++ Strict Aliasing.

The article goes through several ways two swap the halves of a 32-bit integer, giving both good examples and ones that violate the strict aliasing rule. I'm having trouble understanding one of the examples, though.

This code is described as broken.

uint32_t
swaphalves(uint32_t a)
{
    a = (a >> 16) | (a << 16);
    return a;
}

The reason given is:

This version looks reasonable, but you don't know if the right and left sides of the | will each get the original version of a or if one of them will get the result of the other. There's no sequence point here, so we don't know anything about the order of operations here, and you may get different results from the same compiler using different levels of optimization.

I disagree. This code looks fine to me. There is only one write to a in the a = (a >> 16 | (a << 16); line, and I expect that both reads of a take place before that write. Further, there are no pointers or references and no incompatible types.

Am I missing a strict aliasing violation in this code, or is the article incorrect?

zmb
  • 7,605
  • 4
  • 40
  • 55
  • Is this in a single threaded scenario? – user703016 Jul 21 '14 at 16:03
  • That would be my assumption, though I don't think it's explicitly stated in the article. – zmb Jul 21 '14 at 16:04
  • @ParkYoung-Bae If not, it's broken for reasons entirely unrelated to strict aliasing and sequence points, and only having one read on the right hand side won't fix it. –  Jul 21 '14 at 16:04
  • 6
    This example is clearly incorrect. If this scenario was right, then you couldn't code at all. I mean, with the same argument `a = a + a` should not work, either. All in all: don't ever trust this consulting firm. – Kijewski Jul 21 '14 at 16:04
  • @delnan Absolutely, I'm just not finding anything that could make the above code faulty - other than race conditions. – user703016 Jul 21 '14 at 16:05
  • @ParkYoung-Bae Well there's always the option that the article OP refers to is wrong ;-) –  Jul 21 '14 at 16:06
  • 3
    The strict aliasing articles I reference in this [answer](http://stackoverflow.com/a/20956250/1708801) are way better sources. – Shafik Yaghmour Jul 21 '14 at 16:06
  • There's only UB if one of those is `<<=` or `>>=`, or potentially if `a` is signed, but then the UB is obvious. Also, we are talking about a local variable here. Threading doesn't matter. – T.C. Jul 21 '14 at 16:11

1 Answers1

4

There are no pointers and references anywhere in this code, so strict aliasing rules don't even enter the picture. And indeed the author invokes sequence points rather than strict aliasing to justify the assertion that it's undefined. However, it seems this reasoning is wrong and the code snippet has perfectly defined semantics. As Prasoon Saurav explains in more detail:

(§1.9/15) The value computations of the operands of an operator are sequenced before the value computation of the result of the operator.

So regarding the = operator, the evaluation of a and (a >> 16) | (a << 16) are sequenced before the assignment. Neither of those is problematic: Although its parts are all unsequenced relative to each other, no write to a remains that would need to be sequenced.

(Technically this raises the question of how the side effect of the assignment is sequenced w.r.t. its value computation, but I couldn't find anything on this. Presumably it's somewhere in the standard but I don't have a copy handy. I strongly suspect it's sequenced after the value computation for the reasons in the next paragraph.)

You could also apply common sense: The write to a needs to evaluate (a >> 16) | (a << 16) first to write the right value and hence it can't happen in the middle of that evaluation. Another issue with the article is that even if

uint32_t
swaphalves(uint32_t a)
{
    a = (a >> 16) | (a << 16);
    return a;
}

had undefined behavior due to sequence points,

uint32_t
swaphalves(uint32_t a)
{
    return (a >> 16) | (a << 16);
}

wouldn't (there are no writes to be sequenced) and hence the far more complicated versions (unions, memcpy) that take up most of the rest of the article are pointless.

Community
  • 1
  • 1