stacker - stack_area() returns incorrect size#133
Conversation
|
I see that |
| // TODO should we not pass `allocated_stack_size` here? | ||
| // TODO should we not pass `allocated_stack_size` here? | ||
| let panic = psm::on_stack(stack_base, requested_stack_size, move || { | ||
| std::panic::catch_unwind(std::panic::AssertUnwindSafe(callback)).err() |
There was a problem hiding this comment.
Using allocated_stack_size could result in a segfault if the actual size is smaller than is reported by stack_area(), and if the stack overflows into these guard pages.
…f requested_stack_size; results in fails
| assert_eq!( | ||
| stack.stack_area().1, | ||
| ((size + page_size() - 1) / page_size()) * page_size() | ||
| ) |
There was a problem hiding this comment.
Would you mind adjusting the test to also attempt a write to every byte of the allocated stack mapping?
There was a problem hiding this comment.
Sure! Good idea. I will also simplify that test to use div_ceil instead of that nasty looking operation there. Clippy checks would have caught that. I wish there was just a way to hook into git commit that ran clippy and fmt automatically prior to the commit being committed
There was a problem hiding this comment.
I updated the test, but:
cargo test --libprobably won't get ran by GitHub Actions, as the original test I wrote wasn't failing on Actions but would fail when I ran that command.- I am not aware of any arches/oses that have a "reversed" stack direction. We might want to run the test on one of those targets to make sure it still passes, but if it fails, it can probably be fixed by using
rust_psm_stack_direction(). But then again... this function is not directly used by stacker, so it might not be an issue. Copilot says that rust doesn't support any arches that use reversed stack directions, but I would trust your word over Copilot's.
There was a problem hiding this comment.
Also, I confirmed segfaults occur when using + 1, 16, 32, 1024 at the max end of the range in the test to see if the stack is any larger by chance. Used different numbers in case alignment was the issue.
2 participants
Hey, I was experimenting with your code when I found that the
stack_area()function includes an extra guard page in the usable size... unless I'm using it wrong? I believe that the stack area includes 2 guard pages and only 1 of them is subtracted. I have a test that fails, and switching it to subtract 2 guard pages fixes my test.Fixes #134