Bug 156512 – Crash at com.apple.JavaScriptCore: bool WTF::startsWith<WTF::StringImpl, WTF::StringImpl> + 8

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: CSS

[Antti Koivisto]

<style> [class^="randomstring"] { background: white; } </style>
<div class="" id="test"></div>
<script> document.getElementById("test").attributes.class.value = null; </script>

[Antti Koivisto]

rdar://problem/24220567

[Antti Koivisto]

We crash in compiled selector code.

[Antti Koivisto]

Created attachment 276270 [details]
patch

[Benjamin Poulain]

Comment on attachment 276270 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=276270&action=review

> Source/WebCore/cssjit/SelectorCompiler.cpp:2823
> +    if (attribute->value().isEmpty())
> +        return false;

Looks like this should be merged with the "AtomicStringImpl& valueImpl = *attribute->value().impl();" below.

[Antti Koivisto]

Looks like Attr.value path that Chris fixed was the only way to make a null attribute value. All normal paths treat null value as "remove the attribute". I think we can do without the null check, I'll just land the test case here.

*** This bug has been marked as a duplicate of bug 156515 ***

[Antti Koivisto]

Landed the test in https://trac.webkit.org/r199428

[Darin Adler]

Comment on attachment 276270 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=276270&action=review

>> Source/WebCore/cssjit/SelectorCompiler.cpp:2823
>> +        return false;
> 
> Looks like this should be merged with the "AtomicStringImpl& valueImpl = *attribute->value().impl();" below.

Checking for the null string would be more efficient than checking for the empty string for the non-null, non-empty case. So unless we want to optimize the empty string case, I suggest using isNull or checking if impl() is nullptr instead.

[Darin Adler]

Oh, looks like we decided not to land this change and instead prevent null from ever getting here. Great!

[Darin Adler]

Comment on attachment 276270 [details]
patch

Clearing the review flag because we decided not to land this.