Add asserts and state tracking to ensure that we do not call

Add asserts and state tracking to ensure that we do not call
into C++ without having a valid stack frame that can be
traversed at GC.
Also add asserts to track that we do not try to generate a stub
while we are generating a stub, since the stub creation code is
not GC safe.
Committed: http://code.google.com/p/v8/source/detail?r=8122

[Søren Thygesen Gjesse, 2011-05-31 07:51:45]

LGTM

http://codereview.chromium.org/7084032/diff/4004/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/arm/builtins-arm.cc#newc...
src/arm/builtins-arm.cc:1065: // Exit the JS frame and remove the parameters
(except function), and
JS frame -> internal frame.

http://codereview.chromium.org/7084032/diff/4004/src/arm/builtins-arm.cc#newc...
src/arm/builtins-arm.cc:1135: // Tear down temporary frame.
temporary -> internal

http://codereview.chromium.org/7084032/diff/4004/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:159: FrameScope frame_scope(masm_,
StackFrame::NONE);
How about using StackFrame::JAVA_SCRIPT, and maybe place it after the __ Push
where the frame is set up? Then a FrameScopt for StackFrame::JAVA_SCRIPT of
cause should do nothing but mark that a frame is present.

http://codereview.chromium.org/7084032/diff/4004/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:89: FrameScope frame_scope(masm_,
StackFrame::NONE);
StackFrame::JAVA_SCRIPT?

http://codereview.chromium.org/7084032/diff/4004/src/arm/macro-assembler-arm.h
File src/arm/macro-assembler-arm.h (right):

http://codereview.chromium.org/7084032/diff/4004/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.h:1063: 
Can these two classes be in platform independent macro-assembler.h?

http://codereview.chromium.org/7084032/diff/4004/src/arm/regexp-macro-assembl...
File src/arm/regexp-macro-assembler-arm.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/arm/regexp-macro-assembl...
src/arm/regexp-macro-assembler-arm.cc:380: __ CallCFunction(function,
argument_count);
How about having CallCFunctionWithoutFrame or CallCFunctionNotCausingGC instead?
Then the comment is not needed.

http://codereview.chromium.org/7084032/diff/4004/src/builtins.cc
File src/builtins.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/builtins.cc#newcode1620
src/builtins.cc:1620: NoCurrentFrameScope scope(&masm);
This might deserve a comment.

http://codereview.chromium.org/7084032/diff/4004/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/ia32/builtins-ia32.cc#ne...
src/ia32/builtins-ia32.cc:466: __ ret(1 * kPointerSize);  // Remove receiver.
Maybe remove 1 * while you are here.

[Erik Corry, 2011-05-31 11:52:51]

http://codereview.chromium.org/7084032/diff/4004/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/arm/builtins-arm.cc#newc...
src/arm/builtins-arm.cc:1065: // Exit the JS frame and remove the parameters
(except function), and
On 2011/05/31 07:51:45, Søren Gjesse wrote:
> JS frame -> internal frame.

Done.

http://codereview.chromium.org/7084032/diff/4004/src/arm/builtins-arm.cc#newc...
src/arm/builtins-arm.cc:1135: // Tear down temporary frame.
On 2011/05/31 07:51:45, Søren Gjesse wrote:
> temporary -> internal

Done.

http://codereview.chromium.org/7084032/diff/4004/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:159: FrameScope frame_scope(masm_,
StackFrame::NONE);
On 2011/05/31 07:51:45, Søren Gjesse wrote:
> How about using StackFrame::JAVA_SCRIPT, and maybe place it after the __ Push
> where the frame is set up? Then a FrameScopt for StackFrame::JAVA_SCRIPT of
> cause should do nothing but mark that a frame is present.

Used StackFrame::MANUAL

http://codereview.chromium.org/7084032/diff/4004/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:89: FrameScope frame_scope(masm_,
StackFrame::NONE);
On 2011/05/31 07:51:45, Søren Gjesse wrote:
> StackFrame::JAVA_SCRIPT?

Done.

http://codereview.chromium.org/7084032/diff/4004/src/arm/macro-assembler-arm.h
File src/arm/macro-assembler-arm.h (right):

http://codereview.chromium.org/7084032/diff/4004/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.h:1063: 
On 2011/05/31 07:51:45, Søren Gjesse wrote:
> Can these two classes be in platform independent macro-assembler.h?

Done.

http://codereview.chromium.org/7084032/diff/4004/src/arm/regexp-macro-assembl...
File src/arm/regexp-macro-assembler-arm.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/arm/regexp-macro-assembl...
src/arm/regexp-macro-assembler-arm.cc:380: __ CallCFunction(function,
argument_count);
On 2011/05/31 07:51:45, Søren Gjesse wrote:
> How about having CallCFunctionWithoutFrame or CallCFunctionNotCausingGC
instead?
> Then the comment is not needed.

This turns out to be messy because there are lots and lots of CallCFunction
variants.  Instead I introduced a new scoped variable called
AllowExternalCallThatCantCauseGC.  I am hoping with a name like that I don't
need a comment.

http://codereview.chromium.org/7084032/diff/4004/src/builtins.cc
File src/builtins.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/builtins.cc#newcode1620
src/builtins.cc:1620: NoCurrentFrameScope scope(&masm);
On 2011/05/31 07:51:45, Søren Gjesse wrote:
> This might deserve a comment.

Replaced with an assert that there is no current frame.

http://codereview.chromium.org/7084032/diff/4004/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/7084032/diff/4004/src/ia32/builtins-ia32.cc#ne...
src/ia32/builtins-ia32.cc:466: __ ret(1 * kPointerSize);  // Remove receiver.
On 2011/05/31 07:51:45, Søren Gjesse wrote:
> Maybe remove 1 * while you are here.

Done.