226503 2021-06-01 09:33:32 -0700 Fix inadvertent tag corruption in functionAddressOf 2021-06-09 08:21:48 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mikhail webkit-unassigned angelos darin ews-watchlist keith_miller mark.lam msaboff saam tzagallo webkit-bug-importer oldest_to_newest 1765639 0 430269 mikhail 2021-06-01 09:33:32 -0700 Created attachment 430269 test case The attached program triggers an assertion failure when running jsc in 32 bits and debug mode. In 32 bits, asCell() returns a pointer to the payload and using reinterpret_cast will sign extend asNumber to FFFF FFFF + <value-address>, which will fail the isImpureNaN in jsNumber. So first cast the address to uint32_t then to uint64_t to prevent the tag corruption. 1765644 1 430271 mikhail 2021-06-01 09:41:37 -0700 Created attachment 430271 Patch 1765660 2 430274 mikhail 2021-06-01 10:09:06 -0700 Created attachment 430274 Patch 1765759 3 430274 darin 2021-06-01 14:25:35 -0700 Comment on attachment 430274 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430274&action=review > Source/JavaScriptCore/jsc.cpp:1493 > +#if USE(JSVALUE32_64) > + // In 32 bits, asCell() returns a pointer to the payload and using reinterpret_cast > + // will sign extend asNumber to FFFF FFFF + <value-address>, which will fail the > + // isImpureNaN check in jsNumber. To prevent that, first reinterpret_cast the address > + // to uint32_t then cast it to uint64_t. AddressOf in 32 bits will return the same > + // address returned by describe() but in double > + uint64_t asNumber = static_cast<uint64_t>(reinterpret_cast<uint32_t>(value.asCell())); > +#else > // Need to cast to uint64_t so bitwise_cast will play along. > uint64_t asNumber = reinterpret_cast<uint64_t>(value.asCell()); > +#endif Better fix is to just replace the incorrect <uint64_t> with the correct <uintptr_t> instead of adding code and comments. No need to use static_cast to expand a 32-bit unsigned int to 64-bit. 1765796 4 mikhail 2021-06-01 15:29:09 -0700 Indeed, <uintptr_t> sounds like a better solution here, but we would still need the static_cast to 64 bits because of the bitcast<double>. In 32 bits <uintptr_t> is 32 bits long and the bitcast will fail. 1765805 5 darin 2021-06-01 15:43:22 -0700 (In reply to Mikhail R. Gadelha from comment #4) > Indeed, <uintptr_t> sounds like a better solution here, but we would still > need the static_cast to 64 bits because of the bitcast<double>. No static_cast needed. Write this: uint64_t asNumber = reinterpret_cast<uintptr_t>(value.asCell()); There’s no need for a static_cast on this line of code. The unsigned integer conversion, with possible promotion, can be done without any casting. 1765806 6 mikhail 2021-06-01 15:50:27 -0700 I got what you mean now! I'll send an updated patch shortly 1765809 7 430302 mikhail 2021-06-01 16:01:48 -0700 Created attachment 430302 Patch 1765812 8 430302 darin 2021-06-01 16:09:30 -0700 Comment on attachment 430302 Patch I suspect that other callers of reinterpret_cast<uint64_t> throughout WebKit could have the same issue on 32-bit. There are 20 of them at least, and many are likely converting from pointers to integers; the same fix would likely apply. 1767799 9 webkit-bug-importer 2021-06-08 09:34:18 -0700 <rdar://problem/79015579> 1768188 10 ews-feeder 2021-06-09 08:21:45 -0700 Committed r278660 (238642@main): <https://commits.webkit.org/238642@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 430302. 430269 2021-06-01 09:33:32 -0700 2021-06-01 09:33:32 -0700 test case foo.js text/javascript 36 mikhail ZnVuY3Rpb24gYSgpIHt9CnByaW50KGFkZHJlc3NPZihhKSkK 430271 2021-06-01 09:41:37 -0700 2021-06-01 10:11:27 -0700 Patch bug-226503-20210601124135.patch text/plain 2318 mikhail U3VidmVyc2lvbiBSZXZpc2lvbjogMjc4Mjc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBm Mzc3OTI5MDRlNmFkYmExZTIwZGFkZDVjNDBiZTIyZjU3YzA4YzEwLi41MjhiMjkyM2E5YmQ0YzEz ZWRhZTFiMjlhNTVkYjdlZjc1MDkyMzYzIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMiBAQAorMjAyMS0wNi0wMSAgTWlraGFpbCBSLiBHYWRlbGhhICA8bWlraGFpbEBpZ2Fs aWEuY29tPgorCisgICAgICAgIEZpeCBpbmFkdmVydGVudCB0YWcgY29ycnVwdGlvbiBpbiBmdW5j dGlvbkFkZHJlc3NPZgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MjI2NTAzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgT3JpZ2luYWwgcGF0Y2ggYnkgQW5nZWxvcyBPaWtvbm9tb3BvdWxvcy4KKworICAgICAg ICBUaGUgY2FzdCB3YXMgc2lnbi1leHRlbmRpbmcgdGhlIEpTVmFsdWUgcGF5bG9hZCwgc28gdGhh dCBhZGRyZXNzZXMKKyAgICAgICAgdGhhdCBoYWQgdGhlIG1vc3Qgc2lnbmlmaWNhbnQgc2V0IGdh dmUgdXMgYSBzaWduLWV4dGVuZGVkIHJlc3VsdCBpbgorICAgICAgICBhc051bWJlciB3aGljaCB3 YXMgdGhlbiBjb252ZXJ0ZWQgdG8gYW4gaW52YWxpZCBOYU4gYnkgdGhlIGJpdGNhc3QuCisgICAg CisgICAgICAgIEluc3RlYWQsIGNhc3QgdGhlIHBheWxvYWQgdG8gdWludDMyX3QsIHNvIHRoYXQg dGhlIGNhc3QgdG8gdWludDY0X3QKKyAgICAgICAgd2lsbCBub3Qgc2lnbi1leHRlbmQuCisgICAg ICAgIAorICAgICAgICAqIGpzYy5jcHA6CisgICAgICAgIChKU0NfREVGSU5FX0hPU1RfRlVOQ1RJ T04pOgorCiAyMDIxLTA1LTMwICBDaHJpcyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CiAKICAg ICAgICAgRHJvcCBVbmNoZWNrZWRDb25kaXRpb24gLyBVbmNoZWNrZWRMb2NrCmRpZmYgLS1naXQg YS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvanNjLmNwcCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9q c2MuY3BwCmluZGV4IGRiY2IzNTQ5YzQ0ZjBhYmVlZjg0Y2ZjYjhlNDAyMjgyZGNiOGU5MDEuLjdh YmI1MjQ0MmNjMWFjYmE5ZDMzMjBlYzE4YmQ4YzY1ODdmYTQwMWEgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9KYXZhU2NyaXB0Q29yZS9qc2MuY3BwCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9qc2Mu Y3BwCkBAIC0xNDc5LDggKzE0NzksMTkgQEAgSlNDX0RFRklORV9IT1NUX0ZVTkNUSU9OKGZ1bmN0 aW9uQWRkcmVzc09mLCAoSlNHbG9iYWxPYmplY3QqLCBDYWxsRnJhbWUqIGNhbGxGcmEKICAgICBK U1ZhbHVlIHZhbHVlID0gY2FsbEZyYW1lLT5hcmd1bWVudCgwKTsKICAgICBpZiAoIXZhbHVlLmlz Q2VsbCgpKQogICAgICAgICByZXR1cm4gSlNWYWx1ZTo6ZW5jb2RlKGpzVW5kZWZpbmVkKCkpOwor CisgICAgYXV0byBjb25zdCBhc0NlbGwgPSB2YWx1ZS5hc0NlbGwoKTsKKyNpZiBVU0UoSlNWQUxV RTMyXzY0KQorICAgIC8vIGluIDMyIGJpdHMsIGFzQ2VsbCgpIHJldHVybnMgYSBwb2ludGVyIHRv IHRoZSBwYXlsb2FkIGFuZCB1c2luZyByZWludGVycHJldF9jYXN0CisgICAgLy8gd2lsbCBzaWdu IGV4dGVuZCBhc051bWJlciB0byBGRkZGIEZGRkYgKyA8dmFsdWUtYWRkcmVzcz4sIHdoaWNoIHdp bGwgZmFpbCB0aGUKKyAgICAvLyBpc0ltcHVyZU5hTiBpbiBqc051bWJlci4gU28gZmlyc3QgY2Fz dCB0aGUgYWRkcmVzcyB0byB1aW50MzJfdCB0aGVuIHRvIHVpbnQ2NF90CisgICAgLy8gdG8gcHJl dmVudCB0aGUgc2lnbiBleHRlbnNpb24KKyAgICB1aW50NjRfdCBhc051bWJlciA9IHN0YXRpY19j YXN0PHVpbnQ2NF90PihzdGF0aWNfY2FzdDx1aW50MzJfdD4oYXNDZWxsKSk7CisjZWxzZQogICAg IC8vIE5lZWQgdG8gY2FzdCB0byB1aW50NjRfdCBzbyBiaXR3aXNlX2Nhc3Qgd2lsbCBwbGF5IGFs b25nLgotICAgIHVpbnQ2NF90IGFzTnVtYmVyID0gcmVpbnRlcnByZXRfY2FzdDx1aW50NjRfdD4o dmFsdWUuYXNDZWxsKCkpOworICAgIHVpbnQ2NF90IGFzTnVtYmVyID0gcmVpbnRlcnByZXRfY2Fz dDx1aW50NjRfdD4oYXNDZWxsKTsKKyNlbmRpZgorCiAgICAgRW5jb2RlZEpTVmFsdWUgcmV0dXJu VmFsdWUgPSBKU1ZhbHVlOjplbmNvZGUoanNOdW1iZXIoYml0d2lzZV9jYXN0PGRvdWJsZT4oYXNO dW1iZXIpKSk7CiAgICAgcmV0dXJuIHJldHVyblZhbHVlOwogfQo= 430274 2021-06-01 10:09:06 -0700 2021-06-01 16:01:40 -0700 Patch bug-226503-20210601130904.patch text/plain 2337 mikhail U3VidmVyc2lvbiBSZXZpc2lvbjogMjc4Mjc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBm Mzc3OTI5MDRlNmFkYmExZTIwZGFkZDVjNDBiZTIyZjU3YzA4YzEwLi41MjhiMjkyM2E5YmQ0YzEz ZWRhZTFiMjlhNTVkYjdlZjc1MDkyMzYzIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMiBAQAorMjAyMS0wNi0wMSAgTWlraGFpbCBSLiBHYWRlbGhhICA8bWlraGFpbEBpZ2Fs aWEuY29tPgorCisgICAgICAgIEZpeCBpbmFkdmVydGVudCB0YWcgY29ycnVwdGlvbiBpbiBmdW5j dGlvbkFkZHJlc3NPZgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MjI2NTAzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgT3JpZ2luYWwgcGF0Y2ggYnkgQW5nZWxvcyBPaWtvbm9tb3BvdWxvcy4KKworICAgICAg ICBUaGUgY2FzdCB3YXMgc2lnbi1leHRlbmRpbmcgdGhlIEpTVmFsdWUgcGF5bG9hZCwgc28gdGhh dCBhZGRyZXNzZXMKKyAgICAgICAgdGhhdCBoYWQgdGhlIG1vc3Qgc2lnbmlmaWNhbnQgc2V0IGdh dmUgdXMgYSBzaWduLWV4dGVuZGVkIHJlc3VsdCBpbgorICAgICAgICBhc051bWJlciB3aGljaCB3 YXMgdGhlbiBjb252ZXJ0ZWQgdG8gYW4gaW52YWxpZCBOYU4gYnkgdGhlIGJpdGNhc3QuCisgICAg CisgICAgICAgIEluc3RlYWQsIGNhc3QgdGhlIHBheWxvYWQgdG8gdWludDMyX3QsIHNvIHRoYXQg dGhlIGNhc3QgdG8gdWludDY0X3QKKyAgICAgICAgd2lsbCBub3Qgc2lnbi1leHRlbmQuCisgICAg ICAgIAorICAgICAgICAqIGpzYy5jcHA6CisgICAgICAgIChKU0NfREVGSU5FX0hPU1RfRlVOQ1RJ T04pOgorCiAyMDIxLTA1LTMwICBDaHJpcyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CiAKICAg ICAgICAgRHJvcCBVbmNoZWNrZWRDb25kaXRpb24gLyBVbmNoZWNrZWRMb2NrCmRpZmYgLS1naXQg YS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvanNjLmNwcCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9q c2MuY3BwCmluZGV4IGRiY2IzNTQ5YzQ0ZjBhYmVlZjg0Y2ZjYjhlNDAyMjgyZGNiOGU5MDEuLmY3 OWNjNTMyYWE3YTVmOWU5MWY3MTgwNjQ5YThhMDRlYzNjMjNlMGEgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9KYXZhU2NyaXB0Q29yZS9qc2MuY3BwCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9qc2Mu Y3BwCkBAIC0xNDc5LDggKzE0NzksMTkgQEAgSlNDX0RFRklORV9IT1NUX0ZVTkNUSU9OKGZ1bmN0 aW9uQWRkcmVzc09mLCAoSlNHbG9iYWxPYmplY3QqLCBDYWxsRnJhbWUqIGNhbGxGcmEKICAgICBK U1ZhbHVlIHZhbHVlID0gY2FsbEZyYW1lLT5hcmd1bWVudCgwKTsKICAgICBpZiAoIXZhbHVlLmlz Q2VsbCgpKQogICAgICAgICByZXR1cm4gSlNWYWx1ZTo6ZW5jb2RlKGpzVW5kZWZpbmVkKCkpOwor CisjaWYgVVNFKEpTVkFMVUUzMl82NCkKKyAgICAvLyBJbiAzMiBiaXRzLCBhc0NlbGwoKSByZXR1 cm5zIGEgcG9pbnRlciB0byB0aGUgcGF5bG9hZCBhbmQgdXNpbmcgcmVpbnRlcnByZXRfY2FzdAor ICAgIC8vIHdpbGwgc2lnbiBleHRlbmQgYXNOdW1iZXIgdG8gRkZGRiBGRkZGICsgPHZhbHVlLWFk ZHJlc3M+LCB3aGljaCB3aWxsIGZhaWwgdGhlCisgICAgLy8gaXNJbXB1cmVOYU4gY2hlY2sgaW4g anNOdW1iZXIuIFRvIHByZXZlbnQgdGhhdCwgZmlyc3QgcmVpbnRlcnByZXRfY2FzdCB0aGUgYWRk cmVzcyAKKyAgICAvLyB0byB1aW50MzJfdCB0aGVuIGNhc3QgaXQgdG8gdWludDY0X3QuIEFkZHJl c3NPZiBpbiAzMiBiaXRzIHdpbGwgcmV0dXJuIHRoZSBzYW1lIAorICAgIC8vIGFkZHJlc3MgcmV0 dXJuZWQgYnkgZGVzY3JpYmUoKSBidXQgaW4gZG91YmxlCisgICAgdWludDY0X3QgYXNOdW1iZXIg PSBzdGF0aWNfY2FzdDx1aW50NjRfdD4ocmVpbnRlcnByZXRfY2FzdDx1aW50MzJfdD4odmFsdWUu YXNDZWxsKCkpKTsKKyNlbHNlCiAgICAgLy8gTmVlZCB0byBjYXN0IHRvIHVpbnQ2NF90IHNvIGJp dHdpc2VfY2FzdCB3aWxsIHBsYXkgYWxvbmcuCiAgICAgdWludDY0X3QgYXNOdW1iZXIgPSByZWlu dGVycHJldF9jYXN0PHVpbnQ2NF90Pih2YWx1ZS5hc0NlbGwoKSk7CisjZW5kaWYKKwogICAgIEVu Y29kZWRKU1ZhbHVlIHJldHVyblZhbHVlID0gSlNWYWx1ZTo6ZW5jb2RlKGpzTnVtYmVyKGJpdHdp c2VfY2FzdDxkb3VibGU+KGFzTnVtYmVyKSkpOwogICAgIHJldHVybiByZXR1cm5WYWx1ZTsKIH0K 430302 2021-06-01 16:01:48 -0700 2021-06-09 08:21:46 -0700 Patch bug-226503-20210601190147.patch text/plain 1841 mikhail U3VidmVyc2lvbiBSZXZpc2lvbjogMjc4Mjc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBm Mzc3OTI5MDRlNmFkYmExZTIwZGFkZDVjNDBiZTIyZjU3YzA4YzEwLi5lMzE1NDJlNzQzNGFmNDU1 ODdmZWJjM2NhN2FlZjIxNGJhMGY2NzFiIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMiBAQAorMjAyMS0wNi0wMSAgTWlraGFpbCBSLiBHYWRlbGhhICA8bWlraGFpbEBpZ2Fs aWEuY29tPgorCisgICAgICAgIEZpeCBpbmFkdmVydGVudCB0YWcgY29ycnVwdGlvbiBpbiBmdW5j dGlvbkFkZHJlc3NPZgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MjI2NTAzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgRGFyaW4gQWRsZXIuCisKKyAgICAg ICAgT3JpZ2luYWwgcGF0Y2ggYnkgQW5nZWxvcyBPaWtvbm9tb3BvdWxvcy4KKworICAgICAgICBU aGUgY2FzdCB3YXMgc2lnbi1leHRlbmRpbmcgdGhlIEpTVmFsdWUgYWRkcmVzcyBpbiAzMiBiaXRz LCBzbyB0aGF0IGFkZHJlc3NlcworICAgICAgICB0aGF0IGhhZCB0aGUgbW9zdCBzaWduaWZpY2Fu dCBzZXQgZ2F2ZSB1cyBhIHNpZ24tZXh0ZW5kZWQgcmVzdWx0IGluCisgICAgICAgIGFzTnVtYmVy IHdoaWNoIHdhcyB0aGVuIGNvbnZlcnRlZCB0byBhbiBpbnZhbGlkIE5hTiBieSB0aGUgYml0Y2Fz dC4KKworICAgICAgICBJbnN0ZWFkLCBjYXN0IHRoZSBhZGRyZXNzIHRvIHVpbnRwdHJfdCwgYW5k IHRoZSByZXN1bHQgd2lsbCBiZSBwcm9tb3RlZAorICAgICAgICB1aW50NjRfdCB3aXRob3V0IHNp Z24tZXh0ZW5kaW5nIHRoZSBhZGRyZXNzLgorCisgICAgICAgICoganNjLmNwcDoKKyAgICAgICAg KEpTQ19ERUZJTkVfSE9TVF9GVU5DVElPTik6CisKIDIwMjEtMDUtMzAgIENocmlzIER1bWV6ICA8 Y2R1bWV6QGFwcGxlLmNvbT4KIAogICAgICAgICBEcm9wIFVuY2hlY2tlZENvbmRpdGlvbiAvIFVu Y2hlY2tlZExvY2sKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9qc2MuY3BwIGIv U291cmNlL0phdmFTY3JpcHRDb3JlL2pzYy5jcHAKaW5kZXggZGJjYjM1NDljNDRmMGFiZWVmODRj ZmNiOGU0MDIyODJkY2I4ZTkwMS4uYjFkYjMwZWNjY2JiMjI2YWZhYWEzMGM0NWI2M2E4NTBlOGVi ZWZkOSAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2pzYy5jcHAKKysrIGIvU291 cmNlL0phdmFTY3JpcHRDb3JlL2pzYy5jcHAKQEAgLTE0ODAsNyArMTQ4MCw3IEBAIEpTQ19ERUZJ TkVfSE9TVF9GVU5DVElPTihmdW5jdGlvbkFkZHJlc3NPZiwgKEpTR2xvYmFsT2JqZWN0KiwgQ2Fs bEZyYW1lKiBjYWxsRnJhCiAgICAgaWYgKCF2YWx1ZS5pc0NlbGwoKSkKICAgICAgICAgcmV0dXJu IEpTVmFsdWU6OmVuY29kZShqc1VuZGVmaW5lZCgpKTsKICAgICAvLyBOZWVkIHRvIGNhc3QgdG8g dWludDY0X3Qgc28gYml0d2lzZV9jYXN0IHdpbGwgcGxheSBhbG9uZy4KLSAgICB1aW50NjRfdCBh c051bWJlciA9IHJlaW50ZXJwcmV0X2Nhc3Q8dWludDY0X3Q+KHZhbHVlLmFzQ2VsbCgpKTsKKyAg ICB1aW50NjRfdCBhc051bWJlciA9IHJlaW50ZXJwcmV0X2Nhc3Q8dWludHB0cl90Pih2YWx1ZS5h c0NlbGwoKSk7CiAgICAgRW5jb2RlZEpTVmFsdWUgcmV0dXJuVmFsdWUgPSBKU1ZhbHVlOjplbmNv ZGUoanNOdW1iZXIoYml0d2lzZV9jYXN0PGRvdWJsZT4oYXNOdW1iZXIpKSk7CiAgICAgcmV0dXJu IHJldHVyblZhbHVlOwogfQo=