K_SPINLOCK() on single core SOC fails to build with optimization -Og

[tswaehn]

I actually do not understand and may need a quick guess, where to look.

this compiles without errors

	uint32_t count;

	uint32_t irq = arch_irq_lock();
	count = test();
	arch_irq_unlock(irq);
	printf("incrementing %d\n", count++);

but this:

	uint32_t count;

	struct k_spinlock my_lock;

	K_SPINLOCK(&my_lock) {
		count = test();
	}

	printf("incrementing %d\n", count++);

fails - as far as I understand that should be the same as above.

error: 'count' may be used uninitialized

if you wonder, why I am asking, its about this here:

zephyr/drivers/serial/uart_emul.c

Line 922 in 8b8b0a0

K_SPINLOCK(&drv_data->rx_lock) {

[tswaehn]

It turned out hat compiling with -Og produced the bug above, where as -Os is fine. actually I dont understand.

replication procedure:

if you want to test yourself, just add

zephyr_compile_options(-Werror -Og)

to the CMakeLists.txt of your main project, then clear the cmake cache and rebuild.

(and paste the above code to your main.c)

[tswaehn]

Actually, shouldn't it be irrelavant which optimization level ?

[peter-mitsis]

So far, I have been unable to reproduce this. Which compiler (and version) are you using? Also, what board are you building for?

Thinking aloud ... at its core, K_SPINLOCK is a for-loop construct. If the compiler is generating the warning you described, my initial thoughts are that it is determining that the body of the for-loop might not execute. For that to occur, its local spinlock key __i.key would have a non-initialized value which could then potential be non-zero. However, the way it is constructed, it should have an initial value of 0.

This makes me think that either ...
a) my understanding of how the spinlock key structure is initialized is flawed, or
b) there is a problem in the toolchain being used.

[tswaehn]

Using RISCV toolchain for qemu from latest zephyr 3.7.99

board: qemu_riscv32/qemu_virt_riscv32

CMakeLists.txt added this at the very bottom

zephyr_compile_options(-Werror -Og)

my main looks like this:

int32_t test(){
  return 42;
}

int main() {
  uint32_t count;

  struct k_spinlock my_lock;

  K_SPINLOCK(&my_lock) {
    count = test();
  }

  printf("incrementing %d\n", count++);

  while(1) {
  }
}

.../src/main.c: In function 'main':
error: 'count' may be used uninitialized 

@peter-mitsis just for the full picture. I agree, that for some reason the compiler seems a little distracted and assumes there is an option where this can be bypassed. however when actually executing and debugging. I never saw it uninitialized. so the real question is why is gcc so cautious about the for loop prediction based on -Og vs -Os

plus there is still an option that I am completely off. and doing stupid things without noticing. so a confirmation from your side is needed to prove, there is something or not.

[andyross]

Likely[1] this is going to be down to -Og disabling whatever pass was folding/eliding/removing the loop test, so it doesn't know for sure that the loop will execute at least once, thus the assignment to count inside the K_SPINLOCK() can't be proven to be reached, thus the warning.

Obviously the workaround for the specific code in question is just to initialize count at declaration time so the warning can't happen. That may not be acceptable in all cases, but it's at least something to get you unblocked.

Beyond that... I guess my gut says this isn't worth fixing unless we want to enable a "debug build" at that optimization level as an official feature. The workaround is trivial.

But if we do, the form would be to refactor the test expression so that it's always guaranteed to be true the first time it's called.

[1] The other possibility is a toolchain bug, but that's pretty rare.

[peter-mitsis]

Additional info:

A little extra digging suggests that the reported behavior was introduced somewhere in GCC version 12, as I am not seeing it on GCC version 11. Furthermore, I am not seeing this behavior on any optimization level except -Og (I tried using O0, O1, O2, O3, Oz, Os and Og).

I am thinking that this is a toolchain issue, as it makes little sense to me as to why only that optimization should generate the warning/error. In the meantime, my recommendation would be to employ the suggested workaround.

[andyross]

Yeah, agreed. I'm going to WONTFIX this. Please reopen with impact details if that's not acceptable.

[tswaehn]

it took me some time to figure out that the issue comes from compiling with -Og. so just to avoid anybody else wastes a lot of time (like me). is it possible to add something to cmake => "please dont use -Og, there might be issues with the riscv toolchain" ?