[strong] Implement strong mode restrictions on property access

[strong] Implement strong mode restrictions on property access

Implements the strong mode proposal's restrictions on property access.

To be fully explored in a followup: proxies, interceptors, access checks, load from super

BUG=v8:3956
LOG=N

Committed: https://crrev.com/85dbfb9a389e7b21bd2a63862202ee97fc5d7982
Cr-Commit-Position: refs/heads/master@{#29109}

[conradw, 2015-06-08 16:38:25]

conradw@chromium.org changed reviewers:
+ mstarzinger@chromium.org, mvstanton@chromium.org, rossberg@chromium.org,
verwaest@chromium.org

[conradw, 2015-06-08 16:38:26]

PTAL

[Michael Starzinger, 2015-06-09 10:54:14]

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
File src/compiler/js-operator.cc (right):

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:194: lhs.contextual_mode() == rhs.contextual_mode()
&&
This operator should be adapted as well.

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:206: return base::hash_combine(p.name(),
p.contextual_mode(), p.feedback());
This operator should be adapted as well.

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:211: return os << Brief(*p.name().handle()) << ", "
<< p.contextual_mode();
This operator should be adapted as well.

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:216: // Nothing special to print.
This operator should be adapted as well.

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:223: return lhs.feedback() == rhs.feedback();
This operator should be adapted as well.

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:240: return hash_value(p.feedback());
This operator should be adapted as well.

[Michael Starzinger, 2015-06-09 10:56:11]

https://codereview.chromium.org/1168093002/diff/120001/src/code-factory.h
File src/code-factory.h (right):

https://codereview.chromium.org/1168093002/diff/120001/src/code-factory.h#new...
src/code-factory.h:44: LanguageMode language_mode);
nit: I think the language mode should be the second argument, so that this
signature continues to be an extension of the KeyedLoadIC above.

[conradw, 2015-06-09 11:20:30]

https://codereview.chromium.org/1168093002/diff/120001/src/code-factory.h
File src/code-factory.h (right):

https://codereview.chromium.org/1168093002/diff/120001/src/code-factory.h#new...
src/code-factory.h:44: LanguageMode language_mode);
On 2015/06/09 10:56:11, Michael Starzinger wrote:
> nit: I think the language mode should be the second argument, so that this
> signature continues to be an extension of the KeyedLoadIC above.

Done.

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
File src/compiler/js-operator.cc (right):

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:194: lhs.contextual_mode() == rhs.contextual_mode()
&&
On 2015/06/09 10:54:13, Michael Starzinger wrote:
> This operator should be adapted as well.

Done.

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:206: return base::hash_combine(p.name(),
p.contextual_mode(), p.feedback());
On 2015/06/09 10:54:13, Michael Starzinger wrote:
> This operator should be adapted as well.

Done.

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:211: return os << Brief(*p.name().handle()) << ", "
<< p.contextual_mode();
On 2015/06/09 10:54:13, Michael Starzinger wrote:
> This operator should be adapted as well.

Done.

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:216: // Nothing special to print.
On 2015/06/09 10:54:13, Michael Starzinger wrote:
> This operator should be adapted as well.

Done.

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:223: return lhs.feedback() == rhs.feedback();
On 2015/06/09 10:54:13, Michael Starzinger wrote:
> This operator should be adapted as well.

Done.

https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operat...
src/compiler/js-operator.cc:240: return hash_value(p.feedback());
On 2015/06/09 10:54:13, Michael Starzinger wrote:
> This operator should be adapted as well.

Done.

[Toon Verwaest, 2015-06-09 11:58:04]

https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic-state.h
File src/ic/ic-state.h (right):

https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic-state.h#newc...
src/ic/ic-state.h:205: class ContextualModeBits : public
BitField<ContextualMode, 0, 1> {};
Why was this moved up? That looks odd...

https://codereview.chromium.org/1168093002/diff/140001/src/ic/x64/ic-x64.cc
File src/ic/x64/ic-x64.cc (right):

https://codereview.chromium.org/1168093002/diff/140001/src/ic/x64/ic-x64.cc#n...
src/ic/x64/ic-x64.cc:228: __ bind(&return_undefined);
This probably shouldn't be named return_undefined anymore, but rather "absent"
or so.

