CODEWITHSUNDEEP
c++gccarduinoswitch-statementlanguage-lawyer
Adrian McCarthy
Adrian McCarthy

Reputation: 47982

Variable in switch case: UB or compiler bug?

I'm trying to determine whether I invoked undefined behavior or encountered a compiler error.

I was developing some code to interpret messages sent to an Arduino from an external component over a serial connection. Here's a simplified version of the member function I began with. [The Serial.println commands are the Arduino equivalent of printf debugging.]

void decodeMessage() {
  switch (getType()) {
    case 0x3A:
      Serial.println("foo message");
      break;
    case 0x3B:
      Serial.println("bar message");
      break;
    case 0x3C:
      Serial.println("zerz message");
      break;
    ... // and so on for 0x3D through 0x40
    case 0x41:
      Serial.println("ack message");
      break;
    default:
      Serial.println("unknown message type");
      break;
  }
}

This worked fine for all of the message types. Then I modified the case for 0x3B to also check some bits in the message's parameter:

    case 0x3B:
      Serial.println("bar message");
      const auto mask = getParam();
      if (mask & 0x01) Serial.println("bit 0 set");
      if (mask & 0x02) Serial.println("bit 1 set");
      break;

With this code substituted in for the original 0x3B case, everything worked except for the last message type (0x41, "ack"). It's as though the body of that case was gone. The default case continued to work, as did 0x3A through 0x40.

After many attempts to trying to figure out the cause of the problem, I realized that I'd introduced a const variable (mask) in the middle of a switch without scoping it to that particular case. When I added braces, it once again worked for all cases:

    case 0x3B: {
      Serial.println("bar message");
      const auto mask = getParam();
      if (mask & 0x01) Serial.println("bit 0 set");
      if (mask & 0x02) Serial.println("bit 1 set");
      break;
    }  // braces to limit scope of `mask`

Questions:

Upvotes: 2

Views: 330

Answers (1)

Michael Kenzel
Michael Kenzel

Reputation: 15951

I believe this code should have been ill-formed based on [stmt.dcl]/3:

It is possible to transfer into a block, but not in a way that bypasses declarations with initialization (including ones in conditions and init-statements). A program that jumps from a point where a variable with automatic storage duration is not in scope to a point where it is in scope is ill-formed unless the variable has vacuous initialization ([dcl.init]).

emphasis mine. Your variable mask does not have vacuous initialization. At least as far as my understanding goes, "ill-formed" implicitly requires a diagnostic, i.e., a standard-conforming compiler must produce an error message. So no undefined behavior, this should simply never have compiled.

Thus, I would say that the lack of a diagnostic here is definitely to be considered a compiler bug. Note, however, that none of the GCC versions that one can try on godbolt accept this code (and they go quite far back). It would seem that the Arduino IDE must be using a hopelessly outdated/broken version of GCC if it did indeed compile this code without a flinch…

example of proper compilers complaining about this

To fix the issue, simply wrap your variable in a block scope such that there's no possible control flow that could enter the scope in which the variable is declared without passing its declaration, as you have already discovered yourself. For example, turn this

void f(int x)
{
    switch (x)
    {
    case 1:
        const int y = 42;  // error
        break;

    case 2:
        break;
    }
}

into

void f(int x)
{
    switch (x)
    {
    case 1:
        {
            const int y = 42;  // OK
            break;
        }

    case 2:
        break;
    }
}

Upvotes: 7

Related Questions