Bug 110813 - [JSC] Upstream iOS Stack bound checking
Summary: [JSC] Upstream iOS Stack bound checking
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on: 110858
Blocks:
  Show dependency tree / graph
 
Reported: 2013-02-25 15:18 PST by Benjamin Poulain
Modified: 2013-02-26 01:11 PST (History)
3 users (show)

See Also:

Attachments
Patch (6.48 KB, patch)
2013-02-25 15:20 PST, Benjamin Poulain 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2013-02-25 15:18:46 PST 
[JSC] Upstream iOS Stack bound checking
Comment 1 Benjamin Poulain 2013-02-25 15:20:57 PST 
Created attachment 190137 [details]
Patch
Comment 2 Benjamin Poulain 2013-02-25 20:15:15 PST 
Comment on attachment 190137 [details]
Patch

Clearing flags on attachment: 190137

Committed r144004: <http://trac.webkit.org/changeset/144004>
Comment 3 Benjamin Poulain 2013-02-25 20:15:17 PST 
All reviewed patches have been landed.  Closing bug.
Comment 4 Oliver Hunt 2013-02-25 20:42:31 PST 
I'm sure the stack bounds used to be thread local, i'm not sure why that can't just be the rule..
Comment 5 Geoffrey Garen 2013-02-25 20:52:50 PST 
I must be missing something obvious, but I don't understand why the Parser and BytecodeGenerator can't cache the stack bounds. Those are temporary, stack-local objects. Even though WebKit in general may run on the main or web thread, we shouldn't every start parsing on one thread and then switch to another, right?
Comment 6 Oliver Hunt 2013-02-25 20:55:26 PST 
(In reply to comment #5)
> I must be missing something obvious, but I don't understand why the Parser and BytecodeGenerator can't cache the stack bounds. Those are temporary, stack-local objects. Even though WebKit in general may run on the main or web thread, we shouldn't every start parsing on one thread and then switch to another, right?

That was my thought.  Many moons ago that's essentially what they did.  Then gavin refactored, then the thread local disappeared.  I'm not sure why exactly.
Comment 7 Oliver Hunt 2013-02-25 20:55:54 PST 
(In reply to comment #5)
> I must be missing something obvious, but I don't understand why the Parser and BytecodeGenerator can't cache the stack bounds. Those are temporary, stack-local objects. Even though WebKit in general may run on the main or web thread, we shouldn't every start parsing on one thread and then switch to another, right?

(Also workers _do_ only run on a single thread)
Comment 8 Benjamin Poulain 2013-02-25 23:20:18 PST 
Sorry, I do not have a clue, I did not work on the original bug. I did this change to avoid an horrible workaround in StackBounds.

I'll try to find the original radar tomorrow.
Comment 9 Benjamin Poulain 2013-02-26 00:02:54 PST 
After looking at why that was needed in the first place, I think this is wrong.

Can you guys confirm we never enter the event loop or a WebKit callback from BytecodeGenerator and Parser? If that is the case, I will remove the iOS changes on both side.
Comment 10 Geoffrey Garen 2013-02-26 00:40:51 PST 
> Can you guys confirm we never enter the event loop or a WebKit callback from BytecodeGenerator and Parser?

Confirmed.
Comment 11 Benjamin Poulain 2013-02-26 01:11:02 PST 
And done. Thank you for double checking this.