Bug 15733 – glyph-orientation-rounding-test crashes on TOT

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Eric Seidel (no email)]

glyph-orientation-rounding-test crashes on TOT

Adam was having trouble with this test earlier today  under windows... I guess now we know why:


WARNING: Temporarily changing your system color profile from "Generic RGB Profile" to "Generic RGB Profile".
This allows the WebKit pixel-based regression tests to have consistent color values across all machines.
The colors on your screen will change for the duration of the testing.

ERROR: unable to initialize with font (null) at not known
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:147 void WebCore::FontData::platformInit())
ERROR: Corrupt font detected, using (null) in place of (null) located at "not known".
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:154 void WebCore::FontData::platformInit())
ERROR: failed to set up font, using system font Ä;}†
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:161 void WebCore::FontData::platformInit())
ASSERTION FAILED: m_type <= CSS_DIMENSION
(/Stuff/Projects/WebKit/WebCore/css/CSSPrimitiveValue.cpp:403 double WebCore::CSSPrimitiveValue::getDoubleValue(short unsigned int))
Segmentation fault
LEAK: 2 SubresourceLoader
LEAK: 245 Node
LEAK: 2 Page
LEAK: 223 RenderObject
LEAK: 2 Frame
LEAK: 1167 KJS::Node

[Eric Seidel (no email)]

run-webkit-tests --guard --pixel svg/css/glyph-orientation-rounding-test.xhtml

crashes for me on r27439.  It does not crash w/o --pixel.

[Eric Seidel (no email)]

both --guard and --pixel are required to make it crash.  Looks like we're using free'd memory somewhere.

[Eric Seidel (no email)]

Bah!  This only seems to reproduce intermittently, even with --guard.

[Eric Seidel (no email)]

I can't seem to get a crash log out of it (even though I have my CrashLogPrefs set to "Developer").  If someone does get a crash log, please post it. :)

[Eric Seidel (no email)]

Hum.. eventually I should try running multiple copies of this test in the same DRT instance, I bet that might make it crash reliably.

[Eric Seidel (no email)]

WOW.  This turns out to be a *really* bad bug.  The problem is this generated line of code in JSCSSStyleDeclarationPrototypeFunctionGetPropertyCSSValue::callAsFunction:

    KJS::JSValue* result = toJS(exec, WTF::getPtr(imp->getPropertyCSSValue(propertyName)));

See anything wrong?

The crash is caused by the evilness that is:

    template <typename T> inline T* getPtr(const PassRefPtr<T>& p)

We're grabbing the pointer out of a PassRefPtr, but by the time it's returned, if that was the only ref to that pointer, the pointer has already been deleted (since the original PassRefPtr has gone out of scope).

This is *bad*.  Bad design.  We'll need to educate the bindings about PassRefPtrs, w/o using getPtr().

[Eric Seidel (no email)]

Hum... ap and mjs tell me this is actually a feature of C++, and that this usage of getPtr should be safe, since temporaries are guaranteed to live to the end of the expression.  Strange.

[Mark Rowe (bdash)]

<rdar://problem/5650686>

[Alexey Proskuryakov]

This crashes (actually, fails with an assertion) for me pretty reliably if I refresh this test several times in Safari. I couldn't quickly find the problem, though.

Eric - do you still need a crash log? It's pretty meaningless IMHO - clearly, we are dealing with freed memory.

[Darin Adler]

I got this crash today without --guard.

[Sam Weinig]

This assert was easily reproducible for me by running the test in the browser and repeatedly hitting refresh.

[Sam Weinig]

Created attachment 18271 [details]
backtrace

[Darin Adler]

That's an assertion failure about m_type, which probably means this is an overreleased object.

[Sam Weinig]

Fix landed in r29189.