Fix receiver when calling eval() bound by with scope

Fix receiver when calling eval() bound by with scope

Thanks to André Bargull for the report.

BUG=v8:4214
LOG=N
R=arv@chromium.org, mstarzinger@chromium.org

Committed: https://crrev.com/3c5f0db3a1768ade68108bf003676ce378d1cbdc
Cr-Commit-Position: refs/heads/master@{#29259}

[wingo, 2015-06-24 11:55:20]

PTAL.  Another option would be to revert the part of
https://codereview.chromium.org/1178903003 that made kResolvePossiblyDirectEval
return just one result, and make it take the receiver again, dunno though.

[wingo, 2015-06-24 11:55:36]

On 2015/06/24 11:55:20, wingo wrote:
> PTAL.  Another option would be to revert the part of
> https://codereview.chromium.org/1178903003 that made
kResolvePossiblyDirectEval
> return just one result, and make it take the receiver again, dunno though.

also note that this patch is missing TF support and other arch support

[Toon Verwaest, 2015-06-24 13:30:43]

verwaest@chromium.org changed reviewers:
+ verwaest@chromium.org

[Toon Verwaest, 2015-06-24 13:30:44]

https://codereview.chromium.org/1202963005/diff/1/src/x64/full-codegen-x64.cc
File src/x64/full-codegen-x64.cc (right):

https://codereview.chromium.org/1202963005/diff/1/src/x64/full-codegen-x64.cc...
src/x64/full-codegen-x64.cc:3064: // passing the hole to the call function stub.
Seems like you are passing in undefined instead ...

[wingo, 2015-06-24 13:35:15]

On 2015/06/24 13:30:44, Toon Verwaest wrote:
> https://codereview.chromium.org/1202963005/diff/1/src/x64/full-codegen-x64.cc
> File src/x64/full-codegen-x64.cc (right):
> 
>
https://codereview.chromium.org/1202963005/diff/1/src/x64/full-codegen-x64.cc...
> src/x64/full-codegen-x64.cc:3064: // passing the hole to the call function
stub.
> Seems like you are passing in undefined instead ...

Right, forgot to fix this comment from the existing code which was already
incorrect.  Will poke.

[arv (Not doing code reviews), 2015-06-24 13:36:50]

Looks good, ready for other ports

https://codereview.chromium.org/1202963005/diff/1/src/x64/full-codegen-x64.cc
File src/x64/full-codegen-x64.cc (right):

https://codereview.chromium.org/1202963005/diff/1/src/x64/full-codegen-x64.cc...
src/x64/full-codegen-x64.cc:3064: // passing the hole to the call function stub.
On 2015/06/24 13:30:43, Toon Verwaest wrote:
> Seems like you are passing in undefined instead ...

Old code had the same comment bug :-)

[wingo, 2015-06-24 14:46:05]

New patch adds TF support, in case Michael or someone other TF wizard has time
to look at it today.  Still getting other full-codegen things together.

[Michael Starzinger, 2015-06-24 15:05:03]

LGTM on the TurboFan port. About the CL in general, no comment.

[Michael Starzinger, 2015-06-24 15:14:09]

Just to put my thoughts into writing, I am not entirely convinced this is
cleaner than what we had here originally before the cleanup. Also this uses one
more bailout point than before. But I don't feel strongly enough to block this,
and especially I don't want to become the main reviewer for this CL.

[wingo, 2015-06-24 15:23:48]

On 2015/06/24 15:14:09, Michael Starzinger wrote:
> Just to put my thoughts into writing, I am not entirely convinced this is
> cleaner than what we had here originally before the cleanup. Also this uses
one
> more bailout point than before. But I don't feel strongly enough to block
this,
> and especially I don't want to become the main reviewer for this CL.

Thanks for your thoughts Michael.  I was looking into the history now and it
seems that this bug is of long standing; it repros for me in Chrome 42 at least,
and I expect probably up to the beginning of v8.  So really we have to introduce
a special case here, or otherwise simplify things more radically (somehow
separating the possibly-eval aspect from the lookup location).

Instead of VisitForStackValue(callee), we really do have to call out to some
helper that pushes not only the callee but the WithBaseObject as well; nasty but
there you go, and you can't unify with other cases because they use ICs.

[wingo, 2015-06-24 15:54:06]

The CQ bit was checked by wingo@igalia.com to run a CQ dry run

[wingo, 2015-06-24 15:54:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1202963005/#ps40001
(title: "Full-codegen impls for all arches")

[commit-bot: I haz the power, 2015-06-24 15:54:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1202963005/40001

[wingo, 2015-06-24 15:54:31]

Added full-codegen implementations for other arches; ptal :)

[commit-bot: I haz the power, 2015-06-24 16:21:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-24 16:21:24]

Dry run: This issue passed the CQ dry run.

[arv (Not doing code reviews), 2015-06-24 16:40:14]

LGTM

[wingo, 2015-06-24 16:45:48]

The CQ bit was checked by wingo@igalia.com

[commit-bot: I haz the power, 2015-06-24 16:46:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1202963005/40001

[commit-bot: I haz the power, 2015-06-24 16:47:57]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-24 16:48:11]

Patchset 3 (id:??) landed as
https://crrev.com/3c5f0db3a1768ade68108bf003676ce378d1cbdc
Cr-Commit-Position: refs/heads/master@{#29259}

[Michael Achenbach, 2015-06-24 19:08:03]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1201273004/ by machenbach@chromium.org.

The reason for reverting is: [Sheriff] Breaks layout tests. Please fix upstream
blink first.
http://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Mac/builds/574

Please consider extra blink trybots on a reland..