Load the global proxy from the context of the target function.

Load the global proxy from the context of the target function.

BUG=
R=dcarney@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=18458

[dcarney, 2013-12-10 15:28:23]

lgtm

[dcarney, 2013-12-16 13:44:27]

ptal

hydrogen still needed as well as other arches

[Toon Verwaest, 2013-12-18 13:09:33]

https://codereview.chromium.org/111613003/diff/160001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/111613003/diff/160001/src/bootstrapper.cc#new...
src/bootstrapper.cc:2613: native_context()->global_proxy());
I think we should only be doing this if we are deserializing the snapshot.
Basically it should be done around the time we do HookUpInnerGlobal.

https://codereview.chromium.org/111613003/diff/160001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/111613003/diff/160001/src/hydrogen.cc#newcode...
src/hydrogen.cc:7735:
expr->target()->context()->global_object()->global_receiver());
context()->global_proxy()

Weren't you planning on extracting this to a separate method?

https://codereview.chromium.org/111613003/diff/160001/src/stub-cache.cc
File src/stub-cache.cc (right):

https://codereview.chromium.org/111613003/diff/160001/src/stub-cache.cc#newco...
src/stub-cache.cc:1155: FetchGlobalProxy(masm(), function, rdx);
EEK! rdx in src/stub-cache.cc?

Please run tests on all platforms ;)

https://codereview.chromium.org/111613003/diff/160001/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

https://codereview.chromium.org/111613003/diff/160001/src/x64/builtins-x64.cc...
src/x64/builtins-x64.cc:830: CallStubCompiler::FetchGlobalProxy(masm, rdi, rbx);
Maybe we should swap the arguments in FetchGlobalProxy, to have the target on
the left and the source on the right, as in most assembler-related functions.
Otherwise someone is guaranteed to swap them. (Probably me...)

[Toon Verwaest, 2013-12-18 15:05:40]

lgtm

[dcarney, 2013-12-26 13:35:52]

Hey Toon, I hope you can land this before I get back.  I've added a few comments
that it would be great if you could incorporate before landing.

https://codereview.chromium.org/111613003/diff/280001/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

https://codereview.chromium.org/111613003/diff/280001/src/arm/builtins-arm.cc...
src/arm/builtins-arm.cc:1078: __ ldr(r2, ContextOperand(cp,
Context::GLOBAL_OBJECT_INDEX));
here and below we load from the correct context directly. for the other
architectures, we're loading from the function via FetchGlobalProxy, but the
function isn't any more in a register here. maybe the other arches should be
like this one?

https://codereview.chromium.org/111613003/diff/280001/src/ia32/code-stubs-ia3...
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/111613003/diff/280001/src/ia32/code-stubs-ia3...
src/ia32/code-stubs-ia32.cc:2533: __ mov(ecx, GlobalObjectOperand());
this should have a TODO to fix for function proxies

[Toon Verwaest, 2014-01-03 17:56:26]

PTAL again. Sorry for the rebase.

I just modified code-stubs-<arch>.cc and
test/mjsunit/harmony/proxies-function.js.

Apparently passing "undefined_value" as receiver to CALL_FUNCTION_PROXY will end
up patching in the global receiver from the call-trap function in
Execution::Call.

Additionally, notice that on X64/ARM CALL_AS_METHOD was erroneously used, while
IA32 still properly used CALL_AS_FUNCTION for CALL_FUNCTION_PROXY. Apparently
(surprisingly...) it wasn't tested. That path is now tested by the second
with-variant in the function proxy tests that I added.

https://codereview.chromium.org/111613003/diff/280001/src/ia32/code-stubs-ia3...
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/111613003/diff/280001/src/ia32/code-stubs-ia3...
src/ia32/code-stubs-ia32.cc:2533: __ mov(ecx, GlobalObjectOperand());
On 2013/12/26 13:35:52, dcarney wrote:
> this should have a TODO to fix for function proxies

I decided to just fix it anyway. I discovered some bugs in this code by doing
so.

[dcarney, 2014-01-07 08:15:35]

> I decided to just fix it anyway. I discovered some bugs in this code by doing
> so.

nice. I'm glad pries are dealt with
lgtm

[Toon Verwaest, 2014-01-07 08:21:41]

Committed patchset #11 manually as r18458 (presubmit successful).