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.

Sign up for GitHub

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

Jump to bottom

A couple ShmBacking.* tests failing on PowerPC #2774

Closed
Saviq opened this issue Jan 10, 2023 · 1 comment · Fixed by #2806
Closed

A couple ShmBacking.* tests failing on PowerPC #2774

Saviq opened this issue Jan 10, 2023 · 1 comment · Fixed by #2806
Assignees
@RAOF
Labels
bug

Comments

@Saviq
Copy link
Collaborator

Saviq commented Jan 10, 2023

Seen e.g. in PPA:

[ RUN ] ShmBacking.access_fault_is_true_after_invaild_read                                                                                                                                                  
/root/mir-2.10.0+dev147-g0593df6a8f/tests/unit-tests/test_shm_backing.cpp:310: Failure      
Value of: map->access_fault()                                                                                                                                                                               
Actual: false                                                                                         
Expected: true                                                                                                                                                                                              
[ FAILED ] ShmBacking.access_fault_is_true_after_invaild_read (1 ms)
[ RUN ] ShmBacking.access_fault_is_true_after_invaild_write                                                                                                                                                 
/root/mir-2.10.0+dev147-g0593df6a8f/tests/unit-tests/test_shm_backing.cpp:328: Failure
Value of: map->access_fault()                                                                                                                                                                               
Actual: false                                                                                         
Expected: true                                                                                                                                                                                              
[ FAILED ] ShmBacking.access_fault_is_true_after_invaild_write (0 ms)

These are new from #2744.
@Saviq Saviq added the bug label Jan 10, 2023
@Saviq Saviq closed this as completed Jan 19, 2023
@Saviq Saviq closed this as not planned Won't fix, can't repro, duplicate, stale Jan 19, 2023
@Saviq Saviq reopened this Jan 19, 2023
@Saviq
Copy link
Collaborator Author

Saviq commented Jan 19, 2023

I thought this could've been caused by -O3 (see canonical/wlcs#263), but setting -O2 unfortunately didn't help.

@Saviq Saviq mentioned this issue Jan 20, 2023
Closed
@RAOF RAOF mentioned this issue Jan 23, 2023
Merged
bors bot added a commit that referenced this issue Jan 23, 2023
@bors @RAOF
Merge #2806
d0c43ca 
2806: tests/ShmBacking: Improve invalid access tests. r=Saviq a=RAOF

So, `mmap` will provide a mapping *of pages*, which means its going to be a multiple of `PAGE_SIZE`, even if the backing file is not a-multiple-of-`PAGE_SIZE` big. It zero-pads the end up to the next page, *and accesses to this zero-initialised region do not fault*.

Now, it turns out I picked `4000` as the backing size, which is close enough to the `PAGE_SIZE` of `4096` on *most* kernels Ubuntu builds that doubling it puts you in a separate page, so these tests pass on all systems with a 4K `PAGE_SIZE`.

ppc64el has a 64K `PAGE_SIZE` 😠

Fix this by querying `sysconf(_SC_PAGE_SIZE)` and making our backing file *exactly* one page in size. Combined with `mmap` being specified to map on a page-aligned address (which is really the only thing it could do), that guarantees that *any* access outside the valid range is going to hit a different page, and hence fault.

Fixes: #2774

Co-authored-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
bors bot added a commit that referenced this issue Jan 23, 2023
@bors @RAOF
Merge #2806
197108d 
2806: tests/ShmBacking: Improve invalid access tests. r=Saviq a=RAOF

So, `mmap` will provide a mapping *of pages*, which means its going to be a multiple of `PAGE_SIZE`, even if the backing file is not a-multiple-of-`PAGE_SIZE` big. It zero-pads the end up to the next page, *and accesses to this zero-initialised region do not fault*.

Now, it turns out I picked `4000` as the backing size, which is close enough to the `PAGE_SIZE` of `4096` on *most* kernels Ubuntu builds that doubling it puts you in a separate page, so these tests pass on all systems with a 4K `PAGE_SIZE`.

ppc64el has a 64K `PAGE_SIZE` 😠

Fix this by querying `sysconf(_SC_PAGE_SIZE)` and making our backing file *exactly* one page in size. Combined with `mmap` being specified to map on a page-aligned address (which is really the only thing it could do), that guarantees that *any* access outside the valid range is going to hit a different page, and hence fault.

Fixes: #2774

Co-authored-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
@bors bors bot closed this as completed in 1ca9db1 Jan 23, 2023
Saviq pushed a commit that referenced this issue Feb 23, 2023
@bors @RAOF @Saviq
Merge #2806
0873886 
2806: tests/ShmBacking: Improve invalid access tests. r=Saviq a=RAOF

So, `mmap` will provide a mapping *of pages*, which means its going to be a multiple of `PAGE_SIZE`, even if the backing file is not a-multiple-of-`PAGE_SIZE` big. It zero-pads the end up to the next page, *and accesses to this zero-initialised region do not fault*.

Now, it turns out I picked `4000` as the backing size, which is close enough to the `PAGE_SIZE` of `4096` on *most* kernels Ubuntu builds that doubling it puts you in a separate page, so these tests pass on all systems with a 4K `PAGE_SIZE`.

ppc64el has a 64K `PAGE_SIZE` 😠

Fix this by querying `sysconf(_SC_PAGE_SIZE)` and making our backing file *exactly* one page in size. Combined with `mmap` being specified to map on a page-aligned address (which is really the only thing it could do), that guarantees that *any* access outside the valid range is going to hit a different page, and hence fault.

Fixes: #2774

Co-authored-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RAOF RAOF

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

tests/ShmBacking: Improve invalid access tests. canonical/mir
2 participants
@Saviq @RAOF