125928 2013-12-18 08:42:01 -0800 CStack: Fix stack checking and stack overflow handling 2014-01-13 17:13:45 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified ASSIGNED P2 Normal --- 126007 126009 126036 126088 126109 126139 126140 126320 1 mark.lam mark.lam fpizlo ggaren mhahnenberg msaboff oliver oldest_to_newest 960947 0 mark.lam 2013-12-18 08:42:01 -0800 operationStackCheck() (in JITOpereations.cpp) should not be testing the JSStack for overflow. operationStackCheck() is only called when an overflow has been detected by JITted code. Since the C stack is not growable, there's no need to repeat the stack check before throwing a StackOverflowError. Running a small test (the attached test-eval.js), I see that the LLINT failed to detect a stack overflow and crashed after exhausting the stack. The baseline JIT and DFG fared better in detecting the overflow (with the above stack check in operationStackCheck() commented out), but fails to unwind the stack during handling of the StackOverflowError. They ended up in an infinite loop during the unwind. All of these need to be fixed. 960948 1 219540 mark.lam 2013-12-18 08:42:43 -0800 Created attachment 219540 a stack overflow test 961298 2 mark.lam 2013-12-18 23:54:51 -0800 Also need to search for all places that uses JSStack::grow() to do a stack check. These should either removed or #if ENABLE(LLINT_C_LOOP). In their place, we should be doing a limit check on the C stack. 961398 3 oliver 2013-12-19 08:49:01 -0800 (In reply to comment #2) > Also need to search for all places that uses JSStack::grow() to do a stack check. These should either removed or #if ENABLE(LLINT_C_LOOP). In their place, we should be doing a limit check on the C stack. Does JSStack::grow() ever make sense when using the stack? Assuming it doesn't we should just put #if ENABLE(LLINT_C_LOOP) around the definition and declaration. That would instantly make it impossible to use it accidetanly 967567 4 mark.lam 2014-01-13 17:13:45 -0800 (In reply to comment #3) > Does JSStack::grow() ever make sense when using the stack? Assuming it doesn't we should just put #if ENABLE(LLINT_C_LOOP) around the definition and declaration. That would instantly make it impossible to use it accidetanly FYI, JSStack::grow() was moved into a ENABLE(LLINT_C_LOOP) only section in r160982 <http://trac.webkit.org/r160982> which was landed for https://bugs.webkit.org/show_bug.cgi?id=126140. 219540 2013-12-18 08:42:43 -0800 2013-12-18 08:42:43 -0800 a stack overflow test test-eval.js application/x-javascript 118 mark.lam CnZhciBzcmMgPSAicHJpbnQoJ0luIGV2YWw6Jyk7XG4iICsKIiAgICBmdW5jdGlvbiBhKCkge1xu IiArCiIgICAgICAgIGEoKTtcbiIgKwoiICAgIH1cbiIgKwoiICAgIGEoKTtcbiIKOwpldmFsKHNy Yyk7Cg==