rust-osdev/acpi

Call stack size

Closed this issue · 5 comments

Hello! I recently ran into a problem when using aml together with the Limine bootloader: calling parse_table on one CPU accidentally spoiled the page table of another CPU, leading to a crash on the latter CPU due to page fault.

Before I called parse_table, the stack pointer was something like

(lldb) p/x $rsp
(unsigned long) 0xffff80007dec2570

I set a watchpoint on the page table memory and inspected the stack pointer again when the watchpoint hit:

(lldb) p/x $rsp
(unsigned long) 0xffff80007deb1fd0

The difference was 66976 bytes in the middle of parsing, where the call stack included about 102 frames. The default stack size allocated by Limine is 64 KiB. If I double that size from Limine then everything works.

So any thoughts on the possibility of reducing the call stack size, or is it worth mentioning this somewhere? I'm not sure if someone else would use this crate in a test or resource-limited environment and trigger bizarre memory behaviour.

Feel free to close this.

Qix- commented

Is this in debug mode or release mode? If debug mode, do you see the same issue in release mode? Rust's debug mode adds a lot of stuff to the stack in debug mode that isn't present in release mode. That might be a contributing factor.

Did you also confirm via a debugger that the stack trace was in fact inside this library?

You can also ask limine for a larger stack size. If it's only present in debug mode, you can use something like the following:

#[cfg(debug_assertions)]
#[used]
static REQ_STKSZ: StackSizeRequest = StackSizeRequest::with_revision(0).with_size(16 * 1024 * 1024);

Hi - thanks for using the library! Some useful tips already by @Qix-; it's definitely worth checking if running in release helps, as this will often strip a large amount out of stack frames and also more aggressively inline functions which helps. In general, the approach we're using here is quite stack-heavy, particularly with complex AML tables, so I'm not surprised you need more stack than the average C kernel, to be honest.

It's something we should probably look at in the future to see if we can bring this usage down with some sensible forced inlining and reviewing how we create objects as we traverse the structure, but it's a relatively low priority and I have very limited time to work on the project myself atm unfortunately.

Another thing to mention is a general OSdev thing that will be useful as your kernel matures - it's very useful to be able to detect stack overflows without letting them corrupt other bits of memory, which as you've found can create strange bugs. A common way to do this is to leave a page or two purposefully unmapped at the end of your stack - you can test this address range in your page-fault handler and either extend the stack or give a better error message.

Thank you for the tips! @Qix-

Is this in debug mode or release mode?

It was in debug mode. I tried release mode too, with opt-level = 3 and debug = false in Cargo.toml but saw the same behaviour.

Did you also confirm via a debugger that the stack trace was in fact inside this library?

Yes, with LLDB. Like I mentioned in the issue, I set a watchpoint on the page table address and checked the call stack when the watchpoint hit. There was about 100 frames belonging to this crate.

You can also ask limine for a larger stack size.

Exactly. I tried that before opening this issue. With a 128-KiB stack everything works fine. (Probably a smaller value would work too.)

@IsaacWoods Thanks for the reply.

In general, the approach we're using here is quite stack-heavy

Yeah that makes sense. I understand that the parser is written with parser combinators so the stack use is kind of expected.

but it's a relatively low priority and I have very limited time to work on the project myself atm unfortunately.

No worries at all. I'm just curious if there's any planned future work on this. Temporarily increase stack use is acceptable to me - I can reclaim such memory later, once all the initialization has been done.

A common way to do this is to leave a page or two purposefully unmapped at the end of your stack

Yes, sentinel pages are what I have planned to add.

I'm closing this now given the replies. Thank you guys.