Bug 30347 – Uninitialized conditional in WebCore::CSSParser::validUnit with "width: %" style

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Matt Mueller]

LayoutTests/fast/css/invalid-percentage-property.html causes a valgrind "Conditional jump or move depends on uninitialised value(s)" error.

validUnit is called with the FNonNeg flag, which checks the fValue before checking anything else, but the "width: %" grammar does not set any fValue.

This does not seem to cause any misbehavior in this case since validUnit will always return false regardless if the FNonNeg check fails or the value->unit tests falls through.  However, it does create valgrind noise which is nice to avoid.

Very similar to bug 22772.

Will attach a patch which addresses this particular case be initializing fValue in the grammar, though I don't know if this is the best way to go about it.  validUnit could be refactored so the check is only done for units where it makes sense, though that might introduce a slight runtime or code size cost.

Chromium bug:
http://code.google.com/p/chromium/issues/detail?id=20939

[Matt Mueller]

Created attachment 41137 [details]
initialize fValue

[Darin Adler]

Comment on attachment 41137 [details]
initialize fValue

I think a better fix for this would be to move the negative number check after the switch statement in CSSParser::validUnit.

    if (b && unitflags & FNonNeg && value->fValue < 0)
        b = false;

[Matt Mueller]

Created attachment 41139 [details]
Move the non-negative check after the switch

Yeah, that does look more general.  Should fix 22772 too, though I don't have a test case for that.  I've updated the patch.

[Matt Mueller]

Created attachment 41140 [details]
Move the non-negative check after the switch

(Fixed changelog)

[WebKit Commit Bot]

Comment on attachment 41140 [details]
Move the non-negative check after the switch

Clearing flags on attachment: 41140

Committed r49609: <http://trac.webkit.org/changeset/49609>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Langley]

*** Bug 22772 has been marked as a duplicate of this bug. ***