186196 2018-06-01 08:54:38 -0700 [JSC] Correct values and members of JSBigInt appropriately 2018-06-01 12:01:18 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 ysuzuki ysuzuki darin ews-watchlist keith_miller mark.lam msaboff saam webkit-bug-importer oldest_to_newest 1429231 0 ysuzuki 2018-06-01 08:54:38 -0700 [JSC] Correct values and members of JSBigInt appropriately 1429233 1 341763 ysuzuki 2018-06-01 08:56:19 -0700 Created attachment 341763 Patch 1429266 2 341763 darin 2018-06-01 10:26:33 -0700 Comment on attachment 341763 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341763&action=review > Source/JavaScriptCore/ChangeLog:11 > + 2. JSBigInt::allocationSize should be annotated with `inline`. Does this have any effect? Recently I’ve discovered that for the latest versions of clang, at least, "inline" really means "permission to define this in multiple translation units" or in other words "this function is in a header", and isn’t having an effect on what actually gets inlined. > Source/JavaScriptCore/runtime/JSBigInt.cpp:1309 > - ASSERT(msdTopBit == (xBitLength - 1) % digitBits); > + ASSERT(msdTopBit == static_cast<int>((xBitLength - 1) % digitBits)); Why this change? 1429295 3 341763 ysuzuki 2018-06-01 11:46:41 -0700 Comment on attachment 341763 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341763&action=review Thanks! >> Source/JavaScriptCore/ChangeLog:11 >> + 2. JSBigInt::allocationSize should be annotated with `inline`. > > Does this have any effect? Recently I’ve discovered that for the latest versions of clang, at least, "inline" really means "permission to define this in multiple translation units" or in other words "this function is in a header", and isn’t having an effect on what actually gets inlined. Yeah, in GCC, this is effective. Maybe, this is because this `allocationSize()` function is static class member function, so it can be called outside of JSBigInt.cpp. This `inline` prevents `allocationSize()` function from being called outside of this JSBigInt.cpp. c6400e: 48 8b 87 d0 ff 00 00 mov 0xffd0(%rdi),%rax c64015: 48 89 fd mov %rdi,%rbp c64018: 48 85 c0 test %rax,%rax c6401b: 74 03 je c64020 <JSC::JSBigInt::createWithLength(JSC::VM&, unsigned int)+0x20> c6401d: 4c 8b 28 mov (%rax),%r13 c64020: 44 89 e7 mov %r12d,%edi c64023: e8 68 69 70 ff callq 36a990 <JSC::JSBigInt::allocationSize(unsigned int)@plt> c64028: 48 3d 60 1f 00 00 cmp $0x1f60,%rax c6402e: 76 50 jbe c64080 <JSC::JSBigInt::createWithLength(JSC::VM&, unsigned int)+0x80> c64030: 48 8d bd 40 36 00 00 lea 0x3640(%rbp),%rdi c64037: 45 31 c0 xor %r8d,%r8d c6403a: 31 c9 xor %ecx,%ecx c6403c: 48 89 c2 mov %rax,%rdx >> Source/JavaScriptCore/runtime/JSBigInt.cpp:1309 >> + ASSERT(msdTopBit == static_cast<int>((xBitLength - 1) % digitBits)); > > Why this change? It emits warnings with GCC since this comparison is signed v.s. unsigned. 1429296 4 ysuzuki 2018-06-01 11:47:32 -0700 Committed r232401: <https://trac.webkit.org/changeset/232401> 1429304 5 webkit-bug-importer 2018-06-01 12:01:18 -0700 <rdar://problem/40729485> 341763 2018-06-01 08:56:19 -0700 2018-06-01 10:26:33 -0700 Patch bug-186196-20180602005618.patch text/plain 5026 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjMyMzkzCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA1 MjZjMzFlMDY0M2UyNGVhOTRjYTAwNWUzYzc3ZTlkNjJmMTdhY2VmLi40NzQ5OGE4ZThlZTFhNjU1 MmJkNDk0NzhmZmQ2YjRmODcyMWI5Yjg4IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyNSBAQAorMjAxOC0wNi0wMSAgWXVzdWtlIFN1enVraSAgPHV0YXRhbmUudGVhQGdtYWls LmNvbT4KKworICAgICAgICBbSlNDXSBDb3JyZWN0IHZhbHVlcyBhbmQgbWVtYmVycyBvZiBKU0Jp Z0ludCBhcHByb3ByaWF0ZWx5CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xODYxOTYKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBUaGlzIHBhdGNoIGNsZWFucyB1cCBhIGJpdCB0byBzZWxlY3QgbW9yZSBhcHBy b3ByaWF0ZSB2YWx1ZXMgYW5kIG1lbWJlcnMgb2YgSlNCaWdJbnQuCisKKyAgICAgICAgMS4gSlNC aWdJbnQncyBzdHJ1Y3R1cmUgc2hvdWxkIGJlIFN0cnVjdHVyZUlzSW1tb3J0YWwuCisgICAgICAg IDIuIEpTQmlnSW50OjphbGxvY2F0aW9uU2l6ZSBzaG91bGQgYmUgYW5ub3RhdGVkIHdpdGggYGlu bGluZWAuCisgICAgICAgIDMuIFJlbW92ZSBKU0JpZ0ludDo6dmlzaXRDaGlsZHJlbiBzaW5jZSBp dCBpcyBjb21wbGV0ZWx5IHRoZSBzYW1lIHRvIEpTQ2VsbDo6dmlzaXRDaGlsZHJlbi4KKyAgICAg ICAgNC4gUmVtb3ZlIEpTQmlnSW50OjpmaW5pc2hDcmVhdGlvbiBzaW5jZSBpdCBpcyBjb21wbGV0 ZWx5IHRoZSBzYW1lIHRvIEpTQ2VsbDo6ZmluaXNoQ3JlYXRpb24uCisKKyAgICAgICAgKiBydW50 aW1lL0pTQmlnSW50LmNwcDoKKyAgICAgICAgKEpTQzo6SlNCaWdJbnQ6OmFsbG9jYXRpb25TaXpl KToKKyAgICAgICAgKEpTQzo6SlNCaWdJbnQ6OmFsbG9jYXRlRm9yKToKKyAgICAgICAgKEpTQzo6 SlNCaWdJbnQ6OmNvbXBhcmVUb0RvdWJsZSk6CisgICAgICAgIChKU0M6OkpTQmlnSW50Ojp2aXNp dENoaWxkcmVuKTogRGVsZXRlZC4KKyAgICAgICAgKEpTQzo6SlNCaWdJbnQ6OmZpbmlzaENyZWF0 aW9uKTogRGVsZXRlZC4KKyAgICAgICAgKiBydW50aW1lL0pTQmlnSW50Lmg6CisKIDIwMTgtMDUt MzEgIFl1c3VrZSBTdXp1a2kgIDx1dGF0YW5lLnRlYUBnbWFpbC5jb20+CiAKICAgICAgICAgW0Jh c2VsaW5lXSBTdG9yZSBjb25zdGFudCBkaXJlY3RseSBpbiBlbWl0X29wX21vdgpkaWZmIC0tZ2l0 IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNCaWdJbnQuY3BwIGIvU291cmNlL0ph dmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNCaWdJbnQuY3BwCmluZGV4IDg1NDUyMzMzNWVlYjViYzIx YWVmYzVkYjE1MDJmY2QyZTE3ZmEyYzcuLjY3NDJkOWJjMTNhMjU1YjQ2YjdmZGMzNmI5NGQwZWZj MzVmZmI2NjAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTQmln SW50LmNwcAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU0JpZ0ludC5jcHAK QEAgLTYxLDEzICs2MSw2IEBAIG5hbWVzcGFjZSBKU0MgewogY29uc3QgQ2xhc3NJbmZvIEpTQmln SW50OjpzX2luZm8gPQogICAgIHsgIkpTQmlnSW50IiwgbnVsbHB0ciwgbnVsbHB0ciwgbnVsbHB0 ciwgQ1JFQVRFX01FVEhPRF9UQUJMRShKU0JpZ0ludCkgfTsKIAotdm9pZCBKU0JpZ0ludDo6dmlz aXRDaGlsZHJlbihKU0NlbGwqIGNlbGwsIFNsb3RWaXNpdG9yJiB2aXNpdG9yKQotewotICAgIEpT QmlnSW50KiB0aGlzT2JqZWN0ID0ganNDYXN0PEpTQmlnSW50Kj4oY2VsbCk7Ci0gICAgQVNTRVJU X0dDX09CSkVDVF9JTkhFUklUUyh0aGlzT2JqZWN0LCBpbmZvKCkpOwotICAgIEJhc2U6OnZpc2l0 Q2hpbGRyZW4odGhpc09iamVjdCwgdmlzaXRvcik7Ci19Ci0KIEpTQmlnSW50OjpKU0JpZ0ludChW TSYgdm0sIFN0cnVjdHVyZSogc3RydWN0dXJlLCB1bnNpZ25lZCBsZW5ndGgpCiAgICAgOiBCYXNl KHZtLCBzdHJ1Y3R1cmUpCiAgICAgLCBtX2xlbmd0aChsZW5ndGgpCkBAIC05Miw3ICs4NSw3IEBA IEpTQmlnSW50KiBKU0JpZ0ludDo6Y3JlYXRlWmVybyhWTSYgdm0pCiAgICAgcmV0dXJuIHplcm9C aWdJbnQ7CiB9CiAKLXNpemVfdCBKU0JpZ0ludDo6YWxsb2NhdGlvblNpemUodW5zaWduZWQgbGVu Z3RoKQoraW5saW5lIHNpemVfdCBKU0JpZ0ludDo6YWxsb2NhdGlvblNpemUodW5zaWduZWQgbGVu Z3RoKQogewogICAgIHNpemVfdCBzaXplV2l0aFBhZGRpbmcgPSBXVEY6OnJvdW5kVXBUb011bHRp cGxlT2Y8c2l6ZW9mKHNpemVfdCk+KHNpemVvZihKU0JpZ0ludCkpOwogICAgIHJldHVybiBzaXpl V2l0aFBhZGRpbmcgKyBsZW5ndGggKiBzaXplb2YoRGlnaXQpOwpAQCAtMTA1LDEyICs5OCw2IEBA IEpTQmlnSW50KiBKU0JpZ0ludDo6Y3JlYXRlV2l0aExlbmd0aChWTSYgdm0sIHVuc2lnbmVkIGxl bmd0aCkKICAgICByZXR1cm4gYmlnSW50OwogfQogCi12b2lkIEpTQmlnSW50OjpmaW5pc2hDcmVh dGlvbihWTSYgdm0pCi17Ci0gICAgQmFzZTo6ZmluaXNoQ3JlYXRpb24odm0pOwotfQotCi0KIEpT QmlnSW50KiBKU0JpZ0ludDo6Y3JlYXRlRnJvbShWTSYgdm0sIGludDMyX3QgdmFsdWUpCiB7CiAg ICAgaWYgKCF2YWx1ZSkKQEAgLTEwNTgsNyArMTA0NSw2IEBAIEpTQmlnSW50KiBKU0JpZ0ludDo6 cmlnaHRUcmltKFZNJiB2bSkKIEpTQmlnSW50KiBKU0JpZ0ludDo6YWxsb2NhdGVGb3IoRXhlY1N0 YXRlKiBzdGF0ZSwgVk0mIHZtLCB1bnNpZ25lZCByYWRpeCwgdW5zaWduZWQgY2hhcmNvdW50KQog ewogICAgIEFTU0VSVCgyIDw9IHJhZGl4ICYmIHJhZGl4IDw9IDM2KTsKLSAgICBBU1NFUlQoY2hh cmNvdW50ID49IDApOwogCiAgICAgc2l6ZV90IGJpdHNQZXJDaGFyID0gbWF4Qml0c1BlckNoYXJU YWJsZVtyYWRpeF07CiAgICAgc2l6ZV90IGNoYXJzID0gY2hhcmNvdW50OwpAQCAtMTMyMCw3ICsx MzA2LDcgQEAgSlNCaWdJbnQ6OkNvbXBhcmlzb25SZXN1bHQgSlNCaWdJbnQ6OmNvbXBhcmVUb0Rv dWJsZShKU0JpZ0ludCogeCwgZG91YmxlIHkpCiAKICAgICAvLyAwLWluZGV4ZWQgcG9zaXRpb24g b2Yge3h9J3MgbW9zdCBzaWduaWZpY2FudCBiaXQgd2l0aGluIHRoZSB7bXNkfS4KICAgICBpbnQg bXNkVG9wQml0ID0gZGlnaXRCaXRzIC0gMSAtIG1zZExlYWRpbmdaZXJvczsKLSAgICBBU1NFUlQo bXNkVG9wQml0ID09ICh4Qml0TGVuZ3RoIC0gMSkgJSBkaWdpdEJpdHMpOworICAgIEFTU0VSVCht c2RUb3BCaXQgPT0gc3RhdGljX2Nhc3Q8aW50PigoeEJpdExlbmd0aCAtIDEpICUgZGlnaXRCaXRz KSk7CiAgICAgCiAgICAgLy8gU2hpZnRlZCBjaHVuayBvZiB7bWFudGlzc2F9IGZvciBjb21wYXJp bmcgd2l0aCB7ZGlnaXR9LgogICAgIERpZ2l0IGNvbXBhcmVNYW50aXNzYTsKZGlmZiAtLWdpdCBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTQmlnSW50LmggYi9Tb3VyY2UvSmF2YVNj cmlwdENvcmUvcnVudGltZS9KU0JpZ0ludC5oCmluZGV4IDI4ZTRkYTZjY2NkNzEyNjk0YTNkOWIx YjAzZjU3YzJmZjMwNTE4YzEuLmZiYTVhNmQyZDZmMzg2Y2E5ODg5N2E4ZWM0NjRjODQ0MzljODQ0 MjQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTQmlnSW50LmgK KysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNCaWdJbnQuaApAQCAtMzcsNyAr MzcsNyBAQCBuYW1lc3BhY2UgSlNDIHsKIAogY2xhc3MgSlNCaWdJbnQgZmluYWwgOiBwdWJsaWMg SlNDZWxsIHsKICAgICB1c2luZyBCYXNlID0gSlNDZWxsOwotICAgIHN0YXRpYyBjb25zdCB1bnNp Z25lZCBTdHJ1Y3R1cmVGbGFncyA9IEJhc2U6OlN0cnVjdHVyZUZsYWdzIHwgT3ZlcnJpZGVzVG9U aGlzOworICAgIHN0YXRpYyBjb25zdCB1bnNpZ25lZCBTdHJ1Y3R1cmVGbGFncyA9IEJhc2U6OlN0 cnVjdHVyZUZsYWdzIHwgU3RydWN0dXJlSXNJbW1vcnRhbCB8IE92ZXJyaWRlc1RvVGhpczsKIAog cHVibGljOgogCkBAIC00NiwxMCArNDYsNyBAQCBjbGFzcyBKU0JpZ0ludCBmaW5hbCA6IHB1Ymxp YyBKU0NlbGwgewogICAgIGVudW0gY2xhc3MgSW5pdGlhbGl6YXRpb25UeXBlIHsgTm9uZSwgV2l0 aFplcm8gfTsKICAgICB2b2lkIGluaXRpYWxpemUoSW5pdGlhbGl6YXRpb25UeXBlKTsKIAotICAg IHN0YXRpYyB2b2lkIHZpc2l0Q2hpbGRyZW4oSlNDZWxsKiwgU2xvdFZpc2l0b3ImKTsKLQogICAg IHN0YXRpYyBzaXplX3QgZXN0aW1hdGVkU2l6ZShKU0NlbGwqKTsKLSAgICBzdGF0aWMgc2l6ZV90 IGFsbG9jYXRpb25TaXplKHVuc2lnbmVkIGxlbmd0aCk7CiAKICAgICBzdGF0aWMgU3RydWN0dXJl KiBjcmVhdGVTdHJ1Y3R1cmUoVk0mLCBKU0dsb2JhbE9iamVjdCosIEpTVmFsdWUgcHJvdG90eXBl KTsKICAgICBzdGF0aWMgSlNCaWdJbnQqIGNyZWF0ZVplcm8oVk0mKTsKQEAgLTYyLDggKzU5LDYg QEAgY2xhc3MgSlNCaWdJbnQgZmluYWwgOiBwdWJsaWMgSlNDZWxsIHsKIAogICAgIERFQ0xBUkVf RVhQT1JUX0lORk87CiAKLSAgICB2b2lkIGZpbmlzaENyZWF0aW9uKFZNJik7Ci0KICAgICBKU1Zh bHVlIHRvUHJpbWl0aXZlKEV4ZWNTdGF0ZSosIFByZWZlcnJlZFByaW1pdGl2ZVR5cGUpIGNvbnN0 OwogCiAgICAgdm9pZCBzZXRTaWduKGJvb2wgc2lnbikgeyBtX3NpZ24gPSBzaWduOyB9CkBAIC0x NzUsNiArMTcwLDcgQEAgY2xhc3MgSlNCaWdJbnQgZmluYWwgOiBwdWJsaWMgSlNDZWxsIHsKIAog ICAgIHZvaWQgaW5wbGFjZU11bHRpcGx5QWRkKERpZ2l0IG11bHRpcGxpZXIsIERpZ2l0IHBhcnQp OwogICAgIAorICAgIHN0YXRpYyBzaXplX3QgYWxsb2NhdGlvblNpemUodW5zaWduZWQgbGVuZ3Ro KTsKICAgICBzdGF0aWMgc2l6ZV90IG9mZnNldE9mRGF0YSgpOwogICAgIERpZ2l0KiBkYXRhU3Rv cmFnZSgpOwogCg==