[Webkit-unassigned] [Bug 65290] DFG speculative JIT does not implement load elimination for GetById

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=65290





--- Comment #11 from Oliver Hunt <oliver at apple.com>  2011-07-29 17:25:34 PST ---
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Oliver, could you eyeball the runtime changes to JSObject etc in this patch?
> > > 
> > > Let me summarize.  We need new get/put methods that will not cause side effects (e.g. call getter/setters), that can be used from the speculative path and relied on not to cause arbitrary code execution.
> > > 
> > > Filip has added a putLimitedSideEffect methods which will only write to regular properties on instances of the JSFinalObject class.  This seems safe.
> > > 
> > > getNoSideEffect is a little more subtle.  We've prohibited API objects (since they can do anything!) via the newly added propertyAccessesMayCauseArbitrarySideEffects() check.  Otherwise, this patch will walk the proto chain calling fastGetOwnPropertySlot.  Once it has the property slot it calls a new getValueNoSideEffect(), which won't call getters on it.  Does this sound safe to you?  Do we need guard against any objects in WebCore triggering arbitrary code execution from within a getOwnPropertySlot?
> > 
> > There are a reasonable number of objects in webcore that override getOwnPropertySlot, some of them have side effects on the _get_, most notable they may throw exceptions due to same origin violations.
> > 
> > I believe the side-effect free lookup should do an explicit check for the overridesGetOwnPropertySlot() flag.  propertyAccessesMayCauseArbitrarySideEffects seems very fragile as it is essentially a blacklist, and historically blacklist based filtering has been problematic for us.  We're better off defaulting to property access being unsafe, and then opting in to this optimisation on a case by case basis (this only applies to classes that set overridesGetOwnPropertySlot).
> > 
> > TLDR;  this code is currently unsafe, there are a number of objects in the DOM where getOwnPropertySlot has immediate side effects
> 
> Is this summary true even if exceptions are fine?  By "no side effect" we mean that getOwnPropertySlot() will not modify the heap in some arbitrary way prior to returning.  And by modifying the heap we specifically mean: modify properties that don't involve getters (custom or not).

My point was that you're relying on _all_ code conforming to an invariant we can't enforce statically at compile time.  If you were okay with exceptions being thrown you'd need to ensure you always cleared the exception slot.

I also just realised that getOwnPropertySlot on a JSFunction can modify the object among other things -- it should not be observable that the change has happened, but it does modify the object.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.