https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc#newcode130
src/objects.cc:130: MaybeHandle<Object> Object::GetProperty(LookupIterator* it,
Strength strength) {
It would be nicer if GetProperty etc would also get a LanguageMode. Both
DeleteProperty and SetProperty already get that. And if we do that, we could
encapsulate it in the LookupIterator. I see how this is a bit annoying since you
are saving bits though ... not sure.

https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc#newcode152
src/objects.cc:152: if (is_strong(strength)) {
What about extracting this in a helper method. See WriteToReadOnlyProperty for
an example.

https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-cl...
File src/runtime/runtime-classes.cc (right):

https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-cl...
src/runtime/runtime-classes.cc:265: if (!proto->IsJSReceiver()) {
Helper method. If you just change the return type to MaybeHandle<Object> and
ASSIGN_RETURN_FAILURE_ON_EXCEPTION in the client you can use the same here as in
objects.cc

https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-cl...
src/runtime/runtime-classes.cc:334: language_mode);
Not clear when you use strength and when language_mode ...

https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-ob...
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-ob...
src/runtime/runtime-object.cc:184: ASSIGN_RETURN_ON_EXCEPTION(
If you don't use result you can just return GetObjectProperty(...);

[mvstanton, 2015-06-09 12:46:44]

Hi Conrad, ic stuff looks pretty good, a couple comments.

https://codereview.chromium.org/1168093002/diff/140001/src/builtins.h
File src/builtins.h (right):

https://codereview.chromium.org/1168093002/diff/140001/src/builtins.h#newcode93
src/builtins.h:93: LoadICState::kStrongModeState)                               
           \
You don't need KeyedLoadIC_Initialize_Strong (and could delete
KeyedLoadIC_Initialize while you are at it if you like).

https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic.cc
File src/ic/ic.cc (right):

https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic.cc#newcode937
src/ic/ic.cc:937: return strong ?
isolate->builtins()->KeyedLoadIC_Initialize_Strong()
I think this shows some unfinished clean-up work now that vector ics area always
on for loads. We can't achieve UNINITIALIZED state here because of the statement
above ("if (initialization_state != MEGAMORPHIC)").

Suggests that you could eliminate KeyedLoadIC_Initialize() from the builtins,
and your strong variant then too.

[conradw, 2015-06-09 13:09:28]

https://codereview.chromium.org/1168093002/diff/140001/src/builtins.h
File src/builtins.h (right):

https://codereview.chromium.org/1168093002/diff/140001/src/builtins.h#newcode93
src/builtins.h:93: LoadICState::kStrongModeState)                               
           \
On 2015/06/09 12:46:44, mvstanton wrote:
> You don't need KeyedLoadIC_Initialize_Strong (and could delete
> KeyedLoadIC_Initialize while you are at it if you like).

Done.

https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic-state.h
File src/ic/ic-state.h (right):

https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic-state.h#newc...
src/ic/ic-state.h:205: class ContextualModeBits : public
BitField<ContextualMode, 0, 1> {};
On 2015/06/09 11:58:04, Toon Verwaest wrote:
> Why was this moved up? That looks odd...

Because of the initialisation order, if I want to be able to have a static
member kStrongModeState inititalised using the bitfield then the bitfield needs
to be declared first.

In StoreICState where something similar is done, the bitfield is declared as
public, in an order which gets around this. Not sure which is better.

https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic.cc
File src/ic/ic.cc (right):

https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic.cc#newcode937
src/ic/ic.cc:937: return strong ?
isolate->builtins()->KeyedLoadIC_Initialize_Strong()
On 2015/06/09 12:46:44, mvstanton wrote:
> I think this shows some unfinished clean-up work now that vector ics area
always
> on for loads. We can't achieve UNINITIALIZED state here because of the
statement
> above ("if (initialization_state != MEGAMORPHIC)").
> 
> Suggests that you could eliminate KeyedLoadIC_Initialize() from the builtins,
> and your strong variant then too.

Done.

https://codereview.chromium.org/1168093002/diff/140001/src/ic/x64/ic-x64.cc
File src/ic/x64/ic-x64.cc (right):

https://codereview.chromium.org/1168093002/diff/140001/src/ic/x64/ic-x64.cc#n...
src/ic/x64/ic-x64.cc:228: __ bind(&return_undefined);
On 2015/06/09 11:58:04, Toon Verwaest wrote:
> This probably shouldn't be named return_undefined anymore, but rather "absent"
> or so.

