Disallow garbage collection at another site in the LoadCallback ICs.... (issue504071)

[kmil...@chromium.org]

Reviewers: Kasper Lund,

Description:
Disallow garbage collection at another site in the LoadCallback ICs.
MacroAssembler::PopHandleScope emits a runtime call (through a stub),
which should not be allowed to perform a GC but return a failure
instead.

BUG=30790
TEST=none

Please review this at http://codereview.chromium.org/504071

SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/

Affected files:
M src/ia32/macro-assembler-ia32.h
M src/ia32/macro-assembler-ia32.cc
M src/ia32/stub-cache-ia32.cc

[ipo...@chromium.org]

Kevin,

I think you are fighting a loosing battle here. While this change will
certainly
fix the reported problem, I think a safer approach would be to not drop the
handles when calling ComputeLoadCallback in the first place.

LGTM especially if you clarify the confusing comment.

-Ivan

http://codereview.chromium.org/504071/diff/1/4
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/504071/diff/1/4#newcode818
src/ia32/stub-cache-ia32.cc:818: // collection but instead return a
failure object.
This comment is very confusing. Isn't it so that emitting the code to
call PopHandleScope can cause a GC? This is not what I read in your
comment here.
At the least please clarify that the two instances of the word "call" do
not mean the same call.

http://codereview.chromium.org/504071

[Kevin Millikin]

I agree, it's much better to have this code safe for GC.  This should be seen as a temporary fix.

--
v8-dev mailing list
v8-...@googlegroups.com
http://groups.google.com/group/v8-dev

[Erik Corry]

You are sure that this only affects IA32?

2009/12/20 Kevin Millikin <kmil...@chromium.org>

[Kevin Millikin]

Positive.  We do not use the API stub except on ia32, we call the runtime on x64 and ARM for callback properties.