<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - REGRESSION: ASSERT (impl->isAtomic()) @ facebook.com"
href="https://bugs.webkit.org/show_bug.cgi?id=149965#c2">Comment # 2</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - REGRESSION: ASSERT (impl->isAtomic()) @ facebook.com"
href="https://bugs.webkit.org/show_bug.cgi?id=149965">bug 149965</a>
from <span class="vcard"><a class="email" href="mailto:fpizlo@apple.com" title="Filip Pizlo <fpizlo@apple.com>"> <span class="fn">Filip Pizlo</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=149965#c1">comment #1</a>)
<span class="quote">> I debugged this a little.
>
> In the crashing case, the AbstractInterpreter executes CheckIdent and *does
> not* find a constant. Then, the DFGConstantFoldingPhase executes CheckIdent
> and *does* find a constant. The DFGConstantFoldingPhase constant is string
> equal to the uid expected by CheckIdent, but it is a plain string and not an
> atomic string.</span >
It's more subtle than that.
The constant folding phase code is assuming that it is running after m_interpreter.execute(). It's saying things like, "oh hey this has to be an atomic string because we already did StringIdentUse filtering". It's true that if you had run edge filtering first, then StringIdentUse would have filtered an abstract value containing a plain string to the empty abstract value (representing the fact that we would have always exited). And that's exactly what happens inside AbstractInterpreter - the executeEffects() method that has the CheckIdent case runs after edge filtering, so the AbstractInterpreter sees an empty AbstractValue because the StringIdentUse filtered the non-ident string to the empty value.
The solution is to fix the constant folder to no longer assume that edge filtering already ran. If it sees a constant value, it should verify that the constant value is valid (i.e. would have passed edge filtering, i.e. would have been an atomic string).
Probably just replacing:
ASSERT(impl->isAtomic());
with:
if (!impl->isAtomic())
break;
would go a long way.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>