Fix some corner cases in skipping native methods using caller.

Fix some corner cases in skipping native methods using caller.
Committed: https://code.google.com/p/v8/source/detail?r=12439

[Yang, 2012-09-04 12:43:57]

LGTM with comments addressed.

https://chromiumcodereview.appspot.com/10911063/diff/1/src/accessors.cc
File src/accessors.cc (right):

https://chromiumcodereview.appspot.com/10911063/diff/1/src/accessors.cc#newco...
src/accessors.cc:754: while (potential_caller != NULL &&
potential_caller->IsBuiltin()) {
This suggest that potential_caller can become NULL, in which case setting caller
to potential_caller would cause NULL dereference later.

https://chromiumcodereview.appspot.com/10911063/diff/1/test/mjsunit/regress/r...
File test/mjsunit/regress/regress-145201.js (right):

https://chromiumcodereview.appspot.com/10911063/diff/1/test/mjsunit/regress/r...
test/mjsunit/regress/regress-145201.js:67: if (/native/i.test(caller) ||
/ADD/.test(caller)) {
Why do we only test for /ADD/, not for e.g. /MUL/?

[Erik Corry, 2012-09-05 07:22:33]

https://chromiumcodereview.appspot.com/10911063/diff/1/src/accessors.cc
File src/accessors.cc (right):

https://chromiumcodereview.appspot.com/10911063/diff/1/src/accessors.cc#newco...
src/accessors.cc:754: while (potential_caller != NULL &&
potential_caller->IsBuiltin()) {
On 2012/09/04 12:43:57, Yang wrote:
> This suggest that potential_caller can become NULL, in which case setting
caller
> to potential_caller would cause NULL dereference later.

I think it can't happen, but I will check for NULL before setting caller to
potential_caller.

https://chromiumcodereview.appspot.com/10911063/diff/1/test/mjsunit/regress/r...
File test/mjsunit/regress/regress-145201.js (right):

https://chromiumcodereview.appspot.com/10911063/diff/1/test/mjsunit/regress/r...
test/mjsunit/regress/regress-145201.js:67: if (/native/i.test(caller) ||
/ADD/.test(caller)) {
On 2012/09/04 12:43:57, Yang wrote:
> Why do we only test for /ADD/, not for e.g. /MUL/?

If we ever change the stringification of native functions so that the word
'native' does not appear, then this test would stop working (always pass). 
Testing for a specific function that should not appear is a safeguard against
that.

[Yang, 2012-09-05 07:24:52]

On 2012/09/05 07:22:33, Erik Corry wrote:
> https://chromiumcodereview.appspot.com/10911063/diff/1/src/accessors.cc
> File src/accessors.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10911063/diff/1/src/accessors.cc#newco...
> src/accessors.cc:754: while (potential_caller != NULL &&
> potential_caller->IsBuiltin()) {
> On 2012/09/04 12:43:57, Yang wrote:
> > This suggest that potential_caller can become NULL, in which case setting
> caller
> > to potential_caller would cause NULL dereference later.
> 
> I think it can't happen, but I will check for NULL before setting caller to
> potential_caller.
> 
>
https://chromiumcodereview.appspot.com/10911063/diff/1/test/mjsunit/regress/r...
> File test/mjsunit/regress/regress-145201.js (right):
> 
>
https://chromiumcodereview.appspot.com/10911063/diff/1/test/mjsunit/regress/r...
> test/mjsunit/regress/regress-145201.js:67: if (/native/i.test(caller) ||
> /ADD/.test(caller)) {
> On 2012/09/04 12:43:57, Yang wrote:
> > Why do we only test for /ADD/, not for e.g. /MUL/?
> 
> If we ever change the stringification of native functions so that the word
> 'native' does not appear, then this test would stop working (always pass). 
> Testing for a specific function that should not appear is a safeguard against
> that.

lgtm