18538 2008-04-16 12:28:28 -0700 Uninitialized Value object when parsing '%' crashes an optimized ARM build 2017-03-25 19:09:08 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Mac OS X 10.5 RESOLVED CONFIGURATION CHANGED P2 Normal --- 1 phanna mitz gyuyoung webkit oldest_to_newest 77843 0 phanna 2008-04-16 12:28:28 -0700 Changelist 26482 introduced the ability to skip attribute values that are single '%' characters. This created an uninitialized Value object added to the valueList. This object caused a crash on an arm release build. I have a patch that I have tested against the original bug. 77844 1 20599 phanna 2008-04-16 12:29:27 -0700 Created attachment 20599 Patch for CSSGrammar.y 77887 2 20599 eric 2008-04-16 21:43:45 -0700 Comment on attachment 20599 Patch for CSSGrammar.y Is it possible to make a test case for this? Maybe: width: %; and then asking via getComputedStyle(element, null).width, or similar? 77909 3 phanna 2008-04-17 05:11:38 -0700 Sure. I'm not very familiar with writing a layout test though. Where should I put it? (In reply to comment #2) > (From update of attachment 20599 [edit]) > Is it possible to make a test case for this? > > Maybe: > width: %; > > and then asking via getComputedStyle(element, null).width, or similar? > 77914 4 phanna 2008-04-17 05:58:19 -0700 Looking through the codebase I found LayoutTests/fast/css/invalid-percentage-property.html which uses width: %; Should I augment this test or is it sufficient? (In reply to comment #2) > (From update of attachment 20599 [edit]) > Is it possible to make a test case for this? > > Maybe: > width: %; > > and then asking via getComputedStyle(element, null).width, or similar? > 78619 5 mjs 2008-04-22 22:39:21 -0700 Does the test case you mention crash without your fix, or otherwise change in result with your fix? Ideally we want a test case that crashes without (but maybe the bug is latent). 78676 6 phanna 2008-04-23 10:39:14 -0700 Unfortunately this only crashed for me on an optimized arm build. It crashed because it tried to create a String from an uninitialized ParseString with a huge length. Safari does not crash in debug or release mode. The only way I found out why our port crashed is by using Valgrind on a linux debug build and then tracing through the code. The test case doesn't render any different with or without my fix. 79228 7 20599 mitz 2008-04-29 10:21:05 -0700 Comment on attachment 20599 Patch for CSSGrammar.y r=me 79230 8 phanna 2008-04-29 10:45:58 -0700 Can someone with commit access land this patch for me? 79761 9 rwlbuis 2008-05-06 11:08:51 -0700 I get a changed end result for LayoutTests/fast/css/invalid-percentage-property.html. It does not look like an improvement, for starters it does not match FF3 and Opera. Maybe Mitz can have a look. Cheers, Rob. 80104 10 21063 mrowe 2008-05-10 23:44:03 -0700 Created attachment 21063 Changed test result 80105 11 mrowe 2008-05-10 23:45:05 -0700 Dan, can you take a look at the changed result please? 80210 12 20599 mitz 2008-05-12 11:59:33 -0700 Comment on attachment 20599 Patch for CSSGrammar.y Sorry, changing to r-. WebKit should match other engines on that test. 87139 13 webkit 2008-07-28 03:48:03 -0700 The proposed patch was completely incorrect. It forces WebKit to treat "width: %" like "width 0%" which it violates CSS 2.1 specification.. Current WebKit behavior is correct. I think we should close this bug report. 87144 14 phanna 2008-07-28 05:02:26 -0700 Maybe treating '%' as '0%' is incorrect, but leaving an uninitialized variable can cause WebKit to crash. Instead of closing the bug, maybe there is another solution that does not violate the CSS 2.1 spec. 1291288 15 mitz 2017-03-25 19:09:08 -0700 WebKit no longer uses the CSS parser that had this issue. 20599 2008-04-16 12:29:27 -0700 2010-06-10 18:49:21 -0700 Patch for CSSGrammar.y cssgrammar.patch text/plain 988 phanna PyAgICAgIGNzc2dyYW1tYXIucGF0Y2gKSW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9nCShyZXZpc2lvbiAzMTk1NCkKKysrIFdlYkNvcmUvQ2hh bmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTIgQEAKKzIwMDgtMDQtMTYgIFBhdHJp Y2sgSGFubmEgIDxwaGFubmFAZW1haWwudW5jLmVkdT4KKworICAgICAgICBGaXggYW4gdW5pbml0 aWFsaXplZCBWYWx1ZSB3aGVuIHBhcnNpbmcgYSBzaW5nbGUgJyUnIHZhbHVlLiBUaGlzIHdhcwor ICAgICAgICBkaXNjb3ZlcmVkIG9uIGFuIGFybSBidWlsZCB3aXRoIGhpZ2ggb3B0aW1pemF0aW9u cy4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGNz cy9DU1NHcmFtbWFyLnk6CisKIDIwMDgtMDQtMTUgIEFkYW0gUm9iZW4gIDxhcm9iZW5AYXBwbGUu Y29tPgogCiAgICAgICAgIEFkZCBTdHJpbmcuZm9ybWF0CkluZGV4OiBXZWJDb3JlL2Nzcy9DU1NH cmFtbWFyLnkKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9jc3MvQ1NTR3JhbW1hci55CShyZXZpc2lv biAzMTk0NykKKysrIFdlYkNvcmUvY3NzL0NTU0dyYW1tYXIueQkod29ya2luZyBjb3B5KQpAQCAt MTA2NSw3ICsxMDY1LDcgQEAgdGVybToKICAgfCBmdW5jdGlvbiB7CiAgICAgICAkJCA9ICQxOwog ICB9Ci0gIHwgJyUnIG1heWJlX3NwYWNlIHt9IC8qIEhhbmRsZSB3aWR0aDogJTsgKi8KKyAgfCAn JScgbWF5YmVfc3BhY2UgeyAkJC5pZCA9IDA7ICQkLmZWYWx1ZSA9IDA7ICQkLnVuaXQgPSBDU1NQ cmltaXRpdmVWYWx1ZTo6Q1NTX1BFUkNFTlRBR0U7IH0gLyogSGFuZGxlIHdpZHRoOiAlOyAqLwog ICA7CiAKIHVuYXJ5X3Rlcm06Cg== 21063 2008-05-10 23:44:03 -0700 2008-05-10 23:44:03 -0700 Changed test result diff.txt text/plain 1016 mrowe LS0tIC90bXAvbGF5b3V0LXRlc3QtcmVzdWx0cy9mYXN0L2Nzcy9pbnZhbGlkLXBlcmNlbnRhZ2Ut cHJvcGVydHktZXhwZWN0ZWQudHh0CTIwMDgtMDUtMTAgMjM6NDQ6MjUuMDAwMDAwMDAwIC0wNzAw CisrKyAvdG1wL2xheW91dC10ZXN0LXJlc3VsdHMvZmFzdC9jc3MvaW52YWxpZC1wZXJjZW50YWdl LXByb3BlcnR5LWFjdHVhbC50eHQJMjAwOC0wNS0xMCAyMzo0NDoyNS4wMDAwMDAwMDAgLTA3MDAK QEAgLTE3LDYgKzE3LDExIEBACiAgICAgICAgICAgUmVuZGVySW5saW5lIHtQUk9QRVJUWX0gYXQg KDAsMCkgc2l6ZSAxOTR4MTgKICAgICAgICAgICAgIFJlbmRlclRleHQgeyN0ZXh0fSBhdCAoNTE0 LDApIHNpemUgMTk0eDE4CiAgICAgICAgICAgICAgIHRleHQgcnVuIGF0ICg1MTQsMCkgd2lkdGgg MTk0OiAiOiV9IHN0eWxlIGFyZSBpZ25vcmVkIGJ5IFNhZmFyaSIKLSAgICAgIFJlbmRlckJsb2Nr IHtIM30gYXQgKDAsMzYpIHNpemUgNzg0eDIyIFtjb2xvcj0jMDA4MDAwXQotICAgICAgICBSZW5k ZXJUZXh0IHsjdGV4dH0gYXQgKDAsMCkgc2l6ZSAyNTR4MjIKLSAgICAgICAgICB0ZXh0IHJ1biBh dCAoMCwwKSB3aWR0aCAyNTQ6ICJUaGlzIHRleHQgc2hvdWxkIHNob3cgaW4gZ3JlZW4uIgorICAg ICAgUmVuZGVyQmxvY2sge0gzfSBhdCAoMCwzNikgc2l6ZSAweDEzMiBbY29sb3I9IzAwODAwMF0K KyAgICAgICAgUmVuZGVyVGV4dCB7I3RleHR9IGF0ICgwLDApIHNpemUgNTV4MTMyCisgICAgICAg ICAgdGV4dCBydW4gYXQgKDAsMCkgd2lkdGggMzY6ICJUaGlzIgorICAgICAgICAgIHRleHQgcnVu IGF0ICgwLDIyKSB3aWR0aCAzMDogInRleHQiCisgICAgICAgICAgdGV4dCBydW4gYXQgKDAsNDQp IHdpZHRoIDU1OiAic2hvdWxkIgorICAgICAgICAgIHRleHQgcnVuIGF0ICgwLDY2KSB3aWR0aCA0 MjogInNob3ciCisgICAgICAgICAgdGV4dCBydW4gYXQgKDAsODgpIHdpZHRoIDE2OiAiaW4iCisg ICAgICAgICAgdGV4dCBydW4gYXQgKDAsMTEwKSB3aWR0aCA1MDogImdyZWVuLiI=