Handle accessors when generating Error.stack...

Handle accessors when generating Error.stack

BUG=http://code.google.com/p/v8/issues/detail?id=798
TEST=test/mjsunit/regress/regress-798.js

Committed: http://code.google.com/p/v8/source/detail?r=5258

[Søren Thygesen Gjesse, 2010-08-02 15:37:27]

I did look at adding a isGetterCall/isSetterCall to the CallFrame, but we do not
have enough information to determine that.

[Lasse Reichstein, 2010-08-10 08:29:38]

LGTM if it works.

http://codereview.chromium.org/3082012/diff/1/2
File src/messages.js (right):

http://codereview.chromium.org/3082012/diff/1/2#newcode712
src/messages.js:712: this.receiver.__lookupSetter__(ownName) === this.fun ||
Is __lookup[GS]etter__ safe to use, or can it be changed by the user?
Would it be safer to use ES5 Object.getOwnPropertyDescriptor (+ perhaps
prototype traversal) instead?

http://codereview.chromium.org/3082012/diff/1/3
File test/mjsunit/regress/regress-798.js (right):

http://codereview.chromium.org/3082012/diff/1/3#newcode68
test/mjsunit/regress/regress-798.js:68: 
Try adding tests where users have changed __defineGetter__.

[Søren Thygesen Gjesse, 2010-08-12 14:05:47]

Please take another look, hopefully this is less fragile.

http://codereview.chromium.org/3082012/diff/1/2
File src/messages.js (right):

http://codereview.chromium.org/3082012/diff/1/2#newcode712
src/messages.js:712: this.receiver.__lookupSetter__(ownName) === this.fun ||
On 2010/08/10 08:29:38, Lasse Reichstein wrote:
> Is __lookup[GS]etter__ safe to use, or can it be changed by the user?
> Would it be safer to use ES5 Object.getOwnPropertyDescriptor (+ perhaps
> prototype traversal) instead?

You are right they can be changed, but so can Object.getOwnPropertyDescriptor.
Changed to call the internal implementation:

ObjectLookupGetter.call(this.receiver, ownName)
ObjectLookupSetter.call(this.receiver, ownName)

http://codereview.chromium.org/3082012/diff/1/3
File test/mjsunit/regress/regress-798.js (right):

http://codereview.chromium.org/3082012/diff/1/3#newcode68
test/mjsunit/regress/regress-798.js:68: 
On 2010/08/10 08:29:38, Lasse Reichstein wrote:
> Try adding tests where users have changed __defineGetter__.

Did you mean __lookupGetter__? As I am no longer using that in messages.js there
is no need to test that?

[Lasse Reichstein, 2010-08-13 07:03:21]

LGTM

http://codereview.chromium.org/3082012/diff/1/2
File src/messages.js (right):

http://codereview.chromium.org/3082012/diff/1/2#newcode712
src/messages.js:712: this.receiver.__lookupSetter__(ownName) === this.fun ||
> ObjectLookupGetter.call(this.receiver, ownName)
> ObjectLookupSetter.call(this.receiver, ownName)

Do you know if these internal functions have the same Function.prototype as the
user functions (i.e., can "call" be clobbered)?
You could use
 %_CallFunction(this.receiver, ownName, ObjectLookupGetter)
just to be absolutely sure.

http://codereview.chromium.org/3082012/diff/1/3
File test/mjsunit/regress/regress-798.js (right):

http://codereview.chromium.org/3082012/diff/1/3#newcode68
test/mjsunit/regress/regress-798.js:68: 
I did, and you can ignore it now.

[Søren Thygesen Gjesse, 2010-08-13 08:31:19]

On 2010/08/13 07:03:21, Lasse Reichstein wrote:
> LGTM
> 
> http://codereview.chromium.org/3082012/diff/1/2
> File src/messages.js (right):
> 
> http://codereview.chromium.org/3082012/diff/1/2#newcode712
> src/messages.js:712: this.receiver.__lookupSetter__(ownName) === this.fun ||
> > ObjectLookupGetter.call(this.receiver, ownName)
> > ObjectLookupSetter.call(this.receiver, ownName)
> 
> Do you know if these internal functions have the same Function.prototype as
the
> user functions (i.e., can "call" be clobbered)?
> You could use
>  %_CallFunction(this.receiver, ownName, ObjectLookupGetter)
> just to be absolutely sure.
> 
> http://codereview.chromium.org/3082012/diff/1/3
> File test/mjsunit/regress/regress-798.js (right):
> 
> http://codereview.chromium.org/3082012/diff/1/3#newcode68
> test/mjsunit/regress/regress-798.js:68: 
> I did, and you can ignore it now.

You are right about Function.prototype.call for the natives can be overwritten,
and %_CallFunction should be used. However there are 50 usages of .call in the
natives, so this should be cleaned up in a separate change. I can see you
changed a few in r4653.