Reviewers: Mads Ager,
Description:
Fix GC issue in instanceof stub
The the call of the builtin in InstanceofStub was not correctly protected
with
an internal frame leading to the return address being handled as a pointer
during GC.
Marked the Instanceof stub as allowing stub calls (the RecordWriteStub was
removed some days ago).
This issue was not caught by the assertion designed for this when debug
mode is
run with --debug-code (which out tests always does) as generating code for
Abort
set the allow stub calls flag to true. This has been fixed by restoring the
allow stub calls flag correctly.
Please review this at http://codereview.chromium.org/6097010/
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/arm/macro-assembler-arm.cc
M src/code-stubs.h
M src/code-stubs.cc
M src/ia32/code-stubs-ia32.cc
M src/ia32/lithium-codegen-ia32.cc
M src/ia32/macro-assembler-ia32.cc
M src/x64/macro-assembler-x64.cc
Index: src/arm/macro-assembler-arm.cc
===================================================================
--- src/arm/macro-assembler-arm.cc (revision 6214)
+++ src/arm/macro-assembler-arm.cc (working copy)
@@ -1795,7 +1795,7 @@
}
#endif
// Disable stub call restrictions to always allow calls to abort.
- set_allow_stub_calls(true);
+ AllowStubCallsScope allow_scope(masm, true);
mov(r0, Operand(p0));
push(r0);
Index: src/code-stubs.cc
===================================================================
--- src/code-stubs.cc (revision 6214)
+++ src/code-stubs.cc (working copy)
@@ -49,8 +49,10 @@
void CodeStub::GenerateCode(MacroAssembler* masm) {
// Update the static counter each time a new code stub is generated.
Counters::code_stubs.Increment();
+
// Nested stubs are not allowed for leafs.
- masm->set_allow_stub_calls(AllowsStubCalls());
+ AllowStubCallsScope allow_scope(masm, AllowsStubCalls());
+
// Generate the code for the stub.
masm->set_generating_stub(true);
Generate(masm);
Index: src/code-stubs.h
===================================================================
--- src/code-stubs.h (revision 6214)
+++ src/code-stubs.h (working copy)
@@ -34,7 +34,7 @@
namespace internal {
// List of code stubs used on all platforms. The order in this list is
important
-// as only the stubs up to and including RecordWrite allows nested stub
calls.
+// as only the stubs up to and including Instanceof allows nested stub
calls.
#define CODE_STUB_LIST_ALL_PLATFORMS(V) \
V(CallFunction) \
V(GenericBinaryOp) \
@@ -48,7 +48,7 @@
V(CompareIC) \
V(MathPow) \
V(TranscendentalCache) \
- V(RecordWrite) \
+ V(Instanceof) \
V(ConvertToDouble) \
V(WriteInt32ToHeapNumber) \
V(IntegerMod) \
@@ -59,7 +59,6 @@
V(GenericUnaryOp) \
V(RevertToNumber) \
V(ToBoolean) \
- V(Instanceof) \
V(CounterOp) \
V(ArgumentsAccess) \
V(RegExpExec) \
@@ -180,7 +179,7 @@
MajorKeyBits::encode(MajorKey());
}
- bool AllowsStubCalls() { return MajorKey() <= RecordWrite; }
+ bool AllowsStubCalls() { return MajorKey() <= Instanceof; }
class MajorKeyBits: public BitField<uint32_t, 0, kMajorBits> {};
class MinorKeyBits: public BitField<uint32_t, kMajorBits, kMinorBits> {};
@@ -917,6 +916,24 @@
DISALLOW_COPY_AND_ASSIGN(StringCharAtGenerator);
};
+
+class AllowStubCallsScope {
+ public:
+ AllowStubCallsScope(MacroAssembler* masm, bool allow)
+ : masm_(masm), previous_allow_(masm->allow_stub_calls()) {
+ masm_->set_allow_stub_calls(allow);
+ }
+ ~AllowStubCallsScope() {
+ masm_->set_allow_stub_calls(previous_allow_);
+ }
+
+ private:
+ MacroAssembler* masm_;
+ bool previous_allow_;
+
+ DISALLOW_COPY_AND_ASSIGN(AllowStubCallsScope);
+};
+
} } // namespace v8::internal
#endif // V8_CODE_STUBS_H_
Index: src/ia32/code-stubs-ia32.cc
===================================================================
--- src/ia32/code-stubs-ia32.cc (revision 6214)
+++ src/ia32/code-stubs-ia32.cc (working copy)
@@ -5176,9 +5176,11 @@
__ InvokeBuiltin(Builtins::INSTANCE_OF, JUMP_FUNCTION);
} else {
// Call the builtin and convert 0/1 to true/false.
+ __ EnterInternalFrame();
__ push(object);
__ push(function);
__ InvokeBuiltin(Builtins::INSTANCE_OF, CALL_FUNCTION);
+ __ LeaveInternalFrame();
NearLabel true_value, done;
__ test(eax, Operand(eax));
__ j(zero, &true_value);
Index: src/ia32/lithium-codegen-ia32.cc
===================================================================
--- src/ia32/lithium-codegen-ia32.cc (revision 6214)
+++ src/ia32/lithium-codegen-ia32.cc (working copy)
@@ -1767,7 +1767,7 @@
__ cmp(object, Factory::null_value());
__ j(equal, &false_result);
- // String values is not instance of anything.
+ // String values are not instances of anything.
Condition is_string = masm_->IsObjectStringType(object, temp, temp);
__ j(is_string, &false_result);
Index: src/ia32/macro-assembler-ia32.cc
===================================================================
--- src/ia32/macro-assembler-ia32.cc (revision 6214)
+++ src/ia32/macro-assembler-ia32.cc (working copy)
@@ -1715,7 +1715,7 @@
}
#endif
// Disable stub call restrictions to always allow calls to abort.
- set_allow_stub_calls(true);
+ AllowStubCallsScope allow_scope(this, true);
push(eax);
push(Immediate(p0));
Index: src/x64/macro-assembler-x64.cc
===================================================================
--- src/x64/macro-assembler-x64.cc (revision 6214)
+++ src/x64/macro-assembler-x64.cc (working copy)
@@ -288,7 +288,7 @@
}
#endif
// Disable stub call restrictions to always allow calls to abort.
- set_allow_stub_calls(true);
+ AllowStubCallsScope allow_scope(this, true);
push(rax);
movq(kScratchRegister, p0, RelocInfo::NONE);
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
