LGTM. I would use assignment for the initialization of 'wrapper', but that's probably just me.
On Mon, Oct 6, 2008 at 2:58 PM, <[EMAIL PROTECTED]> wrote: > Reviewers: Kasper Lund, > > Description: > Fixed unsafe code where a GC could occour after a Handle had been > deferenced. > > instances->set(i, *GetScriptWrapper(script)); > > GetScriptWrapper can call GC. The failure have only been seen on ARM, > where > the g++ compiler pulls out the object from the instances handle to a > register > before calling GetScriptWrapper causing set to be called on an object > which > may have moved. > > Marked a test on ARM as no longer flaky, whereas two other fails > consistently > but that is no longer related to the problem fixed above. > > BUG=1308895 > > Please review this at http://codereview.chromium.org/6271 > > Affected files: > M src/runtime.cc > M test/mjsunit/mjsunit.status > > > Index: test/mjsunit/mjsunit.status > =================================================================== > --- test/mjsunit/mjsunit.status (revision 443) > +++ test/mjsunit/mjsunit.status (working copy) > @@ -57,11 +57,7 @@ > debug-step-stub-callfunction: FAIL > debug-stepin-constructor: FAIL > debug-step: FAIL > -regress/regress-998565: FAIL > - > -# Bug number 1308895: These tests pass on the ARM simulator, but > -# fail on the ARM Linux machine. > -debug-script-breakpoints: PASS || FAIL > -debug-scripts-request: PASS || FAIL > +debug-script-breakpoints: FAIL > debug-breakpoints: PASS || FAIL > > +regress/regress-998565: FAIL > Index: src/runtime.cc > =================================================================== > --- src/runtime.cc (revision 443) > +++ src/runtime.cc (working copy) > @@ -4698,7 +4698,8 @@ > // Convert the script objects to proper JS objects. > for (int i = 0; i < count; i++) { > Handle<Script> script(Script::cast(instances->get(i))); > - instances->set(i, *GetScriptWrapper(script)); > + Handle<JSValue> wrapper(GetScriptWrapper(script)); > + instances->set(i, *wrapper); > } > > // Return result as a JS array. > > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
