tests: mem_protect: fix two issues #28102
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
area: Kernel
area: Tests
andrewboie
force-pushed
the
slowly-going-insane
branch
from
f5f25ac to
196f74a
Compare
added 2 commits
September 6, 2020 15:58
The implementations of the test cases had the wrong prototype. The extern declarations (which were in a C file for some reason) were correct. I don't want to talk about the subtle code generation and stack corruption issues that emerged from this which at one point made me question my own sanity. Signed-off-by: Andrew Boie <[email protected]>
We try to invoke `ztest_test_pass()` from inside a fatal exception in a child thread. On SMP this can result in the next test case starting on another CPU, re-using the child thread before it has a chance to exit. Signed-off-by: Andrew Boie <[email protected]>
andrewboie
force-pushed
the
slowly-going-insane
branch
from
196f74a to
9c6db39
Compare
ceolin
approved these changes
Sep 8, 2020
|
@andrewboie This is labeled as bug. Is there an issue # we could link to? |
@jenmwms there is not, I root caused this when working on #26486 and #27533 If a bug were opened, I wouldn't have anything further to add than what is already in the PR description / patch commit messages |
no problem, just thought I should ask. Great eye to catch bugs early :) |
dcpleung
approved these changes
Sep 14, 2020
jenmwms
approved these changes
Sep 14, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes two very subtle problems with the mem_protect test.
The first is that this tests was calling
ztest_test_pass()out of a fatal exception handler for a child thread. On an SMP system, this could cause the next test case to immediately start on another CPU before the child thread self-exits. If the next test tried to recycle the child thread object, the kernel goes haywire. Fixes a very rare crash on x86_64 (and other SMP systems). This could simply be removed since the ztest thread is joining on the child threads anyway.The second fixes a problem where the implementations of the test functions had the wrong prototype. The extern prototypes in another C file (?? why not a header?) were correct (otherwise the setup of the test suite would have caught it) and we had a mismatch. This on x86 fooled the compiler into thinking the stack frame was laid out in a way that wasn't true if it tried to chain execution with
jmpto the last function in the test case instead of calling it and then issuingret, since x86 marshals arguments on the stack and not registers.Debugging the latter issue was the engineering equivalent of slowly going insane staring at assembly code and stack contents, wondering if the compiler was bugged