143532 2015-04-08 12:48:06 -0700 DFG::IntegerCheckCombiningPhase's wrap-around check shouldn't trigger C++ undef behavior on wrap-around 2015-04-10 09:36:33 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 fpizlo fpizlo barraclough benjamin ggaren mark.lam mhahnenb mmirman msaboff nrotem oliver saam sam oldest_to_newest 1083702 0 fpizlo 2015-04-08 12:48:06 -0700 Oh the irony! We were protecting an optimization that only worked if there was no wrap-around in JavaScript. But the C++ code had wrap-around, which is undef in C++. So, if the compiler was smart enough, our compiler would think that there never was wrap-around. 1083705 1 250376 fpizlo 2015-04-08 12:50:13 -0700 Created attachment 250376 the patch 1083716 2 fpizlo 2015-04-08 13:24:02 -0700 Landed in http://trac.webkit.org/changeset/182562 1084323 3 250376 darin 2015-04-10 09:36:33 -0700 Comment on attachment 250376 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=250376&action=review > Source/JavaScriptCore/dfg/DFGIntegerCheckCombiningPhase.cpp:367 > + uint32_t unsignedDifference = maxBound - minBound; > + return !(unsignedDifference >> 31); Could also have written: int32_t difference = maxBound - minBound; return difference >= 0; 250376 2015-04-08 12:50:13 -0700 2015-04-08 13:10:00 -0700 the patch blah.patch text/plain 2454 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTgyNTYwKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBA CisyMDE1LTA0LTA4ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg REZHOjpJbnRlZ2VyQ2hlY2tDb21iaW5pbmdQaGFzZSdzIHdyYXAtYXJvdW5kIGNoZWNrIHNob3Vs ZG4ndCB0cmlnZ2VyIEMrKyB1bmRlZiBiZWhhdmlvciBvbiB3cmFwLWFyb3VuZAorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQzNTMyCisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisgICAgICAgIAorICAgICAgICBPaCB0aGUgaXJv bnkhICBXZSB3ZXJlIHByb3RlY3RpbmcgYW4gb3B0aW1pemF0aW9uIHRoYXQgb25seSB3b3JrZWQg aWYgdGhlcmUgd2FzIG5vIHdyYXAtYXJvdW5kIGluIEphdmFTY3JpcHQuCisgICAgICAgIEJ1dCB0 aGUgQysrIGNvZGUgaGFkIHdyYXAtYXJvdW5kLCB3aGljaCBpcyB1bmRlZiBpbiBDKysuICBTbywg aWYgdGhlIGNvbXBpbGVyIHdhcyBzbWFydCBlbm91Z2gsIG91ciBjb21waWxlcgorICAgICAgICB3 b3VsZCB0aGluayB0aGF0IHRoZXJlIG5ldmVyIHdhcyB3cmFwLWFyb3VuZC4KKyAgICAgICAgCisg ICAgICAgIFRoaXMgZml4ZXMgYSBmYWlsdXJlIGluIHN0cmVzcy90cmlja3ktYXJyYXktYm9pdW5k cy1jaGVja3MuanMgd2hlbiBKU0MgaXMgY29tcGlsZWQgd2l0aCBibGVlZGluZy1lZGdlIGNsYW5n LgorCisgICAgICAgICogZGZnL0RGR0ludGVnZXJDaGVja0NvbWJpbmluZ1BoYXNlLmNwcDoKKyAg ICAgICAgKEpTQzo6REZHOjpJbnRlZ2VyQ2hlY2tDb21iaW5pbmdQaGFzZTo6aXNWYWxpZCk6CisK IDIwMTUtMDQtMDcgIE1pY2hhZWwgU2Fib2ZmICA8bXNhYm9mZkBhcHBsZS5jb20+CiAKICAgICAg ICAgTGF6aWx5IGluaXRpYWxpemUgTG9nVG9TeXN0ZW1Db25zb2xlIGZsYWcgdG8gcmVkdWNlIG1l bW9yeSB1c2FnZQpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdJbnRlZ2VyQ2hl Y2tDb21iaW5pbmdQaGFzZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3Jl L2RmZy9ERkdJbnRlZ2VyQ2hlY2tDb21iaW5pbmdQaGFzZS5jcHAJKHJldmlzaW9uIDE4MjU1NykK KysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHSW50ZWdlckNoZWNrQ29tYmluaW5nUGhh c2UuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0zNTUsOCArMzU1LDE3IEBAIHByaXZhdGU6CiAgICAg ICAgICAgICByZXR1cm4gZmFsc2U7CiAgICAgICAgIAogICAgICAgICBzd2l0Y2ggKGtleS5tX2tp bmQpIHsKLSAgICAgICAgY2FzZSBBcnJheUJvdW5kczoKLSAgICAgICAgICAgIHJldHVybiAocmFu Z2UubV9tYXhCb3VuZCAtIHJhbmdlLm1fbWluQm91bmQpID49IDA7CisgICAgICAgIGNhc2UgQXJy YXlCb3VuZHM6IHsKKyAgICAgICAgICAgIC8vIEhhdmUgdG8gZG8gdGhpcyBjYXJlZnVsbHkgYmVj YXVzZSBDKysgY29tcGlsZXJzIGFyZSB0b28gc21hcnQuIEJ1dCBhbGwgd2UncmUgcmVhbGx5IGRv aW5nIGlzIGRldGVjdGluZyBpZgorICAgICAgICAgICAgLy8gdGhlIGRpZmZlcmVuY2UgYmV0d2Vl biB0aGUgYm91bmRzIGlzIDJeMzEgb3IgbW9yZS4gSWYgaXQgd2FzLCB0aGVuIHdlJ2QgaGF2ZSB0 byB3b3JyeSBhYm91dCB3cmFwLWFyb3VuZC4KKyAgICAgICAgICAgIC8vIFRoZSB3YXkgd2UnZCBs aWtlIHRvIHdyaXRlIHRoaXMgZXhwcmVzc2lvbiBpcyAocmFuZ2UubV9tYXhCb3VuZCAtIHJhbmdl Lm1fbWluQm91bmQpID49IDAsIGJ1dCB0aGF0IGlzIGEKKyAgICAgICAgICAgIC8vIHNpZ25lZCBz dWJ0cmFjdGlvbiBhbmQgY29tcGFyZSwgd2hpY2ggYWxsb3dzIHRoZSBDKysgY29tcGlsZXIgdG8g ZG8gYW55dGhpbmcgaXQgd2FudHMgaW4gY2FzZSBvZgorICAgICAgICAgICAgLy8gd3JhcC1hcm91 bmQuCisgICAgICAgICAgICB1aW50MzJfdCBtYXhCb3VuZCA9IHJhbmdlLm1fbWF4Qm91bmQ7Cisg ICAgICAgICAgICB1aW50MzJfdCBtaW5Cb3VuZCA9IHJhbmdlLm1fbWluQm91bmQ7CisgICAgICAg ICAgICB1aW50MzJfdCB1bnNpZ25lZERpZmZlcmVuY2UgPSBtYXhCb3VuZCAtIG1pbkJvdW5kOwor ICAgICAgICAgICAgcmV0dXJuICEodW5zaWduZWREaWZmZXJlbmNlID4+IDMxKTsKKyAgICAgICAg fQogICAgICAgICAgICAgCiAgICAgICAgIGRlZmF1bHQ6CiAgICAgICAgICAgICByZXR1cm4gdHJ1 ZTsK