-
Notifications
You must be signed in to change notification settings - Fork 18
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
mem: add mprotect tests #292
Conversation
9af2b2a
to
0c0f8f7
Compare
0c0f8f7
to
c53ba48
Compare
140d703
to
4bd561a
Compare
4bd561a
to
1c9c822
Compare
1c9c822
to
1689639
Compare
1689639
to
3cb4110
Compare
Unit Test Results6 003 tests +1 5 362 ✔️ +1 30m 13s ⏱️ -40s Results for commit 329cb3a. ± Comparison against base commit 3905060. This pull request removes 7 and adds 8 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
3cb4110
to
0ad330a
Compare
JIRA: RTOS-666
0ad330a
to
329cb3a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding such simple tests, I think we will extend them in the future to provide better coverage for this function.
|
||
TEST(test_mprotect, test_mprotect_singlecore) | ||
{ | ||
unsigned char *area = mmap(NULL, page_size * PAGES, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be better to not set PROT_READ at the beginning to check whether read access protection has been set properly in the mprotect call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't work then as we require that the protection required by the mrpotect has to be lesser or equal than the one established by mmap
|
||
TEST_SETUP(test_mprotect) | ||
{ | ||
page_size = sysconf(_SC_PAGESIZE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could get it once - at the beginning of a test (for example in runner()), not before all test cases, or alternatively only within the test case body.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: I know that now we have only one test case, but when someone will add more of them it's easy to forget about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to the body
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
JIRA: RTOS-666
Description
Add tests to new mprotect syscall.
Demo PR showing that tests pass with kernel and libphoenix changes applied.
phoenix-rtos/phoenix-rtos-project#922
Motivation and Context
Types of changes
How Has This Been Tested?
Checklist:
Special treatment
mprotect: add syscall phoenix-rtos-kernel#475
Add mprotect syscall libphoenix#311