Closed GoogleCodeExporter closed 9 years ago
I clicked enter too fast, so I was unable to enter all necessary information.
Basically, the estimation for an average stack size (of 8 KB) seems wrong. I
added some logging and it appears that when compiling and running this in
Google most of the stacks sizes are actually KB.
Changing the following constant from 8 to 12 KB seems to fix the issue, however
I'm not confident that this test can be very reliable in the long term in its
current form (average stack size differs based on tools).
static const unsigned kLimitAverageThreadStackLength = 8 * 1024;
Thanks,
-Ivan
Original comment by ivan.penkov
on 11 Dec 2012 at 12:41
Hmm.. which part of the test is failing? Can you include the error message(s)?
That stack size just happened to be what I was seeing in the too-large
minidumps, so it's fairly arbitrary. At the same time, I didn't intend the
unittest to depend on what the stack size actually turns out to be (e.g. it
checks for only a 1KB decrease in per-thread stack size).
Original comment by mkr...@chromium.org
on 11 Dec 2012 at 1:13
Here are actual assertions that were hit:
third_party/breakpad/src/client/linux/minidump_writer/minidump_writer_unittest.c
c:705: Failure
Expected: (st.st_size) < (normal_file_size), actual: 585664 vs 585664
Stack trace:
0x00416e26: (anonymous
namespace)::MinidumpWriterTest_MinidumpSizeLimit_Test::TestBody() @
third_party/breakpad/src/client/linux/minidump_writer/minidump_writer_unittest.c
c:705
0x00490ae6: void
testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test,
void>(testing::Test*, void (testing::Test::*)(), char const*) @
third_party/gtest/src/gtest.cc:2086
0x0048dca6: void
testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,
void>(testing::Test*, void (testing::Test::*)(), char const*) @
third_party/gtest/src/gtest.cc:2138
0x00482609: testing::Test::Run() @ third_party/gtest/src/gtest.cc:2158
0x00482bea: testing::TestInfo::Run() @ third_party/gtest/src/gtest.cc:2334
... gUnit internal frames ...
...
...
third_party/breakpad/src/client/linux/minidump_writer/minidump_writer_unittest.c
c:729: Failure
Expected: (total_limit_stack_size) < (total_normal_stack_size -
min_expected_reduction), actual: 503808 vs 483328
Stack trace:
0x00417493: (anonymous
namespace)::MinidumpWriterTest_MinidumpSizeLimit_Test::TestBody() @
third_party/breakpad/src/client/linux/minidump_writer/minidump_writer_unittest.c
c:729
0x00490ae6: void
testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test,
void>(testing::Test*, void (testing::Test::*)(), char const*) @
third_party/gtest/src/gtest.cc:2086
0x0048dca6: void
testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,
void>(testing::Test*, void (testing::Test::*)(), char const*) @
third_party/gtest/src/gtest.cc:2138
0x00482609: testing::Test::Run() @ third_party/gtest/src/gtest.cc:2158
0x00482bea: testing::TestInfo::Run() @ third_party/gtest/src/gtest.cc:2334
... gUnit internal frames ...
I added some temporary logging which shows average stack size of 12 KB:
...
...
Actual stack lenght is: 32768
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
Actual stack lenght is: 12288
...
...
Original comment by ivan.penkov
on 11 Dec 2012 at 7:07
At the same time when I build and run it, on an SVN client directly off of
http://code.google.com/p/google-breakpad, I get an average stack size of 4 KB,
so the particular tools used for building matter:
...
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 8192
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
Actual stack length is 4096
...
Original comment by ivan.penkov
on 11 Dec 2012 at 7:30
It's actually a reverse effect. The larger stack size is making it so the
size-limiting logic doesn't kick in. I.e. It's actually okay to have the 8KB
estimate for a stack size, but the test has to be changed to work when the
stack size is bigger.
I'll upload a change to adjust the unittest's size limit so it will kick in.
Worst case, if we have an issue like this again, it can be changed to something
like 32KB -- which will always trigger the size-limiting code -- but I'm hoping
it can be a little better than that in order to increase the effectiveness of
the test.
Original comment by mkr...@chromium.org
on 11 Dec 2012 at 10:56
Let me know when you are done with the adjustment and I can test it in google3.
Original comment by ivan.penkov
on 11 Dec 2012 at 11:14
Thanks. You can find the change in CL https://breakpad.appspot.com/504002/.
Original comment by mkr...@chromium.org
on 11 Dec 2012 at 11:17
CL committed.
Original comment by mkr...@chromium.org
on 12 Dec 2012 at 2:31
Original comment by ivan.penkov
on 12 Dec 2012 at 11:53
A similar failure is being hit by the the Chromium Android Tests (dbg) build
bot.
Please follow up with https://code.google.com/p/chromium/issues/detail?id=305008
Original comment by msw@chromium.org
on 8 Oct 2013 at 7:07
Original comment by vapier@chromium.org
on 23 Oct 2013 at 3:12
Original issue reported on code.google.com by
ivan.penkov
on 11 Dec 2012 at 12:29