Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(357)

Issue 1959643004: Fix stack collection with size limit (Closed)

Created:
4 years, 7 months ago by lv
Modified:
4 years, 7 months ago
CC:
google-breakpad-dev_googlegroups.com
Base URL:
https://chromium.googlesource.com/breakpad/breakpad.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Fix stack collection with size limit src/client/linux/minidump_writer/minidump_writer.cc:273 obtains the stack info by calling GetStackInfo(). That method will return the stack base address, aligned to the bottom of the memory page that 'stack_pointer' is in. After that it will cap the size of the memory area to be copied into the minidump to 'max_stack_len', starting from the base address, if the caller requested so. This will be the case when collecting reduced stacks, as introduced by this change: https://breakpad.appspot.com/487002/ In such cases the caller will request 2048 bytes of memory. However GetStackInfo() will have aligned the base address to the page boundary, by default 4096 bytes. If the stack, which grows towards the base address from the top ends before the 2048 bytes of the first block, then we will not collect any useful part of the stack. As a fix we skip chunks of 'max_stack_len' bytes starting from the base address until the stack_pointer is actually contained in the chunk, which we will add to the minidump file. BUG=https://bugs.chromium.org/p/google-breakpad/issues/detail?id=695 R=ivanpe@chromium.org Committed: https://chromium.googlesource.com/breakpad/breakpad/+/f25a4112004efca7065068e87155757ef821d1e9

Patch Set 1 #

Total comments: 4

Patch Set 2 : Addressed review comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -0 lines) Patch
M src/client/linux/minidump_writer/minidump_writer.cc View 1 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
lv
I don't know how to pick reviewers. Could someone please have a look? Thanks, Lars
4 years, 7 months ago (2016-05-06 19:59:20 UTC) #1
lv
4 years, 7 months ago (2016-05-06 22:55:50 UTC) #3
Mark Mentovai
Ivan gave the LG on the original change that this updates, so +him.
4 years, 7 months ago (2016-05-16 17:30:33 UTC) #5
ivanpe
https://codereview.chromium.org/1959643004/diff/1/src/client/linux/minidump_writer/minidump_writer.cc File src/client/linux/minidump_writer/minidump_writer.cc (right): https://codereview.chromium.org/1959643004/diff/1/src/client/linux/minidump_writer/minidump_writer.cc#newcode280 src/client/linux/minidump_writer/minidump_writer.cc:280: while (int_stack + max_stack_len < stack_pointer) { Please, remove ...
4 years, 7 months ago (2016-05-17 01:03:10 UTC) #6
lv
Thanks for the review, please see the new patch. https://codereview.chromium.org/1959643004/diff/1/src/client/linux/minidump_writer/minidump_writer.cc File src/client/linux/minidump_writer/minidump_writer.cc (right): https://codereview.chromium.org/1959643004/diff/1/src/client/linux/minidump_writer/minidump_writer.cc#newcode280 src/client/linux/minidump_writer/minidump_writer.cc:280: ...
4 years, 7 months ago (2016-05-23 15:54:57 UTC) #7
ivanpe
On 2016/05/23 15:54:57, lv wrote: > Thanks for the review, please see the new patch. ...
4 years, 7 months ago (2016-05-23 17:37:01 UTC) #8
ivanpe
4 years, 7 months ago (2016-05-24 18:49:39 UTC) #10
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as
f25a4112004efca7065068e87155757ef821d1e9 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698