Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack pointer not correctly initialized #17

Closed
uhi22 opened this issue May 13, 2024 · 6 comments
Closed

Stack pointer not correctly initialized #17

uhi22 opened this issue May 13, 2024 · 6 comments

Comments

@uhi22
Copy link
Owner

uhi22 commented May 13, 2024

Clara crashes when added the ISO schema. One suspect was a potential collision of the stack with the RAM. And bingo:
The stack pointer points "somewhere" into the middle of the available RAM, and collides with variables which are placed there.
A test function, called in the main() after the serial is available, reveals that the stack variables got strange addresses. It seems that the stack pointer is not initialized in the application, and maybe has just a "random" value left over by the bootloader.

void stackMeasurement_init(void) {
    /* According to the data sheet of the STM32F103RE (from https://www.st.com/en/microcontrollers-microprocessors/stm32f103re.html),
       the SRAM is from 0x2000'0000 until 0x2000'FFFF.
       In the map file, the stack correctly starts at the end of the RAM:
       0x0000000020010000                PROVIDE (_stack = (ORIGIN (ram) + LENGTH (ram)))
       But, the addresses of the local variables are  200047ac,  200047a8 and  200047a4, which is quite far away
       from the 2000ffff. Seems the stack pointer is wrongly initialized.
       
       In https://www.openstm32.org/forumthread7689 they propose to explicitely initialize the stack pointer:
          ldr sp, =_estack
       We try:
          __asm__ ("ldr sp,=_stack;"); 
       at the begin of the reset_handler()
       This changes the addresses of the local variables to  2000f7dc,  2000f7d8,  2000f7d4, which is much more plausible.
    */
    
    int a, b, c;
    a = 3;
    b = 7;
    c = 99;
    printf("stacktest %d %x\n", a, &a);
    printf("stacktest %d %x\n", b, &b);
    printf("stacktest %d %x\n", c, &c);
}

Proposed solution:
In the begin of the reset_handler(), add the described initialization of the stack pointer.
This is a change in the shared libopencm3.

Questions:

  • Is this understanding correct?
  • Are other projects affected by the same issue?
  • How to bring a fix into the lib?
@jsphuebner
Copy link
Collaborator

jsphuebner commented May 13, 2024

That is indeed very strange. The bootloader main() puts nothing on the stack so when leaving main it should be at the end of RAM.
We are using a forked libopencm3 anyway so we can just patch that

It just seems odd that the pre iso software would run like that as it also uses almost the entire RAM

@uhi22
Copy link
Owner Author

uhi22 commented May 14, 2024

Maybe the bootloader is designed for smaller RAM, and its end of RAM is 20004fff or something.

@jsphuebner
Copy link
Collaborator

Of course! The bootloader uses a linker file with 20k RAM
Remarkable that the software never crashed so far

@uhi22
Copy link
Owner Author

uhi22 commented May 15, 2024

Yes, seems it was a lot of luck. In the DIN-only software, the stack at 0x2000'4fff and below overlaps with the large exi decoder structure, which starts at 0x2000'21d8 and has a size of 0x63d8.

 .bss.dinDocDec
                0x00000000200021d8     0x63d8 obj/projectExiConnector.o
                0x00000000200021d8                dinDocDec

Most of this structure is not used while decoding, because the structure contains all possible elements, but in reality only less than the half is used (because Clara never decodes the own transmit messages, and the chargers does not fill all optional fields of the Claras receive messages). Seems this lead to never hurting something important on the stack.

@uhi22
Copy link
Owner Author

uhi22 commented May 15, 2024

For the iso-test branch, updated the libopencm3 to fix the issue: 875fe0b
For the main, still to be updated.

@jsphuebner
Copy link
Collaborator

Main also updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants