202152 2019-09-24 11:57:47 -0700 Address static analysis warning in Allocator.cpp: Null pointer argument in call to memory copy function 2019-09-24 17:14:08 -0700 1 1 1 Unclassified WebKit bmalloc WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 krollin krollin commit-queue fpizlo ggaren mark.lam webkit-bug-importer oldest_to_newest 1573727 0 krollin 2019-09-24 11:57:47 -0700 Xcode's static analysis facility flags the following: .../OpenSource/Source/bmalloc/bmalloc/Allocator.cpp:98:5: warning: Null pointer argument in call to memory copy function memcpy(result, object, copySize); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ https://en.cppreference.com/w/cpp/string/byte/memcpy explains that this is undefined behavior: "If either dest or src is a null pointer, the behavior is undefined, even if count is zero." I suppose that passing in a null source pointer could be bad if the implementation fetched the first source byte to copy before actually checking the number of bytes to copy. So far, it doesn’t seem to be an issue, but we should clean this up. Simply adding "if (result && object)" before the memcpy will add tests and branches in the hot path of this function and so might not be the best solution. Instead, add a little bit of duplicate code earlier in the function at a place where we already know that "object" is NULL. 1573728 1 webkit-bug-importer 2019-09-24 11:58:02 -0700 <rdar://problem/55671444> 1573729 2 379463 krollin 2019-09-24 12:00:56 -0700 Created attachment 379463 Patch 1573784 3 379463 ggaren 2019-09-24 13:20:48 -0700 Comment on attachment 379463 Patch I kinda think "if (!object) return allocateImpl(newSize, action);" at the top of the function would read better. Then you can remove the ASSERT that nullptr is a small object too. This function is starting too feel a little too branch-y and early return-y to follow. And there shouldn't be any cost to an up-front null check. 1573833 4 379492 krollin 2019-09-24 14:30:45 -0700 Created attachment 379492 Patch 1573834 5 krollin 2019-09-24 14:31:40 -0700 Updated as Geoff describes. 1573879 6 379492 ggaren 2019-09-24 16:12:30 -0700 Comment on attachment 379492 Patch r=me 1573908 7 379492 commit-queue 2019-09-24 17:14:06 -0700 Comment on attachment 379492 Patch Clearing flags on attachment: 379492 Committed r250325: <https://trac.webkit.org/changeset/250325> 1573909 8 commit-queue 2019-09-24 17:14:08 -0700 All reviewed patches have been landed. Closing bug. 379463 2019-09-24 12:00:56 -0700 2019-09-24 14:30:44 -0700 Patch bug-202152-20190924120055.patch text/plain 2798 krollin U3VidmVyc2lvbiBSZXZpc2lvbjogMjUwMjQzCmRpZmYgLS1naXQgYS9Tb3VyY2UvYm1hbGxvYy9D aGFuZ2VMb2cgYi9Tb3VyY2UvYm1hbGxvYy9DaGFuZ2VMb2cKaW5kZXggZjBiYjNhZjhkOWUyNDc0 Mzc5ZmZiMGI4ZjYzNTVmZmIxYTZlMTc0NS4uMDdmYzQwYTkwMTQ4ZTA1YjRlMDliOWM2YzM0YWE4 NTQ3NTcxNzFjYiAxMDA2NDQKLS0tIGEvU291cmNlL2JtYWxsb2MvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9ibWFsbG9jL0NoYW5nZUxvZwpAQCAtMSwzICsxLDM2IEBACisyMDE5LTA5LTI0ICBLZWl0 aCBSb2xsaW4gIDxrcm9sbGluQGFwcGxlLmNvbT4KKworICAgICAgICBBZGRyZXNzIHN0YXRpYyBh bmFseXNpcyB3YXJuaW5nIGluIEFsbG9jYXRvci5jcHA6IE51bGwgcG9pbnRlciBhcmd1bWVudCBp biBjYWxsIHRvIG1lbW9yeSBjb3B5IGZ1bmN0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDIxNTIKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzU1 NjcxNDQ0PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFhjb2RlJ3Mgc3RhdGljIGFuYWx5c2lzIGZhY2lsaXR5IGZsYWdzIHRoZSBmb2xsb3dpbmc6CisK KyAgICAgICAgICAgIC4uLi9PcGVuU291cmNlL1NvdXJjZS9ibWFsbG9jL2JtYWxsb2MvQWxsb2Nh dG9yLmNwcDo5ODo1OiB3YXJuaW5nOiBOdWxsIHBvaW50ZXIgYXJndW1lbnQgaW4gY2FsbCB0byBt ZW1vcnkgY29weSBmdW5jdGlvbgorICAgICAgICAgICAgICAgIG1lbWNweShyZXN1bHQsIG9iamVj dCwgY29weVNpemUpOworICAgICAgICAgICAgICAgIF5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+ fn5+fn5+CisKKyAgICAgICAgaHR0cHM6Ly9lbi5jcHByZWZlcmVuY2UuY29tL3cvY3BwL3N0cmlu Zy9ieXRlL21lbWNweSBleHBsYWlucyB0aGF0CisgICAgICAgIHRoaXMgaXMgdW5kZWZpbmVkIGJl aGF2aW9yOgorCisgICAgICAgICAgICAiSWYgZWl0aGVyIGRlc3Qgb3Igc3JjIGlzIGEgbnVsbCBw b2ludGVyLCB0aGUgYmVoYXZpb3IgaXMKKyAgICAgICAgICAgICB1bmRlZmluZWQsIGV2ZW4gaWYg Y291bnQgaXMgemVyby4iCisKKyAgICAgICAgSSBzdXBwb3NlIHRoYXQgcGFzc2luZyBpbiBhIG51 bGwgc291cmNlIHBvaW50ZXIgY291bGQgYmUgYmFkIGlmIHRoZQorICAgICAgICBpbXBsZW1lbnRh dGlvbiBmZXRjaGVkIHRoZSBmaXJzdCBzb3VyY2UgYnl0ZSB0byBjb3B5IGJlZm9yZSBhY3R1YWxs eQorICAgICAgICBjaGVja2luZyB0aGUgbnVtYmVyIG9mIGJ5dGVzIHRvIGNvcHkuIFNvIGZhciwg aXQgZG9lc27igJl0IHNlZW0gdG8gYmUgYW4KKyAgICAgICAgaXNzdWUsIGJ1dCB3ZSBzaG91bGQg Y2xlYW4gdGhpcyB1cC4KKworICAgICAgICBTaW1wbHkgYWRkaW5nICJpZiAocmVzdWx0ICYmIG9i amVjdCkiIGJlZm9yZSB0aGUgbWVtY3B5IHdpbGwgYWRkIHRlc3RzCisgICAgICAgIGFuZCBicmFu Y2hlcyBpbiB0aGUgaG90IHBhdGggb2YgdGhpcyBmdW5jdGlvbiBhbmQgc28gbWlnaHQgbm90IGJl IHRoZQorICAgICAgICBiZXN0IHNvbHV0aW9uLiBJbnN0ZWFkLCBhZGQgYSBsaXR0bGUgYml0IG9m IGR1cGxpY2F0ZSBjb2RlIGVhcmxpZXIgaW4KKyAgICAgICAgdGhlIGZ1bmN0aW9uIGF0IGEgcGxh Y2Ugd2hlcmUgd2UgYWxyZWFkeSBrbm93IHRoYXQgIm9iamVjdCIgaXMgTlVMTC4KKworICAgICAg ICAqIGJtYWxsb2MvQWxsb2NhdG9yLmNwcDoKKyAgICAgICAgKGJtYWxsb2M6OkFsbG9jYXRvcjo6 cmVhbGxvY2F0ZUltcGwpOgorCiAyMDE5LTA5LTE3ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxl LmNvbT4KIAogICAgICAgICBVc2UgY29uc3RleHByIGluc3RlYWQgb2YgY29uc3QgaW4gc3ltYm9s IGRlZmluaXRpb25zIHRoYXQgYXJlIG9idmlvdXNseSBjb25zdGV4cHIuCmRpZmYgLS1naXQgYS9T b3VyY2UvYm1hbGxvYy9ibWFsbG9jL0FsbG9jYXRvci5jcHAgYi9Tb3VyY2UvYm1hbGxvYy9ibWFs bG9jL0FsbG9jYXRvci5jcHAKaW5kZXggZjQxMjA4M2E3ZWEzZTM4ZmRmMTNjYzQ2ZmIxMjJkODdj ZWExMzdmNS4uMWJmN2RiYTk1ZTRkYWYyMWZjYzVlNzM1YTUzNDI4YmQ1OTMwOGQyNiAxMDA2NDQK LS0tIGEvU291cmNlL2JtYWxsb2MvYm1hbGxvYy9BbGxvY2F0b3IuY3BwCisrKyBiL1NvdXJjZS9i bWFsbG9jL2JtYWxsb2MvQWxsb2NhdG9yLmNwcApAQCAtNjksOCArNjksMTIgQEAgdm9pZCogQWxs b2NhdG9yOjpyZWFsbG9jYXRlSW1wbCh2b2lkKiBvYmplY3QsIHNpemVfdCBuZXdTaXplLCBGYWls dXJlQWN0aW9uIGFjdGkKICAgICBzd2l0Y2ggKG9iamVjdFR5cGUobV9oZWFwLCBvYmplY3QpKSB7 CiAgICAgY2FzZSBPYmplY3RUeXBlOjpTbWFsbDogewogICAgICAgICBCQVNTRVJUKG9iamVjdFR5 cGUobV9oZWFwLCBudWxscHRyKSA9PSBPYmplY3RUeXBlOjpTbWFsbCk7Ci0gICAgICAgIGlmICgh b2JqZWN0KQotICAgICAgICAgICAgYnJlYWs7CisgICAgICAgIGlmICghb2JqZWN0KSB7CisgICAg ICAgICAgICB2b2lkKiByZXN1bHQgPSBhbGxvY2F0ZUltcGwobmV3U2l6ZSwgYWN0aW9uKTsKKyAg ICAgICAgICAgIGlmICghcmVzdWx0KQorICAgICAgICAgICAgICAgIEJBU1NFUlQoYWN0aW9uID09 IEZhaWx1cmVBY3Rpb246OlJldHVybk51bGwpOworICAgICAgICAgICAgcmV0dXJuIHJlc3VsdDsK KyAgICAgICAgfQogCiAgICAgICAgIHNpemVfdCBzaXplQ2xhc3MgPSBPYmplY3Qob2JqZWN0KS5w YWdlKCktPnNpemVDbGFzcygpOwogICAgICAgICBvbGRTaXplID0gb2JqZWN0U2l6ZShzaXplQ2xh c3MpOwo= 379492 2019-09-24 14:30:45 -0700 2019-09-24 17:14:06 -0700 Patch bug-202152-20190924143044.patch text/plain 2821 krollin U3VidmVyc2lvbiBSZXZpc2lvbjogMjUwMjQzCmRpZmYgLS1naXQgYS9Tb3VyY2UvYm1hbGxvYy9D aGFuZ2VMb2cgYi9Tb3VyY2UvYm1hbGxvYy9DaGFuZ2VMb2cKaW5kZXggZjBiYjNhZjhkOWUyNDc0 Mzc5ZmZiMGI4ZjYzNTVmZmIxYTZlMTc0NS4uMjBhMTAyODVlNmQzMjY1YzEwZGI3OTYzNDUyNWZk MzZmYzA0M2FlYSAxMDA2NDQKLS0tIGEvU291cmNlL2JtYWxsb2MvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9ibWFsbG9jL0NoYW5nZUxvZwpAQCAtMSwzICsxLDM3IEBACisyMDE5LTA5LTI0ICBLZWl0 aCBSb2xsaW4gIDxrcm9sbGluQGFwcGxlLmNvbT4KKworICAgICAgICBBZGRyZXNzIHN0YXRpYyBh bmFseXNpcyB3YXJuaW5nIGluIEFsbG9jYXRvci5jcHA6IE51bGwgcG9pbnRlciBhcmd1bWVudCBp biBjYWxsIHRvIG1lbW9yeSBjb3B5IGZ1bmN0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDIxNTIKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzU1 NjcxNDQ0PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFhjb2RlJ3Mgc3RhdGljIGFuYWx5c2lzIGZhY2lsaXR5IGZsYWdzIHRoZSBmb2xsb3dpbmc6CisK KyAgICAgICAgICAgIC4uLi9PcGVuU291cmNlL1NvdXJjZS9ibWFsbG9jL2JtYWxsb2MvQWxsb2Nh dG9yLmNwcDo5ODo1OiB3YXJuaW5nOiBOdWxsIHBvaW50ZXIgYXJndW1lbnQgaW4gY2FsbCB0byBt ZW1vcnkgY29weSBmdW5jdGlvbgorICAgICAgICAgICAgICAgIG1lbWNweShyZXN1bHQsIG9iamVj dCwgY29weVNpemUpOworICAgICAgICAgICAgICAgIF5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+ fn5+fn5+CisKKyAgICAgICAgaHR0cHM6Ly9lbi5jcHByZWZlcmVuY2UuY29tL3cvY3BwL3N0cmlu Zy9ieXRlL21lbWNweSBleHBsYWlucyB0aGF0CisgICAgICAgIHRoaXMgaXMgdW5kZWZpbmVkIGJl aGF2aW9yOgorCisgICAgICAgICAgICAiSWYgZWl0aGVyIGRlc3Qgb3Igc3JjIGlzIGEgbnVsbCBw b2ludGVyLCB0aGUgYmVoYXZpb3IgaXMKKyAgICAgICAgICAgICB1bmRlZmluZWQsIGV2ZW4gaWYg Y291bnQgaXMgemVyby4iCisKKyAgICAgICAgSSBzdXBwb3NlIHRoYXQgcGFzc2luZyBpbiBhIG51 bGwgc291cmNlIHBvaW50ZXIgY291bGQgYmUgYmFkIGlmIHRoZQorICAgICAgICBpbXBsZW1lbnRh dGlvbiBmZXRjaGVkIHRoZSBmaXJzdCBzb3VyY2UgYnl0ZSB0byBjb3B5IGJlZm9yZSBhY3R1YWxs eQorICAgICAgICBjaGVja2luZyB0aGUgbnVtYmVyIG9mIGJ5dGVzIHRvIGNvcHkuIFNvIGZhciwg aXQgZG9lc27igJl0IHNlZW0gdG8gYmUgYW4KKyAgICAgICAgaXNzdWUsIGJ1dCB3ZSBzaG91bGQg Y2xlYW4gdGhpcyB1cC4KKworICAgICAgICBTaW1wbHkgYWRkaW5nICJpZiAocmVzdWx0ICYmIG9i amVjdCkiIGJlZm9yZSB0aGUgbWVtY3B5IHdpbGwgYWRkIHRlc3RzCisgICAgICAgIGFuZCBicmFu Y2hlcyBpbiB0aGUgaG90IHBhdGggb2YgdGhpcyBmdW5jdGlvbiBhbmQgc28gbWlnaHQgbm90IGJl IHRoZQorICAgICAgICBiZXN0IHNvbHV0aW9uLiBJbnN0ZWFkLCBzdHJhaWdodGVuIG91dCB0aGUg Y29kZSBhIGxpdHRsZSBiaXQgYnkKKyAgICAgICAgcHV0dGluZyBhbiBlYXJseSBjaGVjayBhbmQg cmV0dXJuIG9uICJvYmplY3QiLiBUaGlzIGFsc28gYWxsb3dzIHVzIHRvCisgICAgICAgIHJlbW92 ZSBzb21lIGludGVybWVkaWF0ZSBjb2RlLgorCisgICAgICAgICogYm1hbGxvYy9BbGxvY2F0b3Iu Y3BwOgorICAgICAgICAoYm1hbGxvYzo6QWxsb2NhdG9yOjpyZWFsbG9jYXRlSW1wbCk6CisKIDIw MTktMDktMTcgIE1hcmsgTGFtICA8bWFyay5sYW1AYXBwbGUuY29tPgogCiAgICAgICAgIFVzZSBj b25zdGV4cHIgaW5zdGVhZCBvZiBjb25zdCBpbiBzeW1ib2wgZGVmaW5pdGlvbnMgdGhhdCBhcmUg b2J2aW91c2x5IGNvbnN0ZXhwci4KZGlmZiAtLWdpdCBhL1NvdXJjZS9ibWFsbG9jL2JtYWxsb2Mv QWxsb2NhdG9yLmNwcCBiL1NvdXJjZS9ibWFsbG9jL2JtYWxsb2MvQWxsb2NhdG9yLmNwcAppbmRl eCBmNDEyMDgzYTdlYTNlMzhmZGYxM2NjNDZmYjEyMmQ4N2NlYTEzN2Y1Li4wY2RkZDY5ZWVlOGUw NjI3N2U1MmM4YTNiMmI5ZDg1OGZkNjllMGVmIDEwMDY0NAotLS0gYS9Tb3VyY2UvYm1hbGxvYy9i bWFsbG9jL0FsbG9jYXRvci5jcHAKKysrIGIvU291cmNlL2JtYWxsb2MvYm1hbGxvYy9BbGxvY2F0 b3IuY3BwCkBAIC02NSwxMyArNjUsMTIgQEAgdm9pZCogQWxsb2NhdG9yOjphbGxvY2F0ZUltcGwo c2l6ZV90IGFsaWdubWVudCwgc2l6ZV90IHNpemUsIEZhaWx1cmVBY3Rpb24gYWN0aW8KIAogdm9p ZCogQWxsb2NhdG9yOjpyZWFsbG9jYXRlSW1wbCh2b2lkKiBvYmplY3QsIHNpemVfdCBuZXdTaXpl LCBGYWlsdXJlQWN0aW9uIGFjdGlvbikKIHsKKyAgICBpZiAoIW9iamVjdCkKKyAgICAgICAgcmV0 dXJuIGFsbG9jYXRlSW1wbChuZXdTaXplLCBhY3Rpb24pOworCiAgICAgc2l6ZV90IG9sZFNpemUg PSAwOwogICAgIHN3aXRjaCAob2JqZWN0VHlwZShtX2hlYXAsIG9iamVjdCkpIHsKICAgICBjYXNl IE9iamVjdFR5cGU6OlNtYWxsOiB7Ci0gICAgICAgIEJBU1NFUlQob2JqZWN0VHlwZShtX2hlYXAs IG51bGxwdHIpID09IE9iamVjdFR5cGU6OlNtYWxsKTsKLSAgICAgICAgaWYgKCFvYmplY3QpCi0g ICAgICAgICAgICBicmVhazsKLQogICAgICAgICBzaXplX3Qgc2l6ZUNsYXNzID0gT2JqZWN0KG9i amVjdCkucGFnZSgpLT5zaXplQ2xhc3MoKTsKICAgICAgICAgb2xkU2l6ZSA9IG9iamVjdFNpemUo c2l6ZUNsYXNzKTsKICAgICAgICAgYnJlYWs7Cg==