Fixed regression http://code.google.com/p/v8/issues/detail?id=236.

Fixed regression http://code.google.com/p/v8/issues/detail?id=236.
The problem was that the case of 'undefined' script source wasn't
handled in Script::InitLineEnds.

[Søren Thygesen Gjesse, 2009-02-24 13:21:42]

LGTM

This could actually cause a crash all the way down in String::ToUC16Vector()?

v8::internal::String::ToUC16Vector()
v8::internal::Runtime::StringMatch()
v8::internal::Script::InitLineEnds()
v8::internal::Accessors::ScriptGetLineEnds()
...

On 2009/02/24 11:55:35, Mikhail Naganov wrote:
>

[Søren Thygesen Gjesse, 2009-02-24 13:23:52]

http://codereview.chromium.org/28068/diff/1/2
File src/objects.cc (right):

http://codereview.chromium.org/28068/diff/1/2#newcode6863
Line 6863: set_line_ends(*(Factory::NewArrayLiteral(0)));
Please use NewJSArray.

http://codereview.chromium.org/28068/diff/1/3
File test/cctest/test-compiler.cc (right):

http://codereview.chromium.org/28068/diff/1/3#newcode317
Line 317: CHECK_EQ(-1, script->GetLineNumber(-1));
As this bug originated from Chrome where source is external strings using an
empty external string in the test case as well might be a good idea.

[Mikhail Naganov, 2009-02-24 13:32:17]

On 2009/02/24 13:21:42, Søren Gjesse wrote:
> LGTM
> 
> This could actually cause a crash all the way down in String::ToUC16Vector()?
> 
> v8::internal::String::ToUC16Vector()
> v8::internal::Runtime::StringMatch()
> v8::internal::Script::InitLineEnds()
> v8::internal::Accessors::ScriptGetLineEnds()
> ...
> 
> On 2009/02/24 11:55:35, Mikhail Naganov wrote:
> >

Yes, I'm almost (99%) sure that the crash was in case of 'undefined'
Script::source. Because in that case source's length is random, and StringMatch
was entered, and sooner or later it was attempting to call a method of a null
object.

When I wrote my test, it was also crashing until I added the 'undefined' case
handling.

I also found 'script->source()->IsUndefined()' checks all around, so it seems to
be that this is a real-life case.

[Mikhail Naganov, 2009-02-24 13:41:01]

http://codereview.chromium.org/28068/diff/1/2
File src/objects.cc (right):

http://codereview.chromium.org/28068/diff/1/2#newcode6863
Line 6863: set_line_ends(*(Factory::NewArrayLiteral(0)));
On 2009/02/24 13:23:52, Søren Gjesse wrote:
> Please use NewJSArray.

Um, but 'NewArrayLiteral' uses NewJSArrayWithElements. Isn't it the same?

http://codereview.chromium.org/28068/diff/1/3
File test/cctest/test-compiler.cc (right):

http://codereview.chromium.org/28068/diff/1/3#newcode317
Line 317: CHECK_EQ(-1, script->GetLineNumber(-1));
On 2009/02/24 13:23:52, Søren Gjesse wrote:
> As this bug originated from Chrome where source is external strings using an
> empty external string in the test case as well might be a good idea.

Ok, will add such test. But it will be a new CL since I already commited this
one immediately after your LGTM ;)