20069 2008-07-16 20:11:20 -0700 CSSPrimitiveValue::parserValue() returns deleted memory 2010-03-12 10:29:45 -0800 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED InRadar P2 Normal --- 1 eric webkit-unassigned hyatt mitz mjs oldest_to_newest 86139 0 eric 2008-07-16 20:11:20 -0700 CSSParserValue CSSPrimitiveValue::parserValue() const: case CSS_IDENT: { value.id = m_value.ident; String name = valueOrPropertyName(m_value.ident); value.string.characters = const_cast<UChar*>(name.characters()); value.string.length = name.length(); break; } This function returns the resulting "value" which has a weak pointer to the UChar buffer which must have been freed when the String went out of scope. I discovered this will trying to remove callers of StringImpl::characters() (see bug 20065) so that we can play around with using different storage techniques for StringImpl's data. The only code which ever uses this is: void CSSStyleSelector::addMatchedDeclaration(CSSMutableStyleDeclaration* decl) Which I'm not sure what it even does. It seems to be used for variable resolution? Maybe for dealing with inline styles? Unclear. Perhaps Hyatt can explain. Once we know how this code is used, it should be easy to produce a test case which will crash under MallocScribble. 86140 1 eric 2008-07-16 20:12:36 -0700 In case you're wondering, valueOrPropertyName returns a char* which is transparently copied to create a String: static const char* valueOrPropertyName(int valueOrPropertyID) { if (const char* valueName = getValueName(valueOrPropertyID)) return valueName; return getPropertyName(static_cast<CSSPropertyID>(valueOrPropertyID)); } Seems sad that it would be copied. :( 196943 2 ap 2010-03-06 16:05:15 -0800 <rdar://problem/7725534> 199093 3 50602 mitz 2010-03-12 09:12:53 -0800 Created attachment 50602 Change valueOrPropertyName() to return an AtomicString from a static table 199119 4 ap 2010-03-12 10:20:20 -0800 You probably could have added a test case with a "checkConsistency"-style assertion. 199124 5 mitz 2010-03-12 10:29:45 -0800 Fixed in <http://trac.webkit.org/projects/webkit/changeset/55914>. 50602 2010-03-12 09:12:53 -0800 2010-03-12 10:09:59 -0800 Change valueOrPropertyName() to return an AtomicString from a static table 7725534_r1.diff text/plain 3196 mitz SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1NTkwOSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTggQEAKKzIwMTAtMDMtMTIgIERhbiBCZXJuc3RlaW4gIDxtaXR6QGFwcGxlLmNv bT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICA8cmRh cjovL3Byb2JsZW0vNzcyNTUzND4gQ1NTUHJpbWl0aXZlVmFsdWU6OnBhcnNlclZhbHVlKCkgcmV0 dXJucyBkZWxldGVkIG1lbW9yeQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MjAwNjkKKworICAgICAgICBObyB0ZXN0IGFkZGVkLCBzaW5jZSB3aXRoIHRo ZSBDU1MgdmFyaWFibGVzIGZlYXR1cmUgZGlzYWJsZWQsIHRoZSBwb2ludGVyCisgICAgICAgIHRv IHRoZSBmcmVlZCBtZW1vcnkgaXMgbmV2ZXIgZGVyZWZlcmVuY2VkLgorCisgICAgICAgICogY3Nz L0NTU1ByaW1pdGl2ZVZhbHVlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OnZhbHVlT3JQcm9wZXJ0 eU5hbWUpOiBDaGFuZ2VkIHRvIHJldHVybiBhIGNvbnN0IEF0b21pY1N0cmluZyYgZnJvbQorICAg ICAgICBhIHN0YXRpYyB0YWJsZS4KKyAgICAgICAgKFdlYkNvcmU6OkNTU1ByaW1pdGl2ZVZhbHVl OjpwYXJzZXJWYWx1ZSk6IFVwZGF0ZWQgZm9yIHRoZSBhYm92ZSBjaGFuZ2UuCisKIDIwMTAtMDMt MTIgIERhbiBCZXJuc3RlaW4gIDxtaXR6QGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBi eSBEYXJpbiBBZGxlci4KSW5kZXg6IFdlYkNvcmUvY3NzL0NTU1ByaW1pdGl2ZVZhbHVlLmNwcAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBXZWJDb3JlL2Nzcy9DU1NQcmltaXRpdmVWYWx1ZS5jcHAJKHJldmlzaW9u IDU1OTA4KQorKysgV2ViQ29yZS9jc3MvQ1NTUHJpbWl0aXZlVmFsdWUuY3BwCSh3b3JraW5nIGNv cHkpCkBAIC0xMTYsMTEgKzExNiwzMCBAQCBQYXNzUmVmUHRyPENTU1ByaW1pdGl2ZVZhbHVlPiBD U1NQcmltaXRpCiAgICAgcmV0dXJuIGFkb3B0UmVmKG5ldyBDU1NQcmltaXRpdmVWYWx1ZSh2YWx1 ZSwgdHlwZSkpOwogfQogCi1zdGF0aWMgY29uc3QgY2hhciogdmFsdWVPclByb3BlcnR5TmFtZShp bnQgdmFsdWVPclByb3BlcnR5SUQpCitzdGF0aWMgY29uc3QgQXRvbWljU3RyaW5nJiB2YWx1ZU9y UHJvcGVydHlOYW1lKGludCB2YWx1ZU9yUHJvcGVydHlJRCkKIHsKLSAgICBpZiAoY29uc3QgY2hh ciogdmFsdWVOYW1lID0gZ2V0VmFsdWVOYW1lKHZhbHVlT3JQcm9wZXJ0eUlEKSkKLSAgICAgICAg cmV0dXJuIHZhbHVlTmFtZTsKLSAgICByZXR1cm4gZ2V0UHJvcGVydHlOYW1lKHN0YXRpY19jYXN0 PENTU1Byb3BlcnR5SUQ+KHZhbHVlT3JQcm9wZXJ0eUlEKSk7CisgICAgQVNTRVJUX0FSRyh2YWx1 ZU9yUHJvcGVydHlJRCwgdmFsdWVPclByb3BlcnR5SUQgPj0gMCk7CisgICAgQVNTRVJUX0FSRyh2 YWx1ZU9yUHJvcGVydHlJRCwgdmFsdWVPclByb3BlcnR5SUQgPCBudW1DU1NWYWx1ZUtleXdvcmRz IHx8ICh2YWx1ZU9yUHJvcGVydHlJRCA+PSBmaXJzdENTU1Byb3BlcnR5ICYmIHZhbHVlT3JQcm9w ZXJ0eUlEIDwgZmlyc3RDU1NQcm9wZXJ0eSArIG51bUNTU1Byb3BlcnRpZXMpKTsKKworICAgIGlm ICh2YWx1ZU9yUHJvcGVydHlJRCA8IDApCisgICAgICAgIHJldHVybiBudWxsQXRvbTsKKworICAg IGlmICh2YWx1ZU9yUHJvcGVydHlJRCA8IG51bUNTU1ZhbHVlS2V5d29yZHMpIHsKKyAgICAgICAg c3RhdGljIEF0b21pY1N0cmluZyogY3NzVmFsdWVLZXl3b3JkU3RyaW5nc1tudW1DU1NWYWx1ZUtl eXdvcmRzXTsKKyAgICAgICAgaWYgKCFjc3NWYWx1ZUtleXdvcmRTdHJpbmdzW3ZhbHVlT3JQcm9w ZXJ0eUlEXSkKKyAgICAgICAgICAgIGNzc1ZhbHVlS2V5d29yZFN0cmluZ3NbdmFsdWVPclByb3Bl cnR5SURdID0gbmV3IEF0b21pY1N0cmluZyhnZXRWYWx1ZU5hbWUodmFsdWVPclByb3BlcnR5SUQp KTsKKyAgICAgICAgcmV0dXJuICpjc3NWYWx1ZUtleXdvcmRTdHJpbmdzW3ZhbHVlT3JQcm9wZXJ0 eUlEXTsKKyAgICB9CisKKyAgICBpZiAodmFsdWVPclByb3BlcnR5SUQgPj0gZmlyc3RDU1NQcm9w ZXJ0eSAmJiB2YWx1ZU9yUHJvcGVydHlJRCA8IGZpcnN0Q1NTUHJvcGVydHkgKyBudW1DU1NQcm9w ZXJ0aWVzKSB7CisgICAgICAgIHN0YXRpYyBBdG9taWNTdHJpbmcqIGNzc1Byb3BlcnR5U3RyaW5n c1tudW1DU1NQcm9wZXJ0aWVzXTsKKyAgICAgICAgaW50IHByb3BlcnR5SW5kZXggPSB2YWx1ZU9y UHJvcGVydHlJRCAtIGZpcnN0Q1NTUHJvcGVydHk7CisgICAgICAgIGlmICghY3NzUHJvcGVydHlT dHJpbmdzW3Byb3BlcnR5SW5kZXhdKQorICAgICAgICAgICAgY3NzUHJvcGVydHlTdHJpbmdzW3By b3BlcnR5SW5kZXhdID0gbmV3IEF0b21pY1N0cmluZyhnZXRQcm9wZXJ0eU5hbWUoc3RhdGljX2Nh c3Q8Q1NTUHJvcGVydHlJRD4odmFsdWVPclByb3BlcnR5SUQpKSk7CisgICAgICAgIHJldHVybiAq Y3NzUHJvcGVydHlTdHJpbmdzW3Byb3BlcnR5SW5kZXhdOworICAgIH0KKworICAgIHJldHVybiBu dWxsQXRvbTsKIH0KIAogLy8gImlkZW50IiBmcm9tIHRoZSBDU1MgdG9rZW5pemVyLCBtaW51cyBi YWNrc2xhc2gtZXNjYXBlIHNlcXVlbmNlcwpAQCAtOTMwLDcgKzk0OSw3IEBAIENTU1BhcnNlclZh bHVlIENTU1ByaW1pdGl2ZVZhbHVlOjpwYXJzZXIKICAgICAgICAgICAgIGJyZWFrOwogICAgICAg ICBjYXNlIENTU19JREVOVDogewogICAgICAgICAgICAgdmFsdWUuaWQgPSBtX3ZhbHVlLmlkZW50 OwotICAgICAgICAgICAgU3RyaW5nIG5hbWUgPSB2YWx1ZU9yUHJvcGVydHlOYW1lKG1fdmFsdWUu aWRlbnQpOworICAgICAgICAgICAgY29uc3QgQXRvbWljU3RyaW5nJiBuYW1lID0gdmFsdWVPclBy b3BlcnR5TmFtZShtX3ZhbHVlLmlkZW50KTsKICAgICAgICAgICAgIHZhbHVlLnN0cmluZy5jaGFy YWN0ZXJzID0gY29uc3RfY2FzdDxVQ2hhcio+KG5hbWUuY2hhcmFjdGVycygpKTsKICAgICAgICAg ICAgIHZhbHVlLnN0cmluZy5sZW5ndGggPSBuYW1lLmxlbmd0aCgpOwogICAgICAgICAgICAgYnJl YWs7Cg==