[Webkit-unassigned] [Bug 12535] Stack-optimizing compilers can trick GC into freeing in-use objects

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=12535





------- Comment #10 from huanr at yahoo.com  2007-02-01 22:07 PDT -------
(In reply to comment #9)
> (In reply to comment #4)
> > (In reply to comment #2)
> > 
> > Anrong, did you mean to say that the compiler may discard the reference to
> > 'baseObj' or to 'baseVal'? Your reasoning points to 'baseVal', not 'baseObj'.
> > Would this bug persist if toObject() made use of 'this', prohibiting the
> > compiler from optimizing out baseVal?
> > 
> > I don't think 'Collector::protect(baseVal);' is a very good solution. It's
> > inefficient, and it doesn't seem to address the root cause of the problem,
> > which may affect lots of different parts of the code.
> > 
> 
> Geoffrey, 
> 
> I meant baseVal. The bug goes away if toObject() made use of 'this' thus
> effectively lock baseVal. 
> Collector::protect(baseVal) fixes this code path. if this is a common pattern
> then other solutions being discussed here may be better if they have no global
> impact.
> 

The following fix has also been verified:

#if COMPILER(MSVC)
#pragma optimize("", off)
#endif
JSObject *StringImp::toObject(ExecState *exec) const
{
    const StringImp *ptr = this;
    return new
StringInstance(exec->lexicalInterpreter()->builtinStringPrototype(), ptr->val);
}
#if COMPILER(MSVC)
#pragma optimize("", on)
#endif

I agree this fix is better than Collector::protect as it addresses all calling
path. Turning off optimization has little cost here since the function is
simple enough. (It turns out the compiler will optimize the variable away even
we declare the local to be volatile)

I could provide the patch. 


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