Done.

https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc#newcode130
src/objects.cc:130: MaybeHandle<Object> Object::GetProperty(LookupIterator* it,
Strength strength) {
On 2015/06/09 11:58:04, Toon Verwaest wrote:
> It would be nicer if GetProperty etc would also get a LanguageMode. Both
> DeleteProperty and SetProperty already get that. And if we do that, we could
> encapsulate it in the LookupIterator. I see how this is a bit annoying since
you
> are saving bits though ... not sure.

Yeah, this is kind of annoying. Putting more than a bit in the minor key means
that sloppy and strict versions of the stub would get cached separately though,
so it's hard to have these functions taking LanguageMode instead of Strength
without either lying or causing a regression.

https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc#newcode152
src/objects.cc:152: if (is_strong(strength)) {
On 2015/06/09 11:58:04, Toon Verwaest wrote:
> What about extracting this in a helper method. See WriteToReadOnlyProperty for
> an example.

Done.

https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-cl...
File src/runtime/runtime-classes.cc (right):

https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-cl...
src/runtime/runtime-classes.cc:265: if (!proto->IsJSReceiver()) {
On 2015/06/09 11:58:04, Toon Verwaest wrote:
> Helper method. If you just change the return type to MaybeHandle<Object> and
> ASSIGN_RETURN_FAILURE_ON_EXCEPTION in the client you can use the same here as
in
> objects.cc

Done.

https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-cl...
src/runtime/runtime-classes.cc:334: language_mode);
On 2015/06/09 11:58:04, Toon Verwaest wrote:
> Not clear when you use strength and when language_mode ...

Currently language_mode is used for runtime functions (and associated helpers)
that never get called from stubs, and strength is used for functions that could
be called from stubs. There's not a particularly good reason for this, it just
naturally fell out from trying to keep the full language mode around for as long
as possible.

Everything could be changed to talk in terms of Strength instead.

https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-ob...
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-ob...
src/runtime/runtime-object.cc:184: ASSIGN_RETURN_ON_EXCEPTION(
On 2015/06/09 11:58:04, Toon Verwaest wrote:
> If you don't use result you can just return GetObjectProperty(...);

Done.

[Toon Verwaest, 2015-06-09 13:38:43]

https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc#newcode130
src/objects.cc:130: MaybeHandle<Object> Object::GetProperty(LookupIterator* it,
Strength strength) {
On 2015/06/09 13:09:27, conradw wrote:
> On 2015/06/09 11:58:04, Toon Verwaest wrote:
> > It would be nicer if GetProperty etc would also get a LanguageMode. Both
> > DeleteProperty and SetProperty already get that. And if we do that, we could
> > encapsulate it in the LookupIterator. I see how this is a bit annoying since
> you
> > are saving bits though ... not sure.
> 
> Yeah, this is kind of annoying. Putting more than a bit in the minor key means
> that sloppy and strict versions of the stub would get cached separately
though,
> so it's hard to have these functions taking LanguageMode instead of Strength
> without either lying or causing a regression.

Since type feedback vectors, aren't there only 2 stubs per language mode? That
seems manageable...

[rossberg, 2015-06-10 08:28:08]

Very nice! Only two main comments:

1. As suggested by Toon and disussed offline, let's just use LanguageMode for
LoadICs.

2. Even more test coverage, see the comments below; also, perhaps a simple
cctest that checks interceptors.

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

https://codereview.chromium.org/1168093002/diff/160001/src/ic/x64/ic-x64.cc#n...
src/ic/x64/ic-x64.cc:194: Label done, in_bounds, return_absent;
Nit: just "absent", it doesn't necessarily return

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-element-mutate-backing-store.js (right):

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element-mutate-backing-store.js:7: // TODO(conradw):
Track implementation of strong bit for other objects, add
See above

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element-mutate-backing-store.js:10: function
getObjects() {
See above

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element-mutate-backing-store.js:202: for (let func of
readStrong) {
Yeah, a quintuple loop like this is probably producing quite a long-running
test. Maybe you don't need to test the full matrix, but just vary a few of the
dimensions at a time?

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-element.js (right):

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element.js:7: // TODO(conradw): Track implementation of
strong bit for other objects, add
Currently, the semantics does not rely on that, does it?

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element.js:10: function getObjects() {
Other interesting cases to add:

- arguments (both strict and sloppy)
- typed arrays
- a non-standard builtin instance (e.g. of Date)
- some builtins (Object, Array, Function)
- primitives (numbers, bools, which should be wrapped implicitly)
- proxies (in a separate test file)

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element.js:200: // Attempting to access a property on
an objects with no defined properties
Nit: typo

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-property-mutate-backing-store.js (right):

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-property-mutate-backing-store.js:7: // TODO(conradw):
Track implementation of strong bit for other objects, add
See above

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-property-mutate-backing-store.js:10: function
getObjects() {
See above

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-property.js (right):

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-property.js:7: // TODO(conradw): Track implementation
of strong bit for other objects, add
See above

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-property.js:10: function getObjects() {
See above

[rossberg, 2015-06-10 08:29:42]

On 2015/06/10 08:28:08, rossberg wrote:
>
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
> test/mjsunit/strong/load-element-mutate-backing-store.js:7: // TODO(conradw):
> Track implementation of strong bit for other objects, add
> See above
> 
>
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
> test/mjsunit/strong/load-element-mutate-backing-store.js:10: function
> getObjects() {
> See above

Doh, why does Rietvield order the files differently here?

[conradw, 2015-06-10 12:03:23]

Currently refactoring everything back to use language mode instead of strength,
other changes are here.

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

https://codereview.chromium.org/1168093002/diff/160001/src/ic/x64/ic-x64.cc#n...
src/ic/x64/ic-x64.cc:194: Label done, in_bounds, return_absent;
On 2015/06/10 08:28:08, rossberg wrote:
> Nit: just "absent", it doesn't necessarily return

Done.

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-element-mutate-backing-store.js (right):

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element-mutate-backing-store.js:7: // TODO(conradw):
Track implementation of strong bit for other objects, add
On 2015/06/10 08:28:08, rossberg wrote:
> See above

Done.

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element-mutate-backing-store.js:10: function
getObjects() {
On 2015/06/10 08:28:08, rossberg wrote:
> See above

Done.

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element-mutate-backing-store.js:202: for (let func of
readStrong) {
On 2015/06/10 08:28:08, rossberg wrote:
> Yeah, a quintuple loop like this is probably producing quite a long-running
> test. Maybe you don't need to test the full matrix, but just vary a few of the
> dimensions at a time?

On reflection, I may have overstated how bad the test run times are. With the
current splitting, they take comfortably less than half (worst case) the time of
the longest running mjsunit test.

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-element.js (right):

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element.js:7: // TODO(conradw): Track implementation of
strong bit for other objects, add
On 2015/06/10 08:28:08, rossberg wrote:
> Currently, the semantics does not rely on that, does it?

Err, this was just a mindless copy and paste mistake.

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element.js:10: function getObjects() {
On 2015/06/10 08:28:08, rossberg wrote:
> Other interesting cases to add:
> 
> - arguments (both strict and sloppy)
> - typed arrays
> - a non-standard builtin instance (e.g. of Date)
> - some builtins (Object, Array, Function)
> - primitives (numbers, bools, which should be wrapped implicitly)
> - proxies (in a separate test file)

Done, the plan right now is to do proxies in a followup, along with the other
"odd" cases mentioned in this CL description.

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element.js:200: // Attempting to access a property on
an objects with no defined properties
On 2015/06/10 08:28:08, rossberg wrote:
> Nit: typo

Done.

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-property-mutate-backing-store.js (right):

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-property-mutate-backing-store.js:7: // TODO(conradw):
Track implementation of strong bit for other objects, add
On 2015/06/10 08:28:08, rossberg wrote:
> See above

Done.

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-property-mutate-backing-store.js:10: function
getObjects() {
On 2015/06/10 08:28:08, rossberg wrote:
> See above

Done.

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-property.js (right):

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-property.js:7: // TODO(conradw): Track implementation
of strong bit for other objects, add
On 2015/06/10 08:28:08, rossberg wrote:
> See above

Done.

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-property.js:10: function getObjects() {
On 2015/06/10 08:28:08, rossberg wrote:
> See above

Done.

[conradw, 2015-06-10 12:38:05]

Patchset #13 (id:240001) has been deleted

[conradw, 2015-06-10 16:25:23]

PTAL, all ports now completed

[Michael Starzinger, 2015-06-10 17:16:52]

LGTM on the TurboFan part, didn't look at the rest, please wait on approval from
other reviewers as well.

[rossberg, 2015-06-11 09:36:34]

LGTM modulo comments, but Toon & Michael Stanton should sign off as well.

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-element.js (right):

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element.js:10: function getObjects() {
On 2015/06/10 12:03:23, conradw wrote:
> On 2015/06/10 08:28:08, rossberg wrote:
> > Other interesting cases to add:
> > 
> > - arguments (both strict and sloppy)
> > - typed arrays
> > - a non-standard builtin instance (e.g. of Date)
> > - some builtins (Object, Array, Function)
> > - primitives (numbers, bools, which should be wrapped implicitly)
> > - proxies (in a separate test file)
> 
> Done, the plan right now is to do proxies in a followup, along with the other
> "odd" cases mentioned in this CL description.

Can you also add

- a typed array instance
- Object, Array, Function
- and I forogt an important one: the global object

What about strings, btw?

https://codereview.chromium.org/1168093002/diff/220001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-property-mutate-backing-store.js (right):

https://codereview.chromium.org/1168093002/diff/220001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-property-mutate-backing-store.js:154: print(object);
Stray debug output?

[conradw, 2015-06-11 13:48:32]

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-element.js (right):

https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-element.js:10: function getObjects() {
On 2015/06/11 09:36:34, rossberg wrote:
> On 2015/06/10 12:03:23, conradw wrote:
> > On 2015/06/10 08:28:08, rossberg wrote:
> > > Other interesting cases to add:
> > > 
> > > - arguments (both strict and sloppy)
> > > - typed arrays
> > > - a non-standard builtin instance (e.g. of Date)
> > > - some builtins (Object, Array, Function)
> > > - primitives (numbers, bools, which should be wrapped implicitly)
> > > - proxies (in a separate test file)
> > 
> > Done, the plan right now is to do proxies in a followup, along with the
other
> > "odd" cases mentioned in this CL description.
> 
> Can you also add
> 
> - a typed array instance
> - Object, Array, Function
> - and I forogt an important one: the global object
> 
> What about strings, btw?

Done, strings are handled in the "literals" list of the latest patch.

https://codereview.chromium.org/1168093002/diff/220001/test/mjsunit/strong/lo...
File test/mjsunit/strong/load-property-mutate-backing-store.js (right):

https://codereview.chromium.org/1168093002/diff/220001/test/mjsunit/strong/lo...
test/mjsunit/strong/load-property-mutate-backing-store.js:154: print(object);
On 2015/06/11 09:36:34, rossberg wrote:
> Stray debug output?

Done.

[Toon Verwaest, 2015-06-18 10:32:13]

lgtm

https://codereview.chromium.org/1168093002/diff/420001/src/lookup.h
File src/lookup.h (right):

https://codereview.chromium.org/1168093002/diff/420001/src/lookup.h#newcode227
src/lookup.h:227: 
spurious change

[conradw, 2015-06-18 11:48:26]

https://codereview.chromium.org/1168093002/diff/420001/src/lookup.h
File src/lookup.h (right):

https://codereview.chromium.org/1168093002/diff/420001/src/lookup.h#newcode227
src/lookup.h:227: 
On 2015/06/18 10:32:12, Toon Verwaest wrote:
> spurious change

Done.

[conradw, 2015-06-18 11:51:14]

The CQ bit was checked by conradw@chromium.org

[conradw, 2015-06-18 11:51:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
rossberg@chromium.org, mstarzinger@chromium.org, verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/1168093002/#ps460001
(title: "rebase")

[commit-bot: I haz the power, 2015-06-18 11:51:32]

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

[commit-bot: I haz the power, 2015-06-18 11:55:45]

Committed patchset #23 (id:460001)

[commit-bot: I haz the power, 2015-06-18 11:55:54]

Patchset 23 (id:??) landed as
https://crrev.com/85dbfb9a389e7b21bd2a63862202ee97fc5d7982
Cr-Commit-Position: refs/heads/master@{#29109}

[conradw, 2015-06-18 13:24:19]

A revert of this CL (patchset #23 id:460001) has been created in
https://codereview.chromium.org/1189153002/ by conradw@chromium.org.

The reason for reverting is: Speculative revert, maybe breaks GC-stress

http://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20-....