(Solved) STM32CubeIDE ignoring optimization even when -O0 was specified?

[DavidAlfa]

I'm trying to diagnose a malloc nightware in STM32 CubeIDE.

Edit: This was nothing to do with malloc!
It was a debugging bug caused my mixing HAL libraries, see message 3!
The bug caused the code shown at the IDE to not match the code running at the CPU, or either it was doing something really weird, nothing made sense at all, stepping the code jumped between different completely random functions.

Removing the unused STM32xxx_HAL folder fixed it.


Every call to malloc/free also updates a global mallinfo() struct, so I can debug the memory usage in realtime.
I can allocate about 9KB in previous steps of the program and then free it. mallinfo reports 9KB used, then 9KB available.
Later, calling for ~2KB, everything fails and I get into the Hardfault handler.
Before allocation, mallinfo() reports 9KB free, going down to 7KB after it, meaning it's actually allocating that amount.
But malloc is returning 0xFFFFFFFF (The famous (void *)-1 ?), while others return a correct value (0x2000...., somehere in RAM).
The same code works in a 32F101 with less ram (10KB), but fails in a 32F072 with larger ram (16KB), there's no special assembly code else than few nops.

The code is too large and won't without optimizations, so I applied -Og globally (optimize for debugging), the disabled function optimization and inlining with gcc attributes.
I recall this worked last time I did it, but debugging this function keeps doing nosense behavior/jumping like when optimized, stepping any of these nops will randomly spawn the cursor at other functions/areas, so I'm unable to properly debug it.
Neither do pP appear on the variables scope.
It's driving me nuts! Any ideas?

Ultimately I disabled 70% of the code and ran the whole thing with -O0. Still doing the same thing!
Code enters the function, but the next steps will "travel" to a GPIO function, to HAL_GetTick, to the ADC...!

Code: [Select]

__attribute__ ((noinline)) __attribute__((optimize("O0")))
void allocTest(void) {

  uint32_t *pP;
  asm("nop");
  pP = malloc(sizeof(something_t));

  if( pP==NULL )
    asm("nop");
  else if( pP==(void*)-1 )
    asm("nop");
  else{
    asm("nop");
    free(pP);
   }
}


[DavidAlfa]

Fixed the mistery!
STM32F0xx and STM32F1xx libraries shouldn't coexist.
Although the F1xx folder was excluded from the build, something was still happening.
No errors or warnings, neither was it the culprit, seemed to only break code stepping/debugging, not the real thing.
After that, I was able to properly step it and find the issue.

The actual bug is described here.

[hans]

Very subtle. Will reply here to no derail that FW thread.

Perhaps a point of discussion: is it best-practice to use the + operator be used for integers only? I know it's very common to use it for pointer indexing as well, but it's effectively the same as the bracket operators when instantly dereferenced. Bracket operators only works on pointer types so would have thrown an error for this case.

Possibly making a local pointer of page address first, like uint32_t* pageAddressPtr = (uint32_t*) pageAddress, and then do the indexing (via brackets or addition) is perhaps another good practice idea. It avoids the one-liner trick where a subtle bracket change makes all the difference.

Not trying to demean anyone here. I know these kind of subtle bugs can be very frustrating and time consuming. They are right in front of you while you are blind. That's why I like best-practices to avoid these kinds of issues. The time I've lost in C++ to figure out why object references were lost when using the "auto" operator, is something I will never get back. Does that make 'auto' bad? Or should anyone just know 'auto&' also exists? Obviously you can get any code in a given format to work, but the path of least resistance and mental effort (now and future) is probably preferred.

[nctnico]

Indeed. I have made it a habit to only use arithmetic on void pointers or single byte type pointers. And after that, I cast the pointer to a specific type if necessary.

I still recall hunting down a similar problem in code I inherited that ran in a softcore inside an FPGA. I had to pull out the good old logic analyser to find it. I spend the better part of a week on finding & fixing that issue.

[DavidAlfa]

Just use pointers if you understand them, or don't use them at all, there's no such "use + only for integers".
That bug was made by a 3rd party, not me, otherwise I would have spotted it much faster.

Malloc was doing fine, it was the broken debugging showing all kind of garbage, so ignore all that.

[hans]

I know, it's not 1st party code, so it's hard to control their best practices or design flow. Unfortunately bugs get in everything.

But I don't agree with your first statement. I know how to solve sudoku's but that still means you can make a mistake or wrong assumption and ruin your whole solution attempt. It can take ages to backtrack that single mistake, and unless you use a live sudoku puzzle program that automatically highlights your errors, it will need to do so one some assumptions (such as using the solution, or automatic sanity checks).

That's why my suggestion of using + only on integers would be one such "sanity check". Sure if you know the bracket operator, you can also reverse the position of the ptr and idx, e.g. use a code style like "  i[pageAddressPtr] == 0xFFFFFFFF  ". It works, it's bug free, but it's bad code.

[DavidAlfa]

Can't agree, I know well the difference between these

Code: [Select]

address = 0x8000
(uint32_t*)address + 1   -> (uint32_t*)0x8004
(uint32_t*)(address + 1) -> (uint32_t*)0x8001
Anyways, the original code was using a pointer, wasn't bad, worked nice, so why change it.
I guess all programmers have specific tendencies/obessions .

Code: [Select]

  for (uint16_t i = 0; i < sizeof(Something)/(sizeof(uint32_t)); i++) {
    if( *dest++ != 0xFFFFFFFF){
      Flash_error();
    }
  }

Why is this considered bad code? Looks neat to me.

Code: [Select]

  for (uint16_t i = 0; i < sizeof(Something)/(sizeof(uint32_t)); i++) {
    if( dest[i] != 0xFFFFFFFF){
      Flash_error();
    }
  }

I find it more simple and easier to read than:

Code: [Select]

  for (uint32_t i = 0u; i < (numPages * FLASH_PAGE_SIZE / sizeof(int32_t)); i++) {
    if( *((uint32_t*)pageAddress + i) != 0xFFFFFFFF){
      Flash_error();
    }
  }
}