212193 2020-05-21 01:51:42 -0700 [JSC] Fix 32bit JSBigInt with INT32_MAX < x <= UINT32_MAX 2020-05-21 13:39:42 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 ysuzuki ysuzuki ews-watchlist keith_miller mark.lam msaboff saam tzagallo webkit-bug-importer oldest_to_newest 1654844 0 ysuzuki 2020-05-21 01:51:42 -0700 [JSC] Fix 32bit JSBigInt with INT32_MAX < x <= UINT32_MAX 1654845 1 399945 ysuzuki 2020-05-21 02:02:01 -0700 Created attachment 399945 Patch 1654938 2 399945 mark.lam 2020-05-21 09:06:52 -0700 Comment on attachment 399945 Patch r=me 1654975 3 ews-feeder 2020-05-21 10:21:19 -0700 Committed r262012: <https://trac.webkit.org/changeset/262012> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399945. 1654977 4 webkit-bug-importer 2020-05-21 10:22:21 -0700 <rdar://problem/63498967> 1655035 5 399945 saam 2020-05-21 12:51:43 -0700 Comment on attachment 399945 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399945&action=review > Source/JavaScriptCore/runtime/JSBigInt.cpp:176 > + unsignedValue = static_cast<uint64_t>(-(value + 1)) + 1; why not just use abs? 1655038 6 399945 mark.lam 2020-05-21 12:54:22 -0700 Comment on attachment 399945 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399945&action=review >> Source/JavaScriptCore/runtime/JSBigInt.cpp:176 >> + unsignedValue = static_cast<uint64_t>(-(value + 1)) + 1; > > why not just use abs? Would abs guarantee no undefined behavior when value is INT64_MIN? 1655039 7 saam 2020-05-21 12:54:29 -0700 (In reply to Saam Barati from comment #5) > Comment on attachment 399945 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=399945&action=review > > > Source/JavaScriptCore/runtime/JSBigInt.cpp:176 > > + unsignedValue = static_cast<uint64_t>(-(value + 1)) + 1; > > why not just use abs? Maybe we're explicitly avoiding UB if Abs has UB for INT_MIN? 1655040 8 saam 2020-05-21 12:54:53 -0700 (In reply to Mark Lam from comment #6) > Comment on attachment 399945 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=399945&action=review > > >> Source/JavaScriptCore/runtime/JSBigInt.cpp:176 > >> + unsignedValue = static_cast<uint64_t>(-(value + 1)) + 1; > > > > why not just use abs? > > Would abs guarantee no undefined behavior when value is INT64_MIN? Yep that's the question. Maybe we should invent our own abs if it does have UB 1655057 9 399945 ysuzuki 2020-05-21 13:39:42 -0700 Comment on attachment 399945 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399945&action=review >>>>> Source/JavaScriptCore/runtime/JSBigInt.cpp:176 >>>>> + unsignedValue = static_cast<uint64_t>(-(value + 1)) + 1; >>>> >>>> why not just use abs? >>> >>> Would abs guarantee no undefined behavior when value is INT64_MIN? >> >> Maybe we're explicitly avoiding UB if Abs has UB for INT_MIN? > > Yep that's the question. Maybe we should invent our own abs if it does have UB abs always returns signed types. https://en.cppreference.com/w/cpp/numeric/math/abs So, std::abs(value) -> int64_t. And in this case, INT64_MIN will not fit and this is UB[1]. [1]: "Computes the absolute value of an integer number. The behavior is undefined if the result cannot be represented by the return type." from https://en.cppreference.com/w/cpp/numeric/math/abs 399945 2020-05-21 02:02:01 -0700 2020-05-21 10:21:20 -0700 Patch bug-212193-20200521020200.patch text/plain 5093 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjYxOTg2CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBm ZmMzMTZiYTA2NzYxMTRhNjBmZGI1MDk2NDQ4YTdiNjJmYjYzNzRlLi5kMzY0OTZjNmFlNGY4OTFl MWU0MTkwMjM0ZDVkY2MxMzAzMTcyNTVmIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyNCBAQAorMjAyMC0wNS0yMSAgWXVzdWtlIFN1enVraSAgPHlzdXp1a2lAYXBwbGUuY29t PgorCisgICAgICAgIFtKU0NdIEZpeCAzMmJpdCBKU0JpZ0ludCB3aXRoIElOVDMyX01BWCA8IHgg PD0gVUlOVDMyX01BWAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MjEyMTkzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgSW4gMzJiaXQgYXJjaGl0ZWN0dXJlLCB3ZSBhcmUgY3JlYXRpbmcgb25lLWxlbmd0aCBK U0JpZ0ludCBmb3IgSU5UMzJfTUlOIDw9IHggPD0gSU5UMzJfTUFYLCBhbmQgdHdvLWxlbmd0aCBK U0JpZ0ludCBvdGhlcndpc2UuCisgICAgICAgIFRoaXMgaXMgd3Jvbmcgc2luY2Ugb25lLWxlbmd0 aCBKU0JpZ0ludCBzaG91bGQgY292ZXIgZnJvbSAtVUlOVDMyX01BWCA8PSB4IDw9IFVJTlQzMl9N QVguCisKKyAgICAgICAgVGhpcyBwYXRjaCBmaXhlcyB0aGUgYnVnIGFuZCBjbGVhbnMgdXAgY3Jl YXRlRnJvbShWTSYsIGludDY0X3QpLiBBbmQgaXQgYWxzbyBhZGRzIEpTQmlnSW50OjpjcmVhdGVG cm9tKFZNJiwgdWludDY0X3QpIGluIHByZXBhcmF0aW9uIGZvciBbMV0KKyAgICAgICAgQ3VycmVu dGx5LCB0aGlzIHBhdGggaXMgbm90IHVzZWQgd2hpbGUgaXQgd2FzIHVzZWQgcHJldmlvdXNseSBi ZWNhdXNlIEJpZ0ludENvbnN0cnVjdG9yIHN0YXJ0cyB1c2luZyBKU0JpZ0ludDo6Y3JlYXRlRnJv bShWTSYsIGRvdWJsZSkuIEJ1dCB0aGlzCisgICAgICAgIHdpbGwgYmUgdXNlZCBpbiBbMV0sIGFu ZCBzaW1wbHkgdGhlIGV4aXN0aW5nIGltcGxlbWVudGF0aW9uIGlzIHdyb25nLgorCisgICAgICAg IFsxXTogaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE5MDgwMAorCisg ICAgICAgICogcnVudGltZS9KU0JpZ0ludC5jcHA6CisgICAgICAgIChKU0M6OkpTQmlnSW50Ojpj cmVhdGVGcm9tSW1wbCk6CisgICAgICAgIChKU0M6OkpTQmlnSW50OjpjcmVhdGVGcm9tKToKKyAg ICAgICAgKiBydW50aW1lL0pTQmlnSW50Lmg6CisKIDIwMjAtMDUtMjAgIE1pY2hhZWwgU2Fib2Zm ICA8bXNhYm9mZkBhcHBsZS5jb20+CiAKICAgICAgICAgW1dhc21dIExpbWl0IHRoZSBzaXplIG9m IFdhc20gZnVuY3Rpb24gd2Ugb3B0aW1pemUgaW4gT01HIG1vZGUKZGlmZiAtLWdpdCBhL1NvdXJj ZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTQmlnSW50LmNwcCBiL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9ydW50aW1lL0pTQmlnSW50LmNwcAppbmRleCAzNWVmZmQ1YmI2YzZlYTcwYmU2MWI2NDQ1 ODYzOWQzYjZiMzQyMTJjLi41MDdlYmMyYzcwMzlkZTlmYWNiNGE3YTZmZTRhYjgxYTI0M2YxNzQx IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU0JpZ0ludC5jcHAK KysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNCaWdJbnQuY3BwCkBAIC0xMzUs NDMgKzEzNSw1MSBAQCBKU0JpZ0ludCogSlNCaWdJbnQ6OmNyZWF0ZUZyb20oVk0mIHZtLCB1aW50 MzJfdCB2YWx1ZSkKICAgICByZXR1cm4gYmlnSW50OwogfQogCi1KU0JpZ0ludCogSlNCaWdJbnQ6 OmNyZWF0ZUZyb20oVk0mIHZtLCBpbnQ2NF90IHZhbHVlKQoraW5saW5lIEpTQmlnSW50KiBKU0Jp Z0ludDo6Y3JlYXRlRnJvbUltcGwoVk0mIHZtLCB1aW50NjRfdCB2YWx1ZSwgYm9vbCBzaWduKQog ewogICAgIGlmICghdmFsdWUpCiAgICAgICAgIHJldHVybiBjcmVhdGVaZXJvKHZtKTsKIAogICAg IC8vIFRoaXMgcGF0aCBpcyBub3QganVzdCBhbiBvcHRpbWl6YXRpb246IGJlY2F1c2Ugd2UgZG8g bm90IGNhbGwgcmlnaHRUcmltIGF0IHRoZSBlbmQgb2YgdGhpcyBmdW5jdGlvbiwKICAgICAvLyBp dCB3b3VsZCBiZSBhIGJ1ZyB0byBjcmVhdGUgYSBCaWdJbnQgd2l0aCBsZW5ndGg9MiBpbiB0aGlz IGNhc2UuCi0gICAgaWYgKHNpemVvZihEaWdpdCkgPT0gOCB8fCAodmFsdWUgPD0gSU5UX01BWCAm JiB2YWx1ZSA+PSBJTlRfTUlOKSkgeworICAgIGlmIChzaXplb2YoRGlnaXQpID09IDggfHwgdmFs dWUgPD0gVUlOVDMyX01BWCkgewogICAgICAgICBKU0JpZ0ludCogYmlnSW50ID0gY3JlYXRlV2l0 aExlbmd0aFVuY2hlY2tlZCh2bSwgMSk7Ci0gICAgICAgIGlmICh2YWx1ZSA8IDApIHsKLSAgICAg ICAgICAgIGJpZ0ludC0+c2V0RGlnaXQoMCwgc3RhdGljX2Nhc3Q8RGlnaXQ+KHN0YXRpY19jYXN0 PHVpbnQ2NF90PigtKHZhbHVlICsgMSkpICsgMSkpOwotICAgICAgICAgICAgYmlnSW50LT5zZXRT aWduKHRydWUpOwotICAgICAgICB9IGVsc2UKLSAgICAgICAgICAgIGJpZ0ludC0+c2V0RGlnaXQo MCwgc3RhdGljX2Nhc3Q8RGlnaXQ+KHZhbHVlKSk7Ci0gICAgICAgIAorICAgICAgICBiaWdJbnQt PnNldERpZ2l0KDAsIHN0YXRpY19jYXN0PERpZ2l0Pih2YWx1ZSkpOworICAgICAgICBiaWdJbnQt PnNldFNpZ24oc2lnbik7CiAgICAgICAgIHJldHVybiBiaWdJbnQ7CiAgICAgfQotICAgIAorCisg ICAgQVNTRVJUKHNpemVvZihEaWdpdCkgPT0gNCk7CiAgICAgSlNCaWdJbnQqIGJpZ0ludCA9IGNy ZWF0ZVdpdGhMZW5ndGhVbmNoZWNrZWQodm0sIDIpOwotICAgIHVpbnQ2NF90IHRlbXBWYWx1ZTsK LSAgICBib29sIHNpZ24gPSBmYWxzZTsKLSAgICBpZiAodmFsdWUgPCAwKSB7Ci0gICAgICAgIHRl bXBWYWx1ZSA9IHN0YXRpY19jYXN0PHVpbnQ2NF90PigtKHZhbHVlICsgMSkpICsgMTsKLSAgICAg ICAgc2lnbiA9IHRydWU7Ci0gICAgfSBlbHNlCi0gICAgICAgIHRlbXBWYWx1ZSA9IHZhbHVlOwot ICAgIAotICAgIERpZ2l0IGxvd0JpdHMgID0gc3RhdGljX2Nhc3Q8RGlnaXQ+KHRlbXBWYWx1ZSAm IDB4ZmZmZmZmZmYpOwotICAgIERpZ2l0IGhpZ2hCaXRzID0gc3RhdGljX2Nhc3Q8RGlnaXQ+KCh0 ZW1wVmFsdWUgPj4gMzIpICYgMHhmZmZmZmZmZik7Ci0gICAgCisgICAgRGlnaXQgbG93Qml0cyAg PSBzdGF0aWNfY2FzdDxEaWdpdD4odmFsdWUgJiAweGZmZmZmZmZmKTsKKyAgICBEaWdpdCBoaWdo Qml0cyA9IHN0YXRpY19jYXN0PERpZ2l0PigodmFsdWUgPj4gMzIpICYgMHhmZmZmZmZmZik7CisK KyAgICBBU1NFUlQoaGlnaEJpdHMpOworCiAgICAgYmlnSW50LT5zZXREaWdpdCgwLCBsb3dCaXRz KTsKICAgICBiaWdJbnQtPnNldERpZ2l0KDEsIGhpZ2hCaXRzKTsKICAgICBiaWdJbnQtPnNldFNp Z24oc2lnbik7Ci0gICAgCisKICAgICByZXR1cm4gYmlnSW50OwogfQogCitKU0JpZ0ludCogSlNC aWdJbnQ6OmNyZWF0ZUZyb20oVk0mIHZtLCB1aW50NjRfdCB2YWx1ZSkKK3sKKyAgICByZXR1cm4g Y3JlYXRlRnJvbUltcGwodm0sIHZhbHVlLCBmYWxzZSk7Cit9CisKK0pTQmlnSW50KiBKU0JpZ0lu dDo6Y3JlYXRlRnJvbShWTSYgdm0sIGludDY0X3QgdmFsdWUpCit7CisgICAgdWludDY0X3QgdW5z aWduZWRWYWx1ZTsKKyAgICBib29sIHNpZ24gPSBmYWxzZTsKKyAgICBpZiAodmFsdWUgPCAwKSB7 CisgICAgICAgIHVuc2lnbmVkVmFsdWUgPSBzdGF0aWNfY2FzdDx1aW50NjRfdD4oLSh2YWx1ZSAr IDEpKSArIDE7CisgICAgICAgIHNpZ24gPSB0cnVlOworICAgIH0gZWxzZQorICAgICAgICB1bnNp Z25lZFZhbHVlID0gdmFsdWU7CisgICAgcmV0dXJuIGNyZWF0ZUZyb21JbXBsKHZtLCB1bnNpZ25l ZFZhbHVlLCBzaWduKTsKK30KKwogSlNCaWdJbnQqIEpTQmlnSW50OjpjcmVhdGVGcm9tKFZNJiB2 bSwgYm9vbCB2YWx1ZSkKIHsKICAgICBpZiAoIXZhbHVlKQpkaWZmIC0tZ2l0IGEvU291cmNlL0ph dmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNCaWdJbnQuaCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9y dW50aW1lL0pTQmlnSW50LmgKaW5kZXggNzAwODRhN2Y3ZDU1NjZiODNhMzYwNzMzZDc5NWVkZTYw NzI4Mzg3NC4uNmJmOGFiMmE1NjcxOWY0N2U2Yjc3YTgwOTgwMTdmNGFiZWZhZDM4NyAxMDA2NDQK LS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNCaWdJbnQuaAorKysgYi9Tb3Vy Y2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU0JpZ0ludC5oCkBAIC03Miw2ICs3Miw3IEBAIGNs YXNzIEpTQmlnSW50IGZpbmFsIDogcHVibGljIEpTQ2VsbCB7CiAgICAgSlNfRVhQT1JUX1BSSVZB VEUgc3RhdGljIEpTQmlnSW50KiBjcmVhdGVGcm9tKFZNJiwgaW50MzJfdCB2YWx1ZSk7CiAgICAg c3RhdGljIEpTQmlnSW50KiBjcmVhdGVGcm9tKFZNJiwgdWludDMyX3QgdmFsdWUpOwogICAgIHN0 YXRpYyBKU0JpZ0ludCogY3JlYXRlRnJvbShWTSYsIGludDY0X3QgdmFsdWUpOworICAgIHN0YXRp YyBKU0JpZ0ludCogY3JlYXRlRnJvbShWTSYsIHVpbnQ2NF90IHZhbHVlKTsKICAgICBzdGF0aWMg SlNCaWdJbnQqIGNyZWF0ZUZyb20oVk0mLCBib29sIHZhbHVlKTsKICAgICBzdGF0aWMgSlNCaWdJ bnQqIGNyZWF0ZUZyb20oVk0mLCBkb3VibGUgdmFsdWUpOwogCkBAIC00MjcsNiArNDI4LDggQEAg Y2xhc3MgSlNCaWdJbnQgZmluYWwgOiBwdWJsaWMgSlNDZWxsIHsKIHByaXZhdGU6CiAgICAgSlNC aWdJbnQoVk0mLCBTdHJ1Y3R1cmUqLCBEaWdpdCosIHVuc2lnbmVkIGxlbmd0aCk7CiAKKyAgICBz dGF0aWMgSlNCaWdJbnQqIGNyZWF0ZUZyb21JbXBsKFZNJiwgdWludDY0X3QgdmFsdWUsIGJvb2wg c2lnbik7CisKICAgICBzdGF0aWMgY29uc3RleHByIHVuc2lnbmVkIGJpdHNQZXJCeXRlID0gODsK ICAgICBzdGF0aWMgY29uc3RleHByIHVuc2lnbmVkIGRpZ2l0Qml0cyA9IHNpemVvZihEaWdpdCkg KiBiaXRzUGVyQnl0ZTsKICAgICBzdGF0aWMgY29uc3RleHByIHVuc2lnbmVkIGhhbGZEaWdpdEJp dHMgPSBkaWdpdEJpdHMgLyAyOwo=