30347 2009-10-13 16:20:08 -0700 Uninitialized conditional in WebCore::CSSParser::validUnit with "width: %" style 2009-10-15 16:55:28 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 0 mattm webkit-unassigned agl commit-queue oldest_to_newest 154484 0 mattm 2009-10-13 16:20:08 -0700 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 154485 1 41137 mattm 2009-10-13 16:24:40 -0700 Created attachment 41137 initialize fValue 154487 2 41137 darin 2009-10-13 16:36:09 -0700 Comment on attachment 41137 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; 154495 3 41139 mattm 2009-10-13 17:06:09 -0700 Created attachment 41139 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. 154497 4 41140 mattm 2009-10-13 17:09:42 -0700 Created attachment 41140 Move the non-negative check after the switch (Fixed changelog) 154724 5 41140 commit-queue 2009-10-14 23:12:47 -0700 Comment on attachment 41140 Move the non-negative check after the switch Clearing flags on attachment: 41140 Committed r49609: <http://trac.webkit.org/changeset/49609> 154725 6 commit-queue 2009-10-14 23:12:51 -0700 All reviewed patches have been landed. Closing bug. 154951 7 agl 2009-10-15 16:55:28 -0700 *** Bug 22772 has been marked as a duplicate of this bug. *** 41137 2009-10-13 16:24:40 -0700 2009-10-13 17:06:09 -0700 initialize fValue webkit_30347_01.diff text/plain 1013 mattm ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg MjJhYzVjOS4uM2FiZjJmZCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAwOS0xMC0xMyAgTWF0dCBNdWVsbGVy ICA8bWF0dG1AY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIEluaXRpYWxpemUgZlZhbHVlIGluICJ3aWR0aDogJSIgZ3JhbW1hciB0 byBhdm9pZCB2YWxncmluZCB1bmluaXRpYWxpc2VkIGNvbmRpdGlvbmFsIHJlZmVyZW5jZSBpbiBX ZWJDb3JlOjpDU1NQYXJzZXI6OnZhbGlkVW5pdC4gIFNlZSBodHRwOi8vY3JidWcuY29tLzIwOTM5 LgorCWh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMDM0NworCisgICAg ICAgIENvdmVyZWQgYnkgcnVubmluZyBMYXlvdXRUZXN0cy9mYXN0L2Nzcy9pbnZhbGlkLXBlcmNl bnRhZ2UtcHJvcGVydHkuaHRtbCB1bmRlciB2YWxncmluZC4KKworICAgICAgICAqIGNzcy9DU1NH cmFtbWFyLnk6CisKIDIwMDktMTAtMTMgIFNpbW9uIEZyYXNlciAgPHNpbW9uLmZyYXNlckBhcHBs ZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGFuIEJlcm5zdGVpbi4KZGlmZiAtLWdpdCBh L1dlYkNvcmUvY3NzL0NTU0dyYW1tYXIueSBiL1dlYkNvcmUvY3NzL0NTU0dyYW1tYXIueQppbmRl eCAxYzFmN2I0Li5kM2NkZmUxIDEwMDY0NAotLS0gYS9XZWJDb3JlL2Nzcy9DU1NHcmFtbWFyLnkK KysrIGIvV2ViQ29yZS9jc3MvQ1NTR3JhbW1hci55CkBAIC0xMzg3LDcgKzEzODcsNyBAQCB0ZXJt OgogICAgICAgJCQgPSAkMTsKICAgfQogICB8ICclJyBtYXliZV9zcGFjZSB7IC8qIEhhbmRsZSB3 aWR0aDogJTsgKi8KLSAgICAgICQkLmlkID0gMDsgJCQudW5pdCA9IDA7CisgICAgICAkJC5pZCA9 IDA7ICQkLnVuaXQgPSAwOyAkJC5mVmFsdWUgPSAwOwogICB9CiAgIDsKIAo= 41139 2009-10-13 17:06:09 -0700 2009-10-13 17:09:42 -0700 Move the non-negative check after the switch webkit_30347_02.diff text/plain 1377 mattm ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg MjJhYzVjOS4uODFlM2M4MyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAwOS0xMC0xMyAgTWF0dCBNdWVsbGVy ICA8bWF0dG1AY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIENoZWNrIEZOb25OZWcgYWZ0ZXIgdGhlIHVuaXQgc3dpdGNoIHRvIGF2 b2lkIHZhbGdyaW5kIHVuaW5pdGlhbGlzZWQgY29uZGl0aW9uYWwgcmVmZXJlbmNlIGluIFdlYkNv cmU6OkNTU1BhcnNlcjo6dmFsaWRVbml0LiAgU2VlIGh0dHA6Ly9jcmJ1Zy5jb20vMjA5MzkuCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMDM0NworCisg ICAgICAgIENvdmVyZWQgYnkgcnVubmluZyBMYXlvdXRUZXN0cy9mYXN0L2Nzcy9pbnZhbGlkLXBl cmNlbnRhZ2UtcHJvcGVydHkuaHRtbCB1bmRlciB2YWxncmluZC4KKworICAgICAgICAqIGNzcy9D U1NHcmFtbWFyLnk6CisKIDIwMDktMTAtMTMgIFNpbW9uIEZyYXNlciAgPHNpbW9uLmZyYXNlckBh cHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGFuIEJlcm5zdGVpbi4KZGlmZiAtLWdp dCBhL1dlYkNvcmUvY3NzL0NTU1BhcnNlci5jcHAgYi9XZWJDb3JlL2Nzcy9DU1NQYXJzZXIuY3Bw CmluZGV4IGQ3NjhiZGIuLmZkNmNiNGQgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvY3NzL0NTU1BhcnNl ci5jcHAKKysrIGIvV2ViQ29yZS9jc3MvQ1NTUGFyc2VyLmNwcApAQCAtNDA1LDkgKzQwNSw2IEBA IERvY3VtZW50KiBDU1NQYXJzZXI6OmRvY3VtZW50KCkgY29uc3QKIAogYm9vbCBDU1NQYXJzZXI6 OnZhbGlkVW5pdChDU1NQYXJzZXJWYWx1ZSogdmFsdWUsIFVuaXRzIHVuaXRmbGFncywgYm9vbCBz dHJpY3QpCiB7Ci0gICAgaWYgKHVuaXRmbGFncyAmIEZOb25OZWcgJiYgdmFsdWUtPmZWYWx1ZSA8 IDApCi0gICAgICAgIHJldHVybiBmYWxzZTsKLQogICAgIGJvb2wgYiA9IGZhbHNlOwogICAgIHN3 aXRjaCAodmFsdWUtPnVuaXQpIHsKICAgICBjYXNlIENTU1ByaW1pdGl2ZVZhbHVlOjpDU1NfTlVN QkVSOgpAQCAtNDUxLDYgKzQ0OCw4IEBAIGJvb2wgQ1NTUGFyc2VyOjp2YWxpZFVuaXQoQ1NTUGFy c2VyVmFsdWUqIHZhbHVlLCBVbml0cyB1bml0ZmxhZ3MsIGJvb2wgc3RyaWN0KQogICAgIGRlZmF1 bHQ6CiAgICAgICAgIGJyZWFrOwogICAgIH0KKyAgICBpZiAoYiAmJiB1bml0ZmxhZ3MgJiBGTm9u TmVnICYmIHZhbHVlLT5mVmFsdWUgPCAwKQorICAgICAgICBiID0gZmFsc2U7CiAgICAgcmV0dXJu IGI7CiB9CiAK 41140 2009-10-13 17:09:42 -0700 2009-10-14 23:12:47 -0700 Move the non-negative check after the switch webkit_30347_03.diff text/plain 1475 mattm ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg MjJhYzVjOS4uZTI2ZTQ2MiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNiBAQAorMjAwOS0xMC0xMyAgTWF0dCBNdWVsbGVy ICA8bWF0dG1AY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIENoZWNrIEZOb25OZWcgYWZ0ZXIgdGhlIHVuaXQgc3dpdGNoIHRvIGF2 b2lkIHZhbGdyaW5kIHVuaW5pdGlhbGlzZWQgY29uZGl0aW9uYWwgcmVmZXJlbmNlIGluIFdlYkNv cmU6OkNTU1BhcnNlcjo6dmFsaWRVbml0LiAgU2VlIGh0dHA6Ly9jcmJ1Zy5jb20vMjA5MzkuCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMDM0NworICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjI3NzIKKworICAg ICAgICBDb3ZlcmVkIGJ5IHJ1bm5pbmcgTGF5b3V0VGVzdHMvZmFzdC9jc3MvaW52YWxpZC1wZXJj ZW50YWdlLXByb3BlcnR5Lmh0bWwgdW5kZXIgdmFsZ3JpbmQuCisKKyAgICAgICAgKiBjc3MvQ1NT UGFyc2VyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkNTU1BhcnNlcjo6dmFsaWRVbml0KToKKwog MjAwOS0xMC0xMyAgU2ltb24gRnJhc2VyICA8c2ltb24uZnJhc2VyQGFwcGxlLmNvbT4KIAogICAg ICAgICBSZXZpZXdlZCBieSBEYW4gQmVybnN0ZWluLgpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9jc3Mv Q1NTUGFyc2VyLmNwcCBiL1dlYkNvcmUvY3NzL0NTU1BhcnNlci5jcHAKaW5kZXggZDc2OGJkYi4u ZmQ2Y2I0ZCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9jc3MvQ1NTUGFyc2VyLmNwcAorKysgYi9XZWJD b3JlL2Nzcy9DU1NQYXJzZXIuY3BwCkBAIC00MDUsOSArNDA1LDYgQEAgRG9jdW1lbnQqIENTU1Bh cnNlcjo6ZG9jdW1lbnQoKSBjb25zdAogCiBib29sIENTU1BhcnNlcjo6dmFsaWRVbml0KENTU1Bh cnNlclZhbHVlKiB2YWx1ZSwgVW5pdHMgdW5pdGZsYWdzLCBib29sIHN0cmljdCkKIHsKLSAgICBp ZiAodW5pdGZsYWdzICYgRk5vbk5lZyAmJiB2YWx1ZS0+ZlZhbHVlIDwgMCkKLSAgICAgICAgcmV0 dXJuIGZhbHNlOwotCiAgICAgYm9vbCBiID0gZmFsc2U7CiAgICAgc3dpdGNoICh2YWx1ZS0+dW5p dCkgewogICAgIGNhc2UgQ1NTUHJpbWl0aXZlVmFsdWU6OkNTU19OVU1CRVI6CkBAIC00NTEsNiAr NDQ4LDggQEAgYm9vbCBDU1NQYXJzZXI6OnZhbGlkVW5pdChDU1NQYXJzZXJWYWx1ZSogdmFsdWUs IFVuaXRzIHVuaXRmbGFncywgYm9vbCBzdHJpY3QpCiAgICAgZGVmYXVsdDoKICAgICAgICAgYnJl YWs7CiAgICAgfQorICAgIGlmIChiICYmIHVuaXRmbGFncyAmIEZOb25OZWcgJiYgdmFsdWUtPmZW YWx1ZSA8IDApCisgICAgICAgIGIgPSBmYWxzZTsKICAgICByZXR1cm4gYjsKIH0KIAo=