2007-08-21-05-trunk passes 2007-08-22-05-trunk ate my linux laptop with 1G ram and forced a power off. On a winxp box with 4G ram, it just hung the process. bug 392973 has jelly on its face.

The test case in question effectively checks that a recursion code like (function foo() { return foo(); })(); terminates with some exception. Changes in bug 392973 removed the hard-coded limit meaning that the recursion will only terminates when malloc returns null. Since on a computer with a swap that pretty much means denial-of-service, something has to be done about that.

Comment on attachment 277936 [details] [diff] [review] fix This is OK as a fix, but I would prefer to see a limit on max size of JSContext.stackPool or even on stackPool and tempPool combined.

Would prefer to wait for ActionMonkey stage N, with JITting and so on. The simple API that directly controls a cheaply tested, pre-existing measure (inlineCallCount in js_Interpret) seems best for now. Thanks, /be

(In reply to comment #4) > Would prefer to wait for ActionMonkey stage N, with JITting and so on. The > simple API that directly controls a cheaply tested, pre-existing measure > (inlineCallCount in js_Interpret) seems best for now. A limit on stackPool and tempPool would allow to test otherwise untested code paths with failed arena allocations. And it is easy to implement as pool can store a pointer to the limit and update it on every malloc/free.

Igor, feel free to steal this, but we probably could use your help on ActionMonkey more than here. I'm with you in wanting bug 392263 fixed for 1.9, and I have a few things to do for SpiderMonkey there. Also anything to avoid merge conflicts that does not cost too much. By this reasoning, is it worth doing more here now? I am open to argument, or just bug stealing by you ;-). /be

This is an implementation on that script stack limit accounting. The patch adds JSArenaPool.sizeLimit, a pointer to the limit which each malloc in arena decreases and each free increases. For all pools except JSRuntime.propertyArenaPool the pointer is set to JSContext.scriptStackSizeLimit.

The new version increases JS_DEFAULT_SCRIPT_STACK_LIMIT to 32MB so ./js1_5/Regress/regress-280769-2.js does not trigger out-of-memory error when compiling scripts. Apparently the test case uses that much of memory when compiling eval's script.

The new version accounts for the realloc call in the arena that the previous patch missed.

Fixing bad accounting bug in JS_ArenaRealloc.

Comment on attachment 278038 [details] [diff] [review] alternative script stack accounting v4 >+JS_PUBLIC_API(void) >+JS_SetScriptStackSizeLimit(JSContext *cx, size_t limit) >+{ >+ cx->scriptStackSizeLimit = limit; Suggest the following global renamings: s/scriptStackSizeLimit/scriptStackQuota/ s/\<sizeLimit\>/quotap/ (exact word match) s/limit/quota/ (in context of above) >+/* >+ * Set the fencepost (one greater than the maximum) limit on the script stack Please lose the "fencepost (...)" language, it's wrong. s/limit/quota/ Great otherwise, r+ applies to next patch. /be

The patches in the bug allow to set quota on the size of stack-like structures that SpiderMonkey uses to compile and execute scripts. With a low values for quota it should help to test with the fuzzier the code paths that otherwise are taken only on real out-of-memory.

The patch with limit->quota renames.

More comment text fixes.

The new version adds scriptStackQuota function to js shell to set/query script stack quota. Here is a usage example: ~/m/trunk/mozilla/js/src $ cat ~/m/y.js var recusrionLevel = 0; function f() { ++recusrionLevel; f(); } try { f(); } catch (ex) { print("Maximum recursion level: "+recusrionLevel); print("Exception: "+ex); } ~/m/trunk/mozilla/js/src $ js -f ~/m/y.js Maximum recursion level: 261503 Exception: InternalError: stack overflow in f ~/m/trunk/mozilla/js/src $ js -e 'scriptStackQuota(0);' -f ~/m/y.js Maximum recursion level: 63 Exception: InternalError: stack overflow in f Note that scriptStackQuota(0) effectively limits the script stack to the currently consumed value. This is the reason why in the above example the shell was able to compile ~/m/y.js despite zero quota which also restricts the buffers for script compilation. To trigger OOM during the compilation phase one has to use some complex script after calling scriptStackQuota(0) or use eval: ~/m/trunk/mozilla $ js -e 'scriptStackQuota(0);' -e 'eval("var x = Math.sin(0)")' -e:1: out of memory Here is a test for the decompiler: ~/m/trunk/mozilla $ js -e 'function f() {}; scriptStackQuota(0); ""+f;' -e:1: out of memory

Comment on attachment 278228 [details] [diff] [review] alternative script stack accounting v5 >+ JS_FN("scriptStackQuota", ScriptStackQuota, 0,0,0,0), Would prefer stackQuota as the function name, StackQuote as the native name, just for similar brevity to surrounding shell functions. >+JS_SetScriptStackQuota(JSContext *cx, size_t limit) >+{ >+ cx->scriptStackQuota = limit; s/limit/quota/g >+ jsuword boff, aoff, extra, hdrsz, gross, grossExtra; Suggest s/grossExtra/growth/g. r+a=me with these picked. /be

The new version addresses the nits and renames scriptStackQuota->stackQuota: js> eval("1+1") 2 js> stackQuota(0) js> eval("1+1") eval("1+1") typein:3: out of memory

The new version fixes bad indentation in the previous one.

I checked in the patch from comment 18 to the trunk: http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1188253203&maxdate=1188253320&who=igor%mir2.org

verified fixed 1.9.0 linux/mac*/windows