163766 2016-10-20 16:43:51 -0700 bmalloc api should crash on failure to allocate when !isBmallocEnabled. 2016-10-20 17:19:37 -0700 1 1 1 Unclassified WebKit bmalloc WebKit Local Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mark.lam mark.lam ggaren oldest_to_newest 1242653 0 mark.lam 2016-10-20 16:43:51 -0700 Patch coming. 1242667 1 292285 mark.lam 2016-10-20 16:55:03 -0700 Created attachment 292285 proposed patch. 1242670 2 292285 keith_miller 2016-10-20 17:00:23 -0700 Comment on attachment 292285 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=292285&action=review r=me with comments > Source/bmalloc/bmalloc/Allocator.cpp:107 > + auto result = realloc(object, newSize); nit: auto seems a little unnecessary here > Source/bmalloc/bmalloc/Allocator.cpp:197 > + auto result = malloc(size); nit: ditto 1242671 3 292285 fpizlo 2016-10-20 17:00:41 -0700 Comment on attachment 292285 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=292285&action=review R=me. I'm assuming that there's a reason why we need to crash. > Source/bmalloc/ChangeLog:3 > + bmalloc api should crash on failure to allocate when GuardMalloc is used. I recommend changing the title or adding a clarification that the crash-on-failure is for the !isBmallocEnabled case, which is broader than the GuardMalloc case. 1242678 4 mark.lam 2016-10-20 17:19:37 -0700 Thanks for the review. I've changed the autos to void *s. I've also changed the title, and added a comment in the ChangeLog to explain why crashing in bmalloc is what we want when we fail to allocate. Landed in r207646: <http://trac.webkit.org/r207646>. 292285 2016-10-20 16:55:03 -0700 2016-10-20 17:00:23 -0700 proposed patch. bug-163766.patch text/plain 2165 mark.lam SW5kZXg6IFNvdXJjZS9ibWFsbG9jL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvYm1h bGxvYy9DaGFuZ2VMb2cJKHJldmlzaW9uIDIwNzY0MykKKysrIFNvdXJjZS9ibWFsbG9jL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDE2LTEwLTIwICBNYXJrIExh bSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBibWFsbG9jIGFwaSBzaG91bGQgY3Jh c2ggb24gZmFpbHVyZSB0byBhbGxvY2F0ZSB3aGVuIEd1YXJkTWFsbG9jIGlzIHVzZWQuCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNjM3NjYKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGJtYWxsb2MvQWxs b2NhdG9yLmNwcDoKKyAgICAgICAgKGJtYWxsb2M6OkFsbG9jYXRvcjo6YWxsb2NhdGVJbXBsKToK KyAgICAgICAgKGJtYWxsb2M6OkFsbG9jYXRvcjo6cmVhbGxvY2F0ZSk6CisgICAgICAgIChibWFs bG9jOjpBbGxvY2F0b3I6OmFsbG9jYXRlU2xvd0Nhc2UpOgorCiAyMDE2LTA5LTI2ICBZb3NoaWFr aSBKaXRzdWthd2EgIDxZb3NoaWFraS5KaXRzdWthd2FAc29ueS5jb20+CiAKICAgICAgICAgQXZv aWQgaW1wbGljaXQgY29udmVyc2lvbiBmcm9tIGl0ZXJhdG9yIHRvIHBvaW50ZXIKSW5kZXg6IFNv dXJjZS9ibWFsbG9jL2JtYWxsb2MvQWxsb2NhdG9yLmNwcAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv Ym1hbGxvYy9ibWFsbG9jL0FsbG9jYXRvci5jcHAJKHJldmlzaW9uIDIwNzYzOSkKKysrIFNvdXJj ZS9ibWFsbG9jL2JtYWxsb2MvQWxsb2NhdG9yLmNwcAkod29ya2luZyBjb3B5KQpAQCAtODAsOCAr ODAsMTEgQEAgdm9pZCogQWxsb2NhdG9yOjphbGxvY2F0ZUltcGwoc2l6ZV90IGFsaQogCiAgICAg aWYgKCFtX2lzQm1hbGxvY0VuYWJsZWQpIHsKICAgICAgICAgdm9pZCogcmVzdWx0ID0gbnVsbHB0 cjsKLSAgICAgICAgaWYgKHBvc2l4X21lbWFsaWduKCZyZXN1bHQsIGFsaWdubWVudCwgc2l6ZSkp CisgICAgICAgIGlmIChwb3NpeF9tZW1hbGlnbigmcmVzdWx0LCBhbGlnbm1lbnQsIHNpemUpKSB7 CisgICAgICAgICAgICBpZiAoY3Jhc2hPbkZhaWx1cmUpCisgICAgICAgICAgICAgICAgQkNSQVNI KCk7CiAgICAgICAgICAgICByZXR1cm4gbnVsbHB0cjsKKyAgICAgICAgfQogICAgICAgICByZXR1 cm4gcmVzdWx0OwogICAgIH0KIApAQCAtMTAwLDggKzEwMywxMiBAQCB2b2lkKiBBbGxvY2F0b3I6 OmFsbG9jYXRlSW1wbChzaXplX3QgYWxpCiAKIHZvaWQqIEFsbG9jYXRvcjo6cmVhbGxvY2F0ZSh2 b2lkKiBvYmplY3QsIHNpemVfdCBuZXdTaXplKQogewotICAgIGlmICghbV9pc0JtYWxsb2NFbmFi bGVkKQotICAgICAgICByZXR1cm4gcmVhbGxvYyhvYmplY3QsIG5ld1NpemUpOworICAgIGlmICgh bV9pc0JtYWxsb2NFbmFibGVkKSB7CisgICAgICAgIGF1dG8gcmVzdWx0ID0gcmVhbGxvYyhvYmpl Y3QsIG5ld1NpemUpOworICAgICAgICBpZiAoIXJlc3VsdCkKKyAgICAgICAgICAgIEJDUkFTSCgp OworICAgICAgICByZXR1cm4gcmVzdWx0OworICAgIH0KIAogICAgIHNpemVfdCBvbGRTaXplID0g MDsKICAgICBzd2l0Y2ggKG9iamVjdFR5cGUob2JqZWN0KSkgewpAQCAtMTg2LDggKzE5MywxMiBA QCBOT19JTkxJTkUgdm9pZCogQWxsb2NhdG9yOjphbGxvY2F0ZUxvZ1NpCiAKIHZvaWQqIEFsbG9j YXRvcjo6YWxsb2NhdGVTbG93Q2FzZShzaXplX3Qgc2l6ZSkKIHsKLSAgICBpZiAoIW1faXNCbWFs bG9jRW5hYmxlZCkKLSAgICAgICAgcmV0dXJuIG1hbGxvYyhzaXplKTsKKyAgICBpZiAoIW1faXNC bWFsbG9jRW5hYmxlZCkgeworICAgICAgICBhdXRvIHJlc3VsdCA9IG1hbGxvYyhzaXplKTsKKyAg ICAgICAgaWYgKCFyZXN1bHQpCisgICAgICAgICAgICBCQ1JBU0goKTsKKyAgICAgICAgcmV0dXJu IHJlc3VsdDsKKyAgICB9CiAKICAgICBpZiAoc2l6ZSA8PSBtYXNrU2l6ZUNsYXNzTWF4KSB7CiAg ICAgICAgIHNpemVfdCBzaXplQ2xhc3MgPSBibWFsbG9jOjptYXNrU2l6ZUNsYXNzKHNpemUpOwo=