Last Comment Bug 547875 - No OOM check on trace when creating a null closure in JSOP_LAMBDA
: No OOM check on trace when creating a null closure in JSOP_LAMBDA
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: general
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-22 16:41 PST by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-08-19 03:59 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Guard for OOM when creating null closures for JSOP_LAMBDA. (502 bytes, patch)
2011-08-09 21:17 PDT, Terrence Cole [:terrence]
dmandelin: review+
Ms2ger: checkin+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2010-02-22 16:41:02 PST

    
Comment 1 Josh Matthews [:jdm] (on vacation until Dec 5) 2011-04-03 10:54:46 PDT
I'm assuming this entails adding a guard statement with an OOM_EXIT.  Is a similar OOM check missing from http://mxr.mozilla.org/mozilla-central/source/js/src/jstracer.cpp#15742 ?
Comment 2 Terrence Cole [:terrence] 2011-08-09 21:17:18 PDT
Created attachment 552004 [details] [diff] [review]
Guard for OOM when creating null closures for JSOP_LAMBDA.

No significant performance difference.  All existing tests pass, but no new tests are included.  There are OOM tests for Function() in jit-test/tests/basic/testCompileScript.js, but it's not clear to me how I can apply that technique to testing creation of null closure lambdas.  Any hints would be appreciated.
Comment 3 David Mandelin [:dmandelin] 2011-08-10 17:14:51 PDT
Comment on attachment 552004 [details] [diff] [review]
Guard for OOM when creating null closures for JSOP_LAMBDA.

Review of attachment 552004 [details] [diff] [review]:
-----------------------------------------------------------------

Only needs one reviewer. Looks fine. I don't mind not having a test case on this particular patch--test cases for OOMs are kind of nasty because they run for a long time and often work differently on different platforms.
Comment 4 Marco Bonardo [::mak] 2011-08-19 03:13:07 PDT
http://hg.mozilla.org/mozilla-central/rev/5691363f380f

Note You need to log in before you can comment on or make changes to this bug.