Curious Conditional Conundrum

Projects Programming
1

Hi all,

Recently I've been working on a project that handles numbers composed of 16 32-bit (uint32_t) words. One of the requirements of the project is to be able to compare two such numbers to each other. To check if one number (a) is greater than or equal to another number (L), I wrote up the following quick and dirty test function:

bool greaterThanOrEqualTo(uint32_t* a) {
    unsigned short i = 0;
    while((a[i] >= L[i]) && (i < 16)) {
        i += 1;
    }

    Serial.println(i);

    if(i == 16) {
        Serial.println("true");
        return true;
    }

    Serial.println("false");
    return false;
}

It should be noted that the variable L is hard-coded elsewhere. At the end of this post is a copy of the entire .ino test file, as well as the serial print-out, and a corrected, working version of the above function for your reference.

Here's the rub: when this function is run with a = L, the serial print-out of i is 16. But, if(i == 16) fails, i.e. greaterThanOrEqualTo returns false when it should return true.

I don't know why this function does not work, but maybe one of you do, and I would love to hear why if so. I have a sneaking suspicion it might have something to do either with memory and / or with working within the Arduino environment directly. I suspect these for the following reasons:

  • If (a[i] >= L[i]) and (i < 16) are swapped within the while loop such that the while loop is: while((i < 16) && (a[i] >= L[i])), the function, interestingly, returns true. This suggests that if a[16] and L[16] are read (which is undefined because a and L are arrays of only size 16), somehow, this disrupts the later inequality if(i == 16).

  • I've had this code converted to C and executed outside of the Arduino IDE, and it ran as intended.

For your reference I am working on an Arduino Nano 33 IoT, and am using version 1.8.13 of the IDE. I might update it to the most recent version and see if that solves my issue, but I don't believe the issue lies there.

Here is a complete copy of the .ino file that I am working from:

#include "Arduino.h"
#include "HardwareSerial.h"

#include <stdint.h>


const uint32_t L[16] = {0x00001000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x000014de, 0x0000f9de, 0x0000a2f7, 0x00009cd6, 0x00005812, 0x0000631a, 0x00005cf5, 0x0000d3ed};

bool greaterThanOrEqualTo(uint32_t* a) { // WARNING: BROKEN!
	unsigned short i = 0;
	while((a[i] >= L[i]) && (i < 16)) {
		i += 1;
	}

	Serial.println(i);

	if(i == 16) {
		Serial.println("true");
		return true;
	}

	Serial.println("false");
	return false;
}


void setup() {
	Serial.begin(9600);
	while(!Serial) {
		delay(250);
	}

	uint32_t a[16] = {0x00001000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x000014de, 0x0000f9de, 0x0000a2f7, 0x00009cd6, 0x00005812, 0x0000631a, 0x00005cf5, 0x0000d3ed};

	if(greaterThanOrEqualTo(a)) {
		Serial.println("true");
	} else {
		Serial.println("false");
	}
}


void loop() {}

When the above .ino file is executed, the following serial print-out is given:

16
false
false

Here is a corrected, working version of the function:

bool greaterThanOrEqualTo(uint32_t* a) {
	unsigned short i = 0;
	while((i < 16) && (a[i] >= L[i])) {
		i += 1;
	}

	Serial.println(i);

	if(i == 16) {
		Serial.println("true");
		return true;
	}

	Serial.println("false");
	return false;
}

Thanks!

1 Like
2

I guess you should do it like this to avoid a[16] and L[16] being tested out of bounds:

while( (i < 16 ) && (a[i] >= L[i]) ) { . . .

C++ will not continue to evaluate the expression if an earlier part of it fails.

1 Like
3

Yup, thanks for the advice! I tried this and it worked. I guess at this point I am just curious why it failed before. In other words, why reading a[16] and L[16] would cause if(i == 16) to fail.

4

L has 16 elements, so an index i is only valid if 0 <= i < 16. Since you are reading L[i], the compiler is free to assume that the index is valid and satisfies 0 <= i < 16, otherwise the program would be invalid. Under this assumption, the check i == 16 can never evaluate to true, so the compiler is free to throw away the corresponding branch of the if statement, since it can never be executed in a valid program.

Accessing elements outside of the bounds of an array is undefined behavior, so that's what you're seeing here: when the optimizer's assumptions aren't met, you get strange results.

5

Gotcha, thanks for the explanation! I was wondering if maybe the compiler was throwing that if statement away, but wasn't sure.

6

I’m pleased you have an explanation you are happy with. Incidentally, it is bad form to go back and edit a post once you have replies to it. Such information should be in a new post. In this case, post #2 appears now to have copied a suggestion already made in the preceding post. Fortunately the edit history is visible.

7

That is good to know, I'll keep that in mind. And now that you mention it, I can see how that could cause misunderstanding. I in no way meant to make it look like post #2 was copied, so for that I apologize.

8

Interestingly, I tried it (on a Uno) to see if it gave any compiler warnings because of a possible out of range condition being detected at compile time. It did not.

What I did notice was:
(a) that the value 'i' could even get to 17 (which should be impossible !)
(b) If, however, 'i' is declared as global instead of automatic, it all appears to work correctly even with the invalid format of the while statement while ((a[i] >= L[i]) && (i < 16)) { . . .

Anyway, little more can be said than the behaviour of code which causes undefined operations will be unpredictable.

#include "Arduino.h"
// #include "HardwareSerial.h"
// #include <stdint.h>

char buf[80] = {0} ;
// unsigned short i = 0;  // if 'i' is global, it appears to work in all cases even with an invalid while condition

const uint32_t L[16] = {0x00001000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x000014de, 0x0000f9de, 0x0000a2f7, 0x00009cd6, 0x00005812, 0x0000631a, 0x00005cf5, 0x0000d3ed};

bool greaterThanOrEqualTo(uint32_t* a) { // WARNING: BROKEN!
  unsigned short i = 0;  // failure case visible only when 'i' is automatic.
  while ((a[i] >= L[i]) && (i < 16)) {  // invalid because "i" can reach 16 and cause an array read out of bounds
    // while( (i < 16) && (a[i] >= L[i]) ) {  // valid because i reaching 16 prevents the a[i] >= L[i] test being executed
    sprintf( buf, "i=%d, a=%lx, L=%lx \n", i, a[i], L[i] ) ;
    Serial.print( buf ) ;
    i += 1;
  }

  Serial.println(i);

  if (i == 16) {
    Serial.println("true");
    return true;
  }

  Serial.println("false");
  return false;
}


void setup() {
  Serial.begin(115200);
  while (!Serial) {
    delay(250);
  }

  uint32_t a[16] = {0x00001000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x000014de, 0x0000f9de, 0x0000a2f7, 0x00009cd6, 0x00005812, 0x0000631a, 0x00005cf5, 0x0000d3ed};

  if (greaterThanOrEqualTo(a)) {
    Serial.println("true");
  } else {
    Serial.println("false");
  }
}


void loop() {}
9

why not use for loop, it is safer and more transparent as to intention of the code

10

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.