Prevent invalid pre-parsing data passed in through the API from crashing V8.

Prevent invalid pre-parsing data passed in through the API from crashing V8.

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

[William Hesse, 2010-07-06 09:37:05]

Should we make a unit test using the API to send bad data?

[Kevin Millikin (Chromium), 2010-07-06 09:48:00]

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

http://codereview.chromium.org/2876046/diff/1/2#newcode3814
src/parser.cc:3814: ReportMessage("invalid preparser data", Vector<const
char*>::empty());
Drive by: don't you want this to be one of the property names in kMessages in
messages.js?

[Kasper Lund, 2010-07-06 09:53:11]

Please add a test case in test-api.cc that checks that you get the right kind of
exception when passing in malformed preparsing data.

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

http://codereview.chromium.org/2876046/diff/1/2#newcode3820
src/parser.cc:3820: ReportMessage("invalid preparser data", Vector<const
char*>::empty());
Please refactor this code and introduce a new helper function for reporting
invalid preparsing data. I agree with Kevin that you want this match something
in messages.js and that you should include the position or something (maybe both
the source position and the position in the preparsing data).

[William Hesse, 2010-07-07 08:31:29]

Comments addressed.  The unit test is hard to write cleanly because we don't
have access to the internals of ScriptDataImpl.  Shall we clean up the unit test
and commit it separately?  The unit test does work.

[Kasper Lund, 2010-07-07 09:59:49]

LGTM.

http://codereview.chromium.org/2876046/diff/19001/20002
File src/parser.cc (right):

http://codereview.chromium.org/2876046/diff/19001/20002#newcode137
src/parser.cc:137: void ReportInvalidPreparseData(Handle<String> name, bool
*ok);
bool *ok => bool* ok

http://codereview.chromium.org/2876046/diff/19001/20003
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/2876046/diff/19001/20003#newcode8571
test/cctest/test-api.cc:8571: // ScriptDataImpl private implementation details
I agree that this is not pretty, but it's much better than no testing. Thanks.
Feel free to clean it up in a separate CL.

http://codereview.chromium.org/2876046/diff/19001/20003#newcode8591
test/cctest/test-api.cc:8591: compiled_script = Script::New(source, NULL, sd);
You should really be running the compiled script and/or catching the exception
here. Please make sure you get the right exception.