Implement correct checking for inherited readonliness on assignment.

Implement correct checking for inherited readonliness on assignment.

Removes 6 out of 8 of our remaining unintentional failures on test262.

Also fixes treatment of inherited setters added after the fact.

Specifically:

- In the runtime, when looking for setter callbacks in the prototype chain,
  also look for read-only properties. If one is found, reject (exception in
  strict mode). If a proxy is found, invoke proper trap.
  Note: this folds in the CanPut function from the spec and avoids an extra
  lookup over the prototype chain.

- In generated code for stores, insert a test for the maps from the prototype
  chain, but only up to the object where the property already exists (which
  may be the object itself).
  In Hydrogen, if the found property is read-only or not cacheable (e.g. a
  proxy), bail out; in a stub, generate an unconditional miss (to get an
  exception in strict mode).

- Add test cases and adapt existing test expectations.

R=mstarzinger@chromium.org
BUG=
TEST=

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

[Michael Starzinger, 2012-05-30 13:08:55]

In general this LGTM. Only a few nits to address.

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/hydrogen.cc
File src/hydrogen.cc (right):

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/hydrogen.cc#new...
src/hydrogen.cc:4278: CHECK_ALIVE({});
Can we rewrite that to use the following syntax. I think that's more in sync
with the rest of the code.

HInstruction* store;
CHECK_ALIVE(store = BuildStoreNamed(literal, value, property));

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/hydrogen.cc#new...
src/hydrogen.cc:4631: CHECK_ALIVE({});
Likewise. Also for the rest of the file.

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc#newc...
src/objects.cc:1769: result_object = SetPropertyViaPrototypes(
The method call should fit into one line if we break right after the '=' sign.

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc#newc...
src/objects.cc:2051: void JSObject::LookupCallbackSetterInPrototypes(String*
name,
Can we rename this to "LookupRealNamedProperty" or something similar? At least
the current name makes no sense anymore I think.

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc#newc...
src/objects.cc:2134: *done = !!(attr & READ_ONLY);
Why not not not use no negation?

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc#newc...
src/objects.cc:2150: ;  // Fall through
Fall through to nothing makes no sense, can we just use "break;" here?

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc#newc...
src/objects.cc:2944: MaybeObject* result_object = SetPropertyViaPrototypes(
The method call should fit into one line if we break right after the '=' sign.

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/mjsunit/fuzz-a...
File test/mjsunit/fuzz-accessors.js (right):

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/mjsunit/fuzz-a...
test/mjsunit/fuzz-accessors.js:69: print("running");
I think we should just remove all of this debugging code again.

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/mjsunit/regres...
File test/mjsunit/regress/regress-334.js (right):

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/mjsunit/regres...
test/mjsunit/regress/regress-334.js:50: print(i);
Can we remove this debugging code again.

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/test262/test26...
File test/test262/test262.status (left):

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/test262/test26...
test/test262/test262.status:51: 15.2.3.6-4-420: FAIL
I like that!!!

[rossberg, 2012-05-31 13:31:27]

PTAL.

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/hydrogen.cc
File src/hydrogen.cc (right):

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/hydrogen.cc#new...
src/hydrogen.cc:4278: CHECK_ALIVE({});
On 2012/05/30 13:08:55, Michael Starzinger wrote:
> Can we rewrite that to use the following syntax. I think that's more in sync
> with the rest of the code.
> 
> HInstruction* store;
> CHECK_ALIVE(store = BuildStoreNamed(literal, value, property));

Done.

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/hydrogen.cc#new...
src/hydrogen.cc:4631: CHECK_ALIVE({});
On 2012/05/30 13:08:55, Michael Starzinger wrote:
> Likewise. Also for the rest of the file.

Done.

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc#newc...
src/objects.cc:1769: result_object = SetPropertyViaPrototypes(
On 2012/05/30 13:08:55, Michael Starzinger wrote:
> The method call should fit into one line if we break right after the '=' sign.

Done.

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc#newc...
src/objects.cc:2051: void JSObject::LookupCallbackSetterInPrototypes(String*
name,
On 2012/05/30 13:08:55, Michael Starzinger wrote:
> Can we rename this to "LookupRealNamedProperty" or something similar? At least
> the current name makes no sense anymore I think.

A method of that name already exists, but does something slightly different.
However, it seems that the difference was rather incidental, so I just removed
this lookup function altogether.

Achievement unlocked: simplified one instance of property handling.

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc#newc...
src/objects.cc:2134: *done = !!(attr & READ_ONLY);
On 2012/05/30 13:08:55, Michael Starzinger wrote:
> Why not not not use no negation?

Standard JS idiom for (explicitly) converting to a proper bool. ;)

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc#newc...
src/objects.cc:2150: ;  // Fall through
On 2012/05/30 13:08:55, Michael Starzinger wrote:
> Fall through to nothing makes no sense, can we just use "break;" here?

Done.

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc#newc...
src/objects.cc:2944: MaybeObject* result_object = SetPropertyViaPrototypes(
On 2012/05/30 13:08:55, Michael Starzinger wrote:
> The method call should fit into one line if we break right after the '=' sign.

Done.

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/mjsunit/fuzz-a...
File test/mjsunit/fuzz-accessors.js (right):

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/mjsunit/fuzz-a...
test/mjsunit/fuzz-accessors.js:69: print("running");
On 2012/05/30 13:08:55, Michael Starzinger wrote:
> I think we should just remove all of this debugging code again.

Done.

[Michael Starzinger, 2012-06-01 08:34:04]

LGTM.

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/10388047/diff/1015/src/objects.cc#newc...
src/objects.cc:2051: void JSObject::LookupCallbackSetterInPrototypes(String*
name,
On 2012/05/31 13:31:28, rossberg wrote:
> On 2012/05/30 13:08:55, Michael Starzinger wrote:
> > Can we rename this to "LookupRealNamedProperty" or something similar? At
least
> > the current name makes no sense anymore I think.
> 
> A method of that name already exists, but does something slightly different.
> However, it seems that the difference was rather incidental, so I just removed
> this lookup function altogether.
> 
> Achievement unlocked: simplified one instance of property handling.

Nice.

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/mjsunit/regres...
File test/mjsunit/regress/regress-334.js (right):

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/mjsunit/regres...
test/mjsunit/regress/regress-334.js:50: print(i);
On 2012/05/30 13:08:55, Michael Starzinger wrote:
> Can we remove this debugging code again.

I think you missed that one.

[rossberg, 2012-06-01 09:42:47]

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/mjsunit/regres...
File test/mjsunit/regress/regress-334.js (right):

https://chromiumcodereview.appspot.com/10388047/diff/1015/test/mjsunit/regres...
test/mjsunit/regress/regress-334.js:50: print(i);
On 2012/06/01 08:34:04, Michael Starzinger wrote:
> On 2012/05/30 13:08:55, Michael Starzinger wrote:
> > Can we remove this debugging code again.
> 
> I think you missed that one.

Indeed, thanks!