Add guard pages to test stacks [crashpad/crashpad : main]
Mark Mentovai (Gerrit)
Mark Mentovai added 1 comment![]( "Open in Gerrit")
DCHECK(stack_size % page_size == 0);This doesn't seem reasonable. Can you instead set stack_alloc_size to 2 pages plus stack_size rounded up to a full page if necessary?
Related details
- Joshua Peraza
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joshua Peraza (Gerrit)
Joshua Peraza added 1 comment![]( "Open in Gerrit")
DCHECK(stack_size % page_size == 0);This doesn't seem reasonable. Can you instead set stack_alloc_size to 2 pages plus stack_size rounded up to a full page if necessary?
If the stack size is not a multiple of the page size, then crashpad will still find more stack than is expected unless we also round up the expected stack size.
Related details
- Mark Mentovai
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Mark Mentovai voted and added 1 comment![]( "Open in Gerrit")
Votes added by Mark Mentovai
| Code-Review | +1 |
1 comment
DCHECK(stack_size % page_size == 0);Joshua PerazaThis doesn't seem reasonable. Can you instead set stack_alloc_size to 2 pages plus stack_size rounded up to a full page if necessary?
If the stack size is not a multiple of the page size, then crashpad will still find more stack than is expected unless we also round up the expected stack size.
If the stack size is not a multiple of the page size, then crashpad will still find more stack than is expected unless we also round up the expected stack size.
Good point, for the purpose of the test I guess this is fine.
I’d suggest `DCHECK_EQ(stack_size % page_size, 0u);` then.
Related details
- Joshua Peraza
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joshua Peraza (Gerrit)
Joshua Peraza voted and added 1 comment![]( "Open in Gerrit")
Votes added by Joshua Peraza
| Commit-Queue | +1 |
1 comment
Joshua PerazaThis doesn't seem reasonable. Can you instead set stack_alloc_size to 2 pages plus stack_size rounded up to a full page if necessary?
Mark MentovaiIf the stack size is not a multiple of the page size, then crashpad will still find more stack than is expected unless we also round up the expected stack size.
If the stack size is not a multiple of the page size, then crashpad will still find more stack than is expected unless we also round up the expected stack size.
Good point, for the purpose of the test I guess this is fine.
I’d suggest `DCHECK_EQ(stack_size % page_size, 0u);` then.
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joshua Peraza (Gerrit)
Joshua Peraza voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Crashpad LUCI CQ (Gerrit)
Crashpad LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: snapshot/linux/process_reader_linux_test.cc
Insertions: 1, Deletions: 1.
@@ -182,7 +182,7 @@
if (stack_size > 0) {
const size_t page_size = getpagesize();
- DCHECK(stack_size % page_size == 0);
+ DCHECK_EQ(stack_size % page_size, 0u);
size_t stack_alloc_size = 2 * page_size + stack_size;
ASSERT_TRUE(thread->stack.ResetMmap(nullptr,
```
Change information
Add guard pages to test stacks
- M snapshot/linux/process_reader_linux_test.cc
Code-Review: +1 by Mark Mentovai
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |