111219 2013-03-01 14:08:24 -0800 REGRESSION (r125809): CFStrings created via StringImpl::createCFString() might reference freed memory when Objective-C garbage collection is enabled 2013-03-01 14:41:40 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 aestes aestes benjamin oldest_to_newest 845488 0 aestes 2013-03-01 14:08:24 -0800 CFStrings created via StringImpl::createCFString() might reference freed memory when Objective-C garbage collection is enabled 845496 1 aestes 2013-03-01 14:15:19 -0800 <rdar://problem/12265868> 845497 2 191035 aestes 2013-03-01 14:18:00 -0800 Created attachment 191035 Patch 845503 3 191035 benjamin 2013-03-01 14:24:39 -0800 Comment on attachment 191035 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191035&action=review sorry about that! > Source/WebCore/ChangeLog:17 > + However, custom allocators aren't supported when Objective-C garbage > + collection is enabled, so in this case we use the default CF allocator. > + Since we can't guarantee the lifetime of the StringImpl in this case, > + we should just fall back to copying the string. You can add rXXXX stupidly broke this by not checking if StringWrapperCFAllocator::allocator returns something. > Source/WebCore/platform/text/cf/StringImplCF.cpp:38 > +#if PLATFORM(MAC) && !PLATFORM(IOS) > Source/WebCore/platform/text/cf/StringImplCF.cpp:-123 > -#if PLATFORM(MAC) > // Since garbage collection isn't compatible with custom allocators, don't use this at all when garbage collection is active. > - if (objc_collectingEnabled()) > + if (garbageCollectionEnabled()) > return 0; > -#endif You should get rid of this and just ASSERT(!garbageCollectionEnabled) The comment: // Since garbage collection isn't compatible with custom allocators, don't use this at all when garbage collection is active. should be put before garbageCollectionEnabled() in StringImpl::createCFString() IMHO 845504 4 benjamin 2013-03-01 14:24:59 -0800 + remove the static as asked on IRC :) 845505 5 benjamin 2013-03-01 14:26:51 -0800 oh, + "inline" for static bool garbageCollectionEnabled() since the static can go away :) 845517 6 aestes 2013-03-01 14:41:40 -0800 Committed r144507: <http://trac.webkit.org/changeset/144507> 191035 2013-03-01 14:18:00 -0800 2013-03-01 14:24:39 -0800 Patch bug-111219-20130301141417.patch text/plain 3473 aestes U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ0NDAwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTNlN2I3NWIyYWFlMTVk NDIxMzZkMDQ3MGJmNzhhOGE0NjliZGUzOC4uOGNkNzBjNGYwNTNmMDg5MjhjMWFhZjcxMDVlYjc3 Y2M2NDcyNzJhZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI4IEBACisyMDEzLTAzLTAxICBBbmR5 IEVzdGVzICA8YWVzdGVzQGFwcGxlLmNvbT4KKworICAgICAgICBDRlN0cmluZ3MgY3JlYXRlZCB2 aWEgU3RyaW5nSW1wbDo6Y3JlYXRlQ0ZTdHJpbmcoKSAgbWlnaHQgcmVmZXJlbmNlIGZyZWVkIG1l bW9yeSB3aGVuIE9iamVjdGl2ZS1DIGdhcmJhZ2UgY29sbGVjdGlvbiBpcyBlbmFibGVkCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMTEyMTkKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBTdHJpbmdJbXBsOjpj cmVhdGVDRlN0cmluZygpIHVzZXMgQ0ZTdHJpbmdDcmVhdGVXaXRoQnl0ZXNOb0NvcHkoKSBpbgor ICAgICAgICBvcmRlciB0byBjcmVhdGUgQ0ZTdHJpbmcgd2l0aG91dCBtYWtpbmcgYW4gdW5uZWNl c3NhcnkgY29weS4gSW4gb3JkZXIKKyAgICAgICAgdG8gZW5zdXJlIHRoYXQgdGhlIHRoZSBTdHJp bmdJbXBsJ3MgYmFja2luZyBidWZmZXIgaXNuJ3QgZGVhbGxvY2F0ZWQKKyAgICAgICAgd2hpbGUg dGhlIENGU3RyaW5nIGlzIHN0aWxsIGFsaXZlLCB3ZSB1c2UgYSBjdXN0b20gQ0ZBbGxvY2F0b3Ig dG8KKyAgICAgICAgcmVmL2RlcmVmIHRoZSBTdHJpbmdJbXBsIGF0IHRoZSBhcHByb3ByaWF0ZSB0 aW1lcy4KKworICAgICAgICBIb3dldmVyLCBjdXN0b20gYWxsb2NhdG9ycyBhcmVuJ3Qgc3VwcG9y dGVkIHdoZW4gT2JqZWN0aXZlLUMgZ2FyYmFnZQorICAgICAgICBjb2xsZWN0aW9uIGlzIGVuYWJs ZWQsIHNvIGluIHRoaXMgY2FzZSB3ZSB1c2UgdGhlIGRlZmF1bHQgQ0YgYWxsb2NhdG9yLgorICAg ICAgICBTaW5jZSB3ZSBjYW4ndCBndWFyYW50ZWUgdGhlIGxpZmV0aW1lIG9mIHRoZSBTdHJpbmdJ bXBsIGluIHRoaXMgY2FzZSwKKyAgICAgICAgd2Ugc2hvdWxkIGp1c3QgZmFsbCBiYWNrIHRvIGNv cHlpbmcgdGhlIHN0cmluZy4KKworICAgICAgICAqIHBsYXRmb3JtL3RleHQvY2YvU3RyaW5nSW1w bENGLmNwcDoKKyAgICAgICAgKGdhcmJhZ2VDb2xsZWN0aW9uRW5hYmxlZCk6IE1vdmVkIHRoZSBj aGVjayBmb3Igd2hldGhlciBnYXJiYWdlCisgICAgICAgIGNvbGxlY3Rpb24gaXMgZW5hYmxlZCBm cm9tIFN0cmluZ1dyYXBwZXJDRkFsbG9jYXRvcjo6Y3JlYXRlKCkgdG8gaGVyZS4KKyAgICAgICAg KFdURjo6U3RyaW5nV3JhcHBlckNGQWxsb2NhdG9yOjpjcmVhdGUpOiBDYWxsIGdhcmJhZ2VDb2xs ZWN0aW9uRW5hYmxlZCgpLgorICAgICAgICAoV1RGOjpTdHJpbmdJbXBsOjpjcmVhdGVDRlN0cmlu Zyk6IElmIGdhcmJhZ2UgY29sbGVjdGlvbiBpcyBlbmFibGVkLAorICAgICAgICBjYWxsIHRoZSB2 YXJpYW50cyBvZiBDRlN0cmluZ0NyZWF0ZSB0aGF0IGNvcHkgdGhlIHN0cmluZy4KKwogMjAxMy0w Mi0yOCAgTGV2aSBXZWludHJhdWIgIDxsZXZpd0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgU3Rh bGUgRnJhbWVTZWxlY3Rpb24gaW4gcmVtb3ZlZCBpZnJhbWUgY2F1c2VzIGNyYXNoCmRpZmYgLS1n aXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS90ZXh0L2NmL1N0cmluZ0ltcGxDRi5jcHAgYi9T b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS90ZXh0L2NmL1N0cmluZ0ltcGxDRi5jcHAKaW5kZXggMDA4 NDkzYmY2MTIyNzk3OGUwNWI3NzNhM2Q0NTZhMjk1ZjkwOTY4OS4uMjYwYWI3M2RkYzdmNTNiMzgx OGMzMGIxZjUyZTQyM2E4NDQ4ODhmOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vdGV4dC9jZi9TdHJpbmdJbXBsQ0YuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L3RleHQvY2YvU3RyaW5nSW1wbENGLmNwcApAQCAtMzMsNiArMzMsMTYgQEAKICNpbmNsdWRlIDxv YmpjL29iamMtYXV0by5oPgogI2VuZGlmCiAKK3N0YXRpYyBib29sIGdhcmJhZ2VDb2xsZWN0aW9u RW5hYmxlZCgpCit7CisjaWYgUExBVEZPUk0oTUFDKQorICAgIHN0YXRpYyBib29sIGdhcmJhZ2VD b2xsZWN0aW9uRW5hYmxlZCA9IG9iamNfY29sbGVjdGluZ0VuYWJsZWQoKTsKKyAgICByZXR1cm4g Z2FyYmFnZUNvbGxlY3Rpb25FbmFibGVkOworI2Vsc2UKKyAgICByZXR1cm4gZmFsc2U7CisjZW5k aWYKK30KKwogbmFtZXNwYWNlIFdURiB7CiAKIG5hbWVzcGFjZSBTdHJpbmdXcmFwcGVyQ0ZBbGxv Y2F0b3IgewpAQCAtMTE2LDExICsxMjYsOSBAQCBuYW1lc3BhY2UgU3RyaW5nV3JhcHBlckNGQWxs b2NhdG9yIHsKIAogICAgIHN0YXRpYyBDRkFsbG9jYXRvclJlZiBjcmVhdGUoKQogICAgIHsKLSNp ZiBQTEFURk9STShNQUMpCiAgICAgICAgIC8vIFNpbmNlIGdhcmJhZ2UgY29sbGVjdGlvbiBpc24n dCBjb21wYXRpYmxlIHdpdGggY3VzdG9tIGFsbG9jYXRvcnMsIGRvbid0IHVzZSB0aGlzIGF0IGFs bCB3aGVuIGdhcmJhZ2UgY29sbGVjdGlvbiBpcyBhY3RpdmUuCi0gICAgICAgIGlmIChvYmpjX2Nv bGxlY3RpbmdFbmFibGVkKCkpCisgICAgICAgIGlmIChnYXJiYWdlQ29sbGVjdGlvbkVuYWJsZWQo KSkKICAgICAgICAgICAgIHJldHVybiAwOwotI2VuZGlmCiAgICAgICAgIENGQWxsb2NhdG9yQ29u dGV4dCBjb250ZXh0ID0geyAwLCAwLCByZXRhaW4sIHJlbGVhc2UsIGNvcHlEZXNjcmlwdGlvbiwg YWxsb2NhdGUsIHJlYWxsb2NhdGUsIGRlYWxsb2NhdGUsIHByZWZlcnJlZFNpemUgfTsKICAgICAg ICAgcmV0dXJuIENGQWxsb2NhdG9yQ3JlYXRlKDAsICZjb250ZXh0KTsKICAgICB9CkBAIC0xMzUs NyArMTQzLDcgQEAgbmFtZXNwYWNlIFN0cmluZ1dyYXBwZXJDRkFsbG9jYXRvciB7CiAKIFJldGFp blB0cjxDRlN0cmluZ1JlZj4gU3RyaW5nSW1wbDo6Y3JlYXRlQ0ZTdHJpbmcoKQogewotICAgIGlm ICghbV9sZW5ndGggfHwgIWlzTWFpblRocmVhZCgpKSB7CisgICAgaWYgKCFtX2xlbmd0aCB8fCAh aXNNYWluVGhyZWFkKCkgfHwgZ2FyYmFnZUNvbGxlY3Rpb25FbmFibGVkKCkpIHsKICAgICAgICAg aWYgKGlzOEJpdCgpKQogICAgICAgICAgICAgcmV0dXJuIGFkb3B0Q0YoQ0ZTdHJpbmdDcmVhdGVX aXRoQnl0ZXMoMCwgcmVpbnRlcnByZXRfY2FzdDxjb25zdCBVSW50OCo+KGNoYXJhY3RlcnM4KCkp LCBtX2xlbmd0aCwga0NGU3RyaW5nRW5jb2RpbmdJU09MYXRpbjEsIGZhbHNlKSk7CiAgICAgICAg IHJldHVybiBhZG9wdENGKENGU3RyaW5nQ3JlYXRlV2l0aENoYXJhY3RlcnMoMCwgcmVpbnRlcnBy ZXRfY2FzdDxjb25zdCBVbmlDaGFyKj4oY2hhcmFjdGVyczE2KCkpLCBtX2xlbmd0aCkpOwo=