WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
ASSIGNED
125928
CStack: Fix stack checking and stack overflow handling
https://bugs.webkit.org/show_bug.cgi?id=125928
Summary
CStack: Fix stack checking and stack overflow handling
Mark Lam
Reported
2013-12-18 08:42:01 PST
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.
Attachments
a stack overflow test
(118 bytes, application/x-javascript)
2013-12-18 08:42 PST
,
Mark Lam
no flags
Details
View All
Add attachment
proposed patch, testcase, etc.
Mark Lam
Comment 1
2013-12-18 08:42:43 PST
Created
attachment 219540
[details]
a stack overflow test
Mark Lam
Comment 2
2013-12-18 23:54:51 PST
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.
Oliver Hunt
Comment 3
2013-12-19 08:49:01 PST
(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
Mark Lam
Comment 4
2014-01-13 17:13:45 PST
(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
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug