From ff5e1c9822081d9a49b5daf20e04a28a0f145c3c Mon Sep 17 00:00:00 2001 From: "erik.corry@gmail.com" Date: Fri, 16 Sep 2011 13:06:51 +0000 Subject: [PATCH] Fix asserts and GC unsafeness in stub generation, bug=1689. Review URL: http://codereview.chromium.org/7920006 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9311 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/code-stubs-arm.cc | 14 +++++++++++++- src/arm/lithium-codegen-arm.cc | 4 +++- src/code-stubs.h | 2 +- src/ia32/code-stubs-ia32.cc | 11 ++++++++++- src/ia32/lithium-codegen-ia32.cc | 2 ++ src/isolate.cc | 1 + src/isolate.h | 7 +++++++ src/mips/code-stubs-mips.cc | 12 +++++++++++- src/x64/code-stubs-x64.cc | 4 ++++ src/x64/lithium-codegen-x64.cc | 2 +- test/mozilla/mozilla.status | 3 --- test/sputnik/sputnik.status | 3 --- 12 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index 4ccdd7b..383803d 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -3181,6 +3181,7 @@ void TranscendentalCacheStub::GenerateCallCFunction(MacroAssembler* masm, } else { __ vmov(r0, r1, d2); } + AllowExternalCallThatCantCauseGC scope(masm); switch (type_) { case TranscendentalCache::SIN: __ CallCFunction(ExternalReference::math_sin_double_function(isolate), @@ -3334,12 +3335,23 @@ bool CEntryStub::NeedsImmovableCode() { bool CEntryStub::CompilingCallsToThisStubIsGCSafe() { - return !save_doubles_ && result_size_ == 1; + return (!save_doubles_ || ISOLATE->fp_stubs_generated()) && + result_size_ == 1; } void CodeStub::GenerateStubsAheadOfTime() { } + + +void CodeStub::GenerateFPStubs() { + CEntryStub save_doubles(1); + save_doubles.SaveDoubles(); + Handle code = save_doubles.GetCode(); + code->GetIsolate()->set_fp_stubs_generated(true); +} + + void CEntryStub::GenerateThrowTOS(MacroAssembler* masm) { __ Throw(r0); } diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index b255b2c..260b2f9 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -83,9 +83,11 @@ bool LCodeGen::GenerateCode() { CpuFeatures::Scope scope1(VFP3); CpuFeatures::Scope scope2(ARMv7); + CodeStub::GenerateFPStubs(); + // Open a frame scope to indicate that there is a frame on the stack. The // NONE indicates that the scope shouldn't actually generate code to set up - // the frame (that is done in GeneatePrologue). + // the frame (that is done in GeneratePrologue). FrameScope frame_scope(masm_, StackFrame::NONE); return GeneratePrologue() && diff --git a/src/code-stubs.h b/src/code-stubs.h index 6bed221..2697a4c 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -147,8 +147,8 @@ class CodeStub BASE_EMBEDDED { return MajorKey() <= Instanceof; } - static void GenerateStubsAheadOfTime(); + static void GenerateFPStubs(); // Some stubs put untagged junk on the stack that cannot be scanned by the // GC. This means that we must be statically sure that no GC can occur while diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index 97025df..4c17353 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -4293,7 +4293,8 @@ bool CEntryStub::NeedsImmovableCode() { bool CEntryStub::CompilingCallsToThisStubIsGCSafe() { - return !save_doubles_ && result_size_ == 1; + return (!save_doubles_ || ISOLATE->fp_stubs_generated()) && + result_size_ == 1; } @@ -4301,6 +4302,14 @@ void CodeStub::GenerateStubsAheadOfTime() { } +void CodeStub::GenerateFPStubs() { + CEntryStub save_doubles(1); + save_doubles.SaveDoubles(); + Handle code = save_doubles.GetCode(); + code->GetIsolate()->set_fp_stubs_generated(true); +} + + void CEntryStub::GenerateThrowTOS(MacroAssembler* masm) { __ Throw(eax); } diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 0a8b694..c7270c5 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -71,6 +71,8 @@ bool LCodeGen::GenerateCode() { status_ = GENERATING; CpuFeatures::Scope scope(SSE2); + CodeStub::GenerateFPStubs(); + // Open a frame scope to indicate that there is a frame on the stack. The // MANUAL indicates that the scope shouldn't actually generate code to set up // the frame (that is done in GeneratePrologue). diff --git a/src/isolate.cc b/src/isolate.cc index fd0f673..b68f5d5 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -1408,6 +1408,7 @@ Isolate::Isolate() global_handles_(NULL), context_switcher_(NULL), thread_manager_(NULL), + fp_stubs_generated_(false), string_tracker_(NULL), regexp_stack_(NULL), embedder_data_(NULL) { diff --git a/src/isolate.h b/src/isolate.h index 2582da6..eb0e1c3 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -879,6 +879,12 @@ class Isolate { RuntimeState* runtime_state() { return &runtime_state_; } + void set_fp_stubs_generated(bool value) { + fp_stubs_generated_ = value; + } + + bool fp_stubs_generated() { return fp_stubs_generated_; } + StaticResource* compiler_safe_string_input_buffer() { return &compiler_safe_string_input_buffer_; } @@ -1136,6 +1142,7 @@ class Isolate { ContextSwitcher* context_switcher_; ThreadManager* thread_manager_; RuntimeState runtime_state_; + bool fp_stubs_generated_; StaticResource compiler_safe_string_input_buffer_; Builtins builtins_; StringTracker* string_tracker_; diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index 37d5133..9584a48 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -3318,6 +3318,7 @@ void TranscendentalCacheStub::GenerateCallCFunction(MacroAssembler* masm, } else { __ mov_d(f12, f4); } + AllowExternalCallThatCantCauseGC scope(masm); switch (type_) { case TranscendentalCache::SIN: __ CallCFunction( @@ -3477,7 +3478,8 @@ bool CEntryStub::NeedsImmovableCode() { bool CEntryStub::CompilingCallsToThisStubIsGCSafe() { - return !save_doubles_ && result_size_ == 1; + return (!save_doubles_ || ISOLATE->fp_stubs_generated()) && + result_size_ == 1; } @@ -3485,6 +3487,14 @@ void CodeStub::GenerateStubsAheadOfTime() { } +void CodeStub::GenerateFPStubs() { + CEntryStub save_doubles(1); + save_doubles.SaveDoubles(); + Handle code = save_doubles.GetCode(); + code->GetIsolate()->set_fp_stubs_generated(true); +} + + void CEntryStub::GenerateThrowTOS(MacroAssembler* masm) { __ Throw(v0); } diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 1c07dfb..e792df5 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -3336,6 +3336,10 @@ void CodeStub::GenerateStubsAheadOfTime() { } +void CodeStub::GenerateFPStubs() { +} + + void CEntryStub::GenerateThrowTOS(MacroAssembler* masm) { // Throw exception in eax. __ Throw(rax); diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 12b8dd2..64d99f4 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -84,7 +84,7 @@ bool LCodeGen::GenerateCode() { // Open a frame scope to indicate that there is a frame on the stack. The // MANUAL indicates that the scope shouldn't actually generate code to set up - // the frame (that is done in GeneatePrologue). + // the frame (that is done in GeneratePrologue). FrameScope frame_scope(masm_, StackFrame::MANUAL); return GeneratePrologue() && diff --git a/test/mozilla/mozilla.status b/test/mozilla/mozilla.status index 218b2ef..8a0da10 100644 --- a/test/mozilla/mozilla.status +++ b/test/mozilla/mozilla.status @@ -69,9 +69,6 @@ js1_5/Array/regress-465980-02: SKIP ecma_3/Date/15.9.3.2-1: SKIP js1_2/function/Number: SKIP -# Causes assert to be triggered: http://code.google.com/p/v8/issues/detail?id=1689 -js1_5/GC/regress-311497: SKIP - ##################### SLOW TESTS ##################### # This takes a long time to run (~100 seconds). It should only be run diff --git a/test/sputnik/sputnik.status b/test/sputnik/sputnik.status index 038936a..2373fb5 100644 --- a/test/sputnik/sputnik.status +++ b/test/sputnik/sputnik.status @@ -37,9 +37,6 @@ S15.3.4.5_A2: FAIL # '__proto__' should be treated as a normal property in JSON. S15.12.2_A1: FAIL -# Assert is triggered by this test: http://code.google.com/p/v8/issues/detail?id=1689 -S13_A18: SKIP - ##################### DELIBERATE INCOMPATIBILITIES ##################### # This tests precision of trignometric functions. We're slightly off -- 2.7.4