http://codereview.chromium.org/2801007/diff/1/2 File src/ia32/ic-ia32.cc (right):
http://codereview.chromium.org/2801007/diff/1/2#newcode98 src/ia32/ic-ia32.cc:98: // Appropriate access checks must be done before entering this code. On 2010/06/17 11:37:04, Mads Ager wrote:
You need to spell out what appropriate checks mean here. We need to
check that
the receiver is not a global object and that it does not have
interceptors. Done. http://codereview.chromium.org/2801007/diff/1/2#newcode1213 src/ia32/ic-ia32.cc:1213: static void GenerateFunctionCall(MacroAssembler* masm, int argc, Label* miss) { On 2010/06/17 11:37:04, Mads Ager wrote:
GenerateFunctionCall makes it sound like the code will return to after GenerateFunctionCall. Rename to GenerateFunctionTailCall or
GenerateJumpFunction
or something. In the macro assembler we use CallX and TailCallX: CallStub/TailCallStub, CallRuntime/TailCallRuntime.
Done. http://codereview.chromium.org/2801007/diff/1/2#newcode1250 src/ia32/ic-ia32.cc:1250: GenerateLoadReceiverCheck(masm, edx, ebx, eax, &miss, &global_proxy); On 2010/06/17 11:37:04, Mads Ager wrote:
Thanks for cleaning this up!
I think we should clean it up even more and bail out on global proxy
objects as
well? The proxy object does not contains any properties on its own and
the
dictionary probing code used to bail out on global proxy objects. The
current
change seems to allow dictionary probing of proxy objects which does
not make
sense (it probably bails out because the properties are not a
dictionary, but we
should avoid the attempt completely).
Done. http://codereview.chromium.org/2801007/diff/1/2#newcode1304 src/ia32/ic-ia32.cc:1304: // This can happen only for regular CallIC but not KeyedCallIC. Before introducing KeyedCallIC keyed call were done via CallFunctionStub that did not patch global receiver and everything worked fine. KeyedCallIC just replaced CallFunctionStub so global object should not show up here. On 2010/06/17 11:37:04, Mads Ager wrote:
Why can't this happen for KeyedCallICs? Because we always do a LoadGlobalReceiver before calling KeyedCallICs (and a LoadGlobal
before calling
CallICs)? It would be very bad to miss the receiver patching for keyed
call ics,
so we need to be 100% sure that this is correct. The best way to
convince
ourself that this is correct is to make sure that there is a test for
both
CallICs and KeyedCallICs where we hit this path and the patching
matters (access
checks should fail).
http://codereview.chromium.org/2801007/diff/1/2#newcode1539 src/ia32/ic-ia32.cc:1539: GenerateLoadReceiverCheck(masm, eax, ebx, edx, &miss, &global); On 2010/06/17 11:37:04, Mads Ager wrote:
Same comment: Thanks for cleaning this up. We should miss on global
proxy as
well.
Done. http://codereview.chromium.org/2801007/diff/5001/6002 File src/arm/ic-arm.cc (right): http://codereview.chromium.org/2801007/diff/5001/6002#newcode53 src/arm/ic-arm.cc:53: Label* miss) { Done (for all 3 platforms) On 2010/06/23 07:18:12, Mads Ager wrote:
miss -> global_object?
http://codereview.chromium.org/2801007/diff/5001/6002#newcode65 src/arm/ic-arm.cc:65: // Generated code falls through if the receiver is a regular JS object On 2010/06/23 07:18:12, Mads Ager wrote:
regular -> regular non-global
Done (for all 3 platforms) http://codereview.chromium.org/2801007/diff/5001/6002#newcode98 src/arm/ic-arm.cc:98: __ b(ne, miss); On 2010/06/23 07:18:12, Mads Ager wrote:
You could you nz here (which is the same as ne so it only matters for readability).
Done. http://codereview.chromium.org/2801007/diff/5001/6002#newcode126 src/arm/ic-arm.cc:126: Register t0, On 2010/06/23 07:18:12, Mads Ager wrote:
Change name back to scratch for these?
Done. http://codereview.chromium.org/2801007/diff/5001/6002#newcode130 src/arm/ic-arm.cc:130: // dictionary. On 2010/06/23 07:18:12, Mads Ager wrote:
Indentation or change name of registers back to scratch.
Done. http://codereview.chromium.org/2801007/diff/5001/6002#newcode347 src/arm/ic-arm.cc:347: __ b(ne, slow); On 2010/06/23 07:18:12, Mads Ager wrote:
Could use nz.
Done. http://codereview.chromium.org/2801007/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev
