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

Understanding the mpu_stack_growth test #290

Open
brghena opened this issue Aug 15, 2022 · 4 comments
Open

Understanding the mpu_stack_growth test #290

brghena opened this issue Aug 15, 2022 · 4 comments
Labels
good first issue Good for newcomers

Comments

@brghena
Copy link
Contributor

brghena commented Aug 15, 2022

I'm not sure what this app is supposed to do and there's no README for it with expectations.

It fails very quickly on the Microbit (printout in grow_stack() only happens once), but I'm not sure that's a bug. I think the stack is going out-of-bounds somewhere in the printf() call, which makes it hard to predict what stack out-of-bounds is expected.

@brghena
Copy link
Contributor Author

brghena commented Aug 15, 2022

@ppannuto maybe you remember how this works?

@ppannuto
Copy link
Member

I'm not sure I have more description to give than what main says it does:

int main(void) {
  register uint32_t* sp asm ("sp");

  printf("\n[TEST] MPU Stack Growth\n");

  printf("This test should recursively add stack frames until exceeding\n");
  printf("the available stack space and triggering a fault\n\n");

  printf("  mem_start:           %p\n", tock_app_memory_begins_at());
  printf("  app_heap_break:      %p\n", sbrk(0));
  printf("  kernel_memory_break: %p\n", tock_app_grant_begins_at());
  printf("  mem_end:             %p\n", tock_app_memory_ends_at());
  printf("  stack pointer (ish): %p\n", sp);

  putchar('\n');

  grow_stack();
}

From Configuration.mk, our default is 0x800 stack:

Configuration.mk:STACK_SIZE       ?= 2048

And the app walks through that reasonably quickly: #define GROW_BY 0x100


This was an early test; I'm not sure it's the most useful as-written. Ditching printf would probably help, but I'm also not sure it's the most critical to refine

@brghena
Copy link
Contributor Author

brghena commented Aug 15, 2022

I guess I was really referring to the logic in grow_stack(). https://github.com/tock/libtock-c/blob/master/examples/tests/mpu_stack_growth/main.c#L26-L39

static void grow_stack(void) {
  register uint32_t* sp asm ("sp");

  uint32_t buffer[GROW_BY];
  printf("stack: %p - buffer: %p - at_least: 0x%4lx\n",
         sp, buffer, size_is_at_least);

  write_ptr(buffer);
  read_ptr(buffer);

  size_is_at_least += GROW_BY;

  grow_stack();
}

It implied to me that size_is_at_least was important and I thought the buffer and GROW_BY would somehow relate to where the error was. Which I guess is just wrong as long as you don't remember otherwise.

I guess the takeaway here is that as long as exceeding the available stack space occurs, this test passes. I think it should likely be deprecated for tests that under/over-run the stack.

@ppannuto
Copy link
Member

I have a vague memory of trying to do this with some precision, and reading blogs / etc that said it's actually really hard to control and report stack size reliably in C; this was some amalgamation of best-effort practices I found. I agree, it should be deprecated in favor of something more principled -- eventually 🤷🏼 .

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

No branches or pull requests

2 participants