Add 4K more memory for the HugeConsStringOutOfMemory test case

Add 4K more memory for HugeConsStringOutOfMemory test case

R=yangguo@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=15472

[haitao.feng, 2013-07-03 02:41:53]

On my Linux and Mac machine, I need 4K (Page::kPageSize) more memory to prevent
HugeConsStringOutOfMemory being crashed when "make x64.release.check
snapshot=off TESTFLAGS="cctest/test-api/HugeConsStringOutOfMemory".

[danno, 2013-07-03 07:40:22]

This seems a bit arbitrary to me. I'm handing off to yang to take a closer look.

[Yang, 2013-07-03 08:25:40]

On 2013/07/03 07:40:22, danno wrote:
> This seems a bit arbitrary to me. I'm handing off to yang to take a closer
look.

This does not seem like a correct fix.

This test is supposed to check whether V8 aborts execution orderly with an out
of memory exception due to the cons string content being too long. The cons
string itself takes less than 1 kB, but the content exceeds the maximum string
length, so we throw an out of memory exception even though we still got fee
space on the heap. For me, lowering the old space size to 1 * K * K still passes
the test.

Maybe you changed something else that led to the crash that you are describing?

[haitao.feng, 2013-07-03 11:12:47]

Hi Yang,

Thanks for the review and have a try to modify the old space size. I did not
change anything. Here is the crash trace in my machine:

#
# Fatal error in ../src/contexts.h, line 236
# CHECK(context->IsContext()) failed
#

==== C stack trace ===============================

 1: V8_Fatal
 2: v8::internal::Context::cast(v8::internal::Object*)
 3: v8::internal::Heap::ClearJSFunctionResultCaches()
 4: v8::internal::Heap::GarbageCollectionPrologue()
 5: v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace,
v8::internal::GarbageCollector, char const*, char const*)
 6: v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, char
const*)
 7: v8::internal::Factory::NewCode(v8::internal::CodeDesc const&, unsigned int,
v8::internal::Handle<v8::internal::Object>, bool, bool)
 8: v8::internal::PlatformCodeStub::GenerateCode()
 9: v8::internal::CodeStub::GetCode(v8::internal::Isolate*)
10: v8::internal::Heap::CreateJSEntryStub()
11: v8::internal::Heap::CreateFixedStubs()
12: v8::internal::Heap::CreateInitialObjects()
13: v8::internal::Heap::CreateHeapObjects()
14: v8::internal::Isolate::Init(v8::internal::Deserializer*)
15: v8::internal::V8::Initialize(v8::internal::Deserializer*)
16: ??
17: ??
18: v8::Context::New(v8::Isolate*, v8::ExtensionConfiguration*,
v8::Handle<v8::ObjectTemplate>, v8::Handle<v8::Value>)
19: LocalContext::LocalContext(v8::ExtensionConfiguration*,
v8::Handle<v8::ObjectTemplate>, v8::Handle<v8::Value>)
20: ??
21: CcTest::Run()

The crash happens in the bootstrap. The function ClearJSFunctionResultCaches
assumes native_contexts_list_ is not NULL, but native_contexts_list_ is set to
undefined_value() after CreateInitialObjects in the CreateHeapObjects. I tried
to modify ClearJSFunctionResultCaches to see whether context (heap.cc:832)is
NULL, but it crashes elsewhere.

Considering https://chromiumcodereview.appspot.com/10533148, it might make sense
to add 4K more memory for this test case. I do not know why this is not caught
by your trybots and could not be reproduced on your machine.

[Yang, 2013-07-03 14:52:34]

On 2013/07/03 11:12:47, haitao.feng wrote:
> Hi Yang,
> 
> Thanks for the review and have a try to modify the old space size. I did not
> change anything. Here is the crash trace in my machine:
> 
> #
> # Fatal error in ../src/contexts.h, line 236
> # CHECK(context->IsContext()) failed
> #
> 
> ==== C stack trace ===============================
> 
>  1: V8_Fatal
>  2: v8::internal::Context::cast(v8::internal::Object*)
>  3: v8::internal::Heap::ClearJSFunctionResultCaches()
>  4: v8::internal::Heap::GarbageCollectionPrologue()
>  5: v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace,
> v8::internal::GarbageCollector, char const*, char const*)
>  6: v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, char
> const*)
>  7: v8::internal::Factory::NewCode(v8::internal::CodeDesc const&, unsigned
int,
> v8::internal::Handle<v8::internal::Object>, bool, bool)
>  8: v8::internal::PlatformCodeStub::GenerateCode()
>  9: v8::internal::CodeStub::GetCode(v8::internal::Isolate*)
> 10: v8::internal::Heap::CreateJSEntryStub()
> 11: v8::internal::Heap::CreateFixedStubs()
> 12: v8::internal::Heap::CreateInitialObjects()
> 13: v8::internal::Heap::CreateHeapObjects()
> 14: v8::internal::Isolate::Init(v8::internal::Deserializer*)
> 15: v8::internal::V8::Initialize(v8::internal::Deserializer*)
> 16: ??
> 17: ??
> 18: v8::Context::New(v8::Isolate*, v8::ExtensionConfiguration*,
> v8::Handle<v8::ObjectTemplate>, v8::Handle<v8::Value>)
> 19: LocalContext::LocalContext(v8::ExtensionConfiguration*,
> v8::Handle<v8::ObjectTemplate>, v8::Handle<v8::Value>)
> 20: ??
> 21: CcTest::Run()
> 
> The crash happens in the bootstrap. The function ClearJSFunctionResultCaches
> assumes native_contexts_list_ is not NULL, but native_contexts_list_ is set to
> undefined_value() after CreateInitialObjects in the CreateHeapObjects. I tried
> to modify ClearJSFunctionResultCaches to see whether context (heap.cc:832)is
> NULL, but it crashes elsewhere.
> 
> Considering https://chromiumcodereview.appspot.com/10533148, it might make
sense
> to add 4K more memory for this test case. I do not know why this is not caught
> by your trybots and could not be reproduced on your machine.

I was able to reproduce it after I added snapshot=off. I guess it makes sense to
bump the limit to 4M altogether instead of 3M+4K. I'll bump the limit to 4M and
land this for you. LGTM.

[Yang, 2013-07-03 14:56:23]

Committed patchset #1 manually as r15472 (presubmit successful).