Bug 128406 - Fix bug in stack limit adjustments in JSLock.
Summary: Fix bug in stack limit adjustments in JSLock.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks: 128408
  Show dependency treegraph
 
Reported: 2014-02-07 15:17 PST by Mark Lam
Modified: 2014-02-07 18:04 PST (History)
6 users (show)

See Also:


Attachments
the patch. (19.10 KB, patch)
2014-02-07 15:31 PST, Mark Lam
ggaren: review-
Details | Formatted Diff | Diff
reduced patch with only the JSLock fix. (3.36 KB, patch)
2014-02-07 16:11 PST, Mark Lam
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2014-02-07 15:17:21 PST
This is the real fix for https://bugs.webkit.org/show_bug.cgi?id=128347 and will undo the workaround committed there.
Comment 1 Mark Lam 2014-02-07 15:31:10 PST
Created attachment 223505 [details]
the patch.
Comment 2 Geoffrey Garen 2014-02-07 15:47:36 PST
Comment on attachment 223505 [details]
the patch.

Let's turn this into three patches:

(1) A three-line patch that removes the problematic assignment of nullptr even if not dropping the lock, and removes the problematic testing of "m_vm->stackPointerAtVMEntry == entryStackPointer".

(2) A mechanical patch that re-enables stack checking.

(3) The re-architectural you've proposed here, which removes the iOS-specific code path.

For (3), I think we need a little research into why that path currently exists, and is iOS-specific. I believe it was needed to fix some nasty bugs -- so we shouldn't just remove it without knowledge of why it's there.
Comment 3 Mark Lam 2014-02-07 16:11:42 PST
Created attachment 223509 [details]
reduced patch with only the JSLock fix.
Comment 4 Geoffrey Garen 2014-02-07 16:15:07 PST
Comment on attachment 223509 [details]
reduced patch with only the JSLock fix.

r=me
Comment 5 Mark Lam 2014-02-07 16:19:57 PST
Thanks.  Landed in r163661: <http://trac.webkit.org/r163661>.
Comment 6 Michael Saboff 2014-02-07 17:29:04 PST
Comment on attachment 223509 [details]
reduced patch with only the JSLock fix.

This patch should have deleted entryStackPointer since it is no longer needed.
Comment 7 Mark Lam 2014-02-07 17:30:48 PST
(In reply to comment #6)
> (From update of attachment 223509 [details])
> This patch should have deleted entryStackPointer since it is no longer needed.

Took care of that in https://webkit.org/b/128413, fixed in <http://trac.webkit.org/r163665>.
Comment 8 Mark Lam 2014-02-07 18:04:00 PST
ref: <rdar://problem/16008492>