Inserted a missing string encoding check in lastIndexOf.

Inserted a missing string encoding check in lastIndexOf.

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

[Yang, 2011-08-19 13:38:52]

Hi Vitaly,

here is something I noticed when implementing stuff for slices. Please take a
look.

Cheers,

Yang

http://codereview.chromium.org/7685005/diff/1/src/runtime.cc
File src/runtime.cc (left):

http://codereview.chromium.org/7685005/diff/1/src/runtime.cc#oldcode3154
src/runtime.cc:3154: 
This check is obsolete as the caller in string.js already makes sure that this
cannot happen.

http://codereview.chromium.org/7685005/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7685005/diff/1/src/runtime.cc#newcode3163
src/runtime.cc:3163: 
The same check for cons string exists in Runtime_StringIndexOf. I quote you:
Possible scenario:
1. a cons is flattened (its first component is a flat ASCII string now)
2. the first component is extracted (we do it in some places to improve.
performance, could be hard to guard against)
3. the first component is externalized.
We have to guard against this case. Though I have to say that I haven't been
able to produce a test case for this scenario using javascript and the v8-API.
Could you give me some pointers here?

[Vitaly Repeshko, 2011-08-19 16:30:00]

We agreed Yang will write a cctest and restore the check in runtime.cc to keep
it safe even when not called from our .js files.


-- Vitaly

[Yang, 2011-08-22 11:54:58]

As it turned out, a similar test case already exists. I constructed it the way
that it would have failed without this patch.

I also contacted Lasse and we came to the conclusion that the way ToAsciiVector
and ToUC16Vector are implemented is unsafe as the encoding checking is currently
the responsibility of the caller. This is risky as we have seen in this CL, and
further examples exist, such as Runtime_StringSplit. No idea comes to mind how
to fix this though.

I will fix Runtime_StringSplit as soon as my other CL containing the more
thorough String::IsAsciiRepresentationUnderneath() is landed so I can use that.
Nevertheless it is probably a good idea to find a good way to deal with this or
at least agree on the status quo before I continue to try to deal with
externalized strings in slices. By the looks of it, I will have to change all
the places where ToAsciiVector and ToUC16Vector is called anyways, to deal with
the case that the encoding of the slice does not agree with the encoding of the
underlying parent string.

[Vitaly Repeshko, 2011-08-22 13:37:46]

LGTM

http://codereview.chromium.org/7685005/diff/5001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7685005/diff/5001/src/runtime.cc#newcode3167
src/runtime.cc:3167: // Extract flattened substrings of cons strings before
determining asciiness.
nit: Move the comment one line above.