Where does this sketch have Undefined Behavior?

Question

Simple sketch and simple question: where does this sketch have undefined behavior?

Note: I don't need a solution to fix the code. I already have code that works. I really need to understand where my thinking is wrong.

Background: some kids were writing a program for a line follower robot which has 3 digital sensors. Many of them bricked their Arduino. I reduced the affected sketches to quite a minimum so that it still reproduces the problem. The sketch is pretty self-explanatory I hope.

It gives no warnings in Arduino IDE 1.8.9, 1.8.15 and 2.0.4 even with all warnings enabled.

Still, this code bricks all the Arduino clones (9 instances of Elegoo Smart Robot Car Kit 3.0 Plus) and I spent several hours to find a reliable procedure to unbrick them. Therefore I have strong belief that this code must have undefined behavior - except that I can't see it.

The unbrick procedure is:

1. Remove the shield, if not removed yet
2. Disconnect Arduino from USB
3. Connect Arduino to USB
4. Press Reset button on the Arduino board
5. Disconnect Arduino from USB again (!)
6. Connect Arduino to USB
7. Compile and upload an empty sketch

long PIN_LINE_LEFT = 10;
long PIN_LINE_MID = 4;
long PIN_LINE_RIGHT = 2;
long DATARATE = 115200;

void setup() {
  pinMode(PIN_LINE_LEFT, INPUT);
  pinMode(PIN_LINE_MID, INPUT);
  pinMode(PIN_LINE_RIGHT, INPUT);
  Serial.begin(DATARATE);
}

void loop() {
  byte leftValue = digitalRead(PIN_LINE_LEFT);
  Serial.println(leftValue);
}

Here's my analysis:

IMHO, the sketch is unusual just in one way: instead of #defineing the pin numbers and other stuff, it declares them as global variables of type long.

According to the Arduino reference about data types a long is 32 bit. Printing sizeof(long) confirms that. Therefore, 115200 will not overflow at 65536 (16 Bit).

For Serial.begin(), the DATARATE will be converted into an unsigned long, which IMHO should not be a problem here, since the number is positive.

Sure enough, the pin numbers needn't be long and should be declared as uint8_t to better match the pinMode() and digitalRead() function. But I still think the implicit conversion down to uint8_t should be fine, since there is no overflow either.

Next, digitalRead() officially returns an int. The result is assigned to a byte which is an alias for uint8_t. It seems that this also gets converted implicitly. However, the values should only be HIGH or LOW, i.e. 1 or 0, which does not generate a signed overflow either.

What I tried:

Changing DATARATE to the more appropriate unsigned long DATARATE = 115200UL; doesn't change anything.

Changing byte leftValue = digitalRead(PIN_LINE_LEFT); to the more appropriate int leftValue = digitalRead(PIN_LINE_LEFT); also didn't change anything.

Changing long PIN_LINE_MID = 4; and all others to the more appropriate uint8_t PIN_LINE_xxx = xxx; also doesn't change anything.

IMHO with these changes, there's basically nothing left to fix and this should be a super valid C/C++ program without any implicit dangerous casts.

A Serial.flush(); in the loop is not needed according to the documentation of Serial.write(). So there will not be a buffer overflow.

Answer 1

Your sketch does not have undefined behaviour. It works:

I already have code that works.

Your problem apparently is that after loading this sketch you have trouble uploading a new sketch:

this code bricks all the Arduino clones

They aren't "bricked" as you say:

But your method works in ~9 out of 10 cases. I can't believe this.

Your sketch, which outputs repeatedly to the serial port, apparently makes it hard for a clone to respond to a new upload. The cheaper USB interface which the clones use, to save a few cents, evidently don't react as well as the genuine Arduino to attempts to upload a new sketch.

The suggested method in the comments (referencing my post Have I bricked my Arduino Uno? Problems with uploading to board) indicates the solution, and I quote:

Take these steps:

• Power off the board completely (remove the USB cable).
• Hold down the Reset button, and keep it held down (or, run a jumper wire from the RESET pin to the GND pin). This stops the problem sketch from starting, and thus activating the watchdog timer
• Still holding down Reset, reconnect the USB cable.
• Start uploading a sketch that does not have this problem (eg. Blink)
• Once the IDE reports "Uploading" release the Reset button (or remove the jumper wire).
• It should now upload OK - as the sketch which activated the watchdog timer never started.

The solution above is to not let the current sketch run at all, thus avoiding whatever-it-is that it does to stop new sketches from uploading.

The references to the watchdog timer are from the original post, they are not relevant here.

---

It gives no warnings in Arduino IDE 1.8.9, 1.8.15 and 2.0.4 even with all warnings enabled.

That's because there are no problems with the sketch per se. The hardware? Well, you buy cheaper hardware, you may get some problems. The technique described above helps you work around those problems.

Answer 2

With some USB to TTL Serial chips it is a problem to start the upload if there is too much input from the MCU on the Serial side. In this case it is the Holtek UART Bridge which is not common for Arduino boards.

To stop the sketch, hold the reset button until the upload from IDE starts. When the reset button is released, the bootloader starts and receives the upload.