235275 2022-01-15 15:39:18 -0800 Do not use pas utils outside of libpas 2022-01-19 13:49:31 -0800 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 230841 1 ysuzuki ysuzuki benjamin cdumez cmarcelo darin ews-watchlist webkit-bug-importer oldest_to_newest 1831274 0 ysuzuki 2022-01-15 15:39:18 -0800 Do not use pas utils outside of libpas 1831275 1 449276 ysuzuki 2022-01-15 15:40:46 -0800 Created attachment 449276 Patch 1831296 2 449276 darin 2022-01-16 07:48:52 -0800 Comment on attachment 449276 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449276&action=review > Source/WebCore/ChangeLog:8 > + We should not use any utility functions from libpas outside of bmalloc. Can we change how the headers are structured so the compile will fail too? That way the rule would “guard itself” and we would not need vigilant programmers thinking about this rule. Typically in separate frameworks the technique would be “mark header internal”; headers would not be installed. Is there a similar technique at least for the bmalloc/WebCore boundary if not bmalloc/WTF? > Source/WTF/wtf/MathExtras.h:773 > +inline uint64_t reverseBits64(uint64_t value) Why add this unused and untested function? > Source/WTF/wtf/MathExtras.h:782 > + return ((uint64_t)reverseBits32((unsigned)value) << (uint64_t)32) I suggest uint32_t here instead of unsigned > Source/WTF/wtf/MathExtras.h:783 > + | (uint64_t)reverseBits32((unsigned)(value >> (uint64_t)32)); Ditto 1832201 3 449276 ysuzuki 2022-01-19 13:28:58 -0800 Comment on attachment 449276 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449276&action=review >> Source/WebCore/ChangeLog:8 >> + We should not use any utility functions from libpas outside of bmalloc. > > Can we change how the headers are structured so the compile will fail too? That way the rule would “guard itself” and we would not need vigilant programmers thinking about this rule. Typically in separate frameworks the technique would be “mark header internal”; headers would not be installed. Is there a similar technique at least for the bmalloc/WebCore boundary if not bmalloc/WTF? Unfortunately, pas_utils.h is basis of libpas, and there are some legit use of pas_utils.h outside of libpas (JSC's jit_heap for example). So we cannot prevent the outer projects from including it for now. >> Source/WTF/wtf/MathExtras.h:773 >> +inline uint64_t reverseBits64(uint64_t value) > > Why add this unused and untested function? I copied these set of functions from libpas to prevent including libpas directly further, but for now, it is not used. For the patch's simplicity, I'll just remove this. 1832208 4 ysuzuki 2022-01-19 13:35:13 -0800 Committed r288239 (246193@trunk): <https://commits.webkit.org/246193@trunk> 1832209 5 webkit-bug-importer 2022-01-19 13:36:17 -0800 <rdar://problem/87788695> 1832221 6 449276 ysuzuki 2022-01-19 13:49:31 -0800 Comment on attachment 449276 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449276&action=review >>> Source/WebCore/ChangeLog:8 >>> + We should not use any utility functions from libpas outside of bmalloc. >> >> Can we change how the headers are structured so the compile will fail too? That way the rule would “guard itself” and we would not need vigilant programmers thinking about this rule. Typically in separate frameworks the technique would be “mark header internal”; headers would not be installed. Is there a similar technique at least for the bmalloc/WebCore boundary if not bmalloc/WTF? > > Unfortunately, pas_utils.h is basis of libpas, and there are some legit use of pas_utils.h outside of libpas (JSC's jit_heap for example). > So we cannot prevent the outer projects from including it for now. It seems that, if we do that, it will break 32bit build. https://bugs.webkit.org/show_bug.cgi?id=235372 449276 2022-01-15 15:40:46 -0800 2022-01-16 07:48:52 -0800 Patch bug-235275-20220115154045.patch text/plain 5945 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjg4MDY4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDYxNGUzMjY5OGYzZTJjMGVmY2ZhMmI0 NjJjZjQwYzI0NDNhZGNjNWMuLjRjODJlOGUzMDBjZTc1ZmMzYjk3M2QzOTY5MzJmZmFkYjFhOWRk NDkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMjItMDEtMTUgIFl1c3VrZSBTdXp1a2kgIDx5c3V6 dWtpQGFwcGxlLmNvbT4KKworICAgICAgICBEbyBub3QgdXNlIHBhcyB1dGlscyBvdXRzaWRlIG9m IGxpYnBhcworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MjM1Mjc1CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg KiB3dGYvTWF0aEV4dHJhcy5oOgorICAgICAgICAoV1RGOjpyZXZlcnNlQml0czMyKToKKyAgICAg ICAgKFdURjo6cmV2ZXJzZUJpdHM2NCk6CisKIDIwMjItMDEtMTQgIFl1c3VrZSBTdXp1a2kgIDx5 c3V6dWtpQGFwcGxlLmNvbT4KIAogICAgICAgICBGaXggV1RGOjpVVUlEJ3MgcG90ZW50aWFsIGNv bGxpc2lvbiB3aXRoIGVtcHR5IGFuZCBkZWxldGVkIHZhbHVlcwpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkNvcmUvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCmluZGV4IGU3NmYz NzljMzExMjdkNzQ3MjAwYjc5MDVjNWZhYWE0NWUzNWEwYWIuLjAxM2M1NGY4MzQ1MTBlMjk2ZTBk NmQwZmUyNGY2YmIzNmQxNzFkZGUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxv ZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyMSBAQAorMjAyMi0w MS0xNSAgWXVzdWtlIFN1enVraSAgPHlzdXp1a2lAYXBwbGUuY29tPgorCisgICAgICAgIERvIG5v dCB1c2UgcGFzIHV0aWxzIG91dHNpZGUgb2YgbGlicGFzCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMzUyNzUKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXZSBzaG91bGQgbm90IHVzZSBhbnkgdXRpbGl0eSBm dW5jdGlvbnMgZnJvbSBsaWJwYXMgb3V0c2lkZSBvZiBibWFsbG9jLgorICAgICAgICBsaWJwYXMg aXMgZGVzaWduZWQgdG8gYmUgc2VsZi1jb250YWluZWQgYW5kIHVzZWQgb3V0c2lkZSBvZiBXZWJL aXQuCisgICAgICAgIFdlIGNhbm5vdCByZWx5IG9uIG5vbiBQQVNfQVBJIGZ1bmN0aW9ucy4KKwor ICAgICAgICBJZiB3ZSBuZWVkIHRoZXNlIHV0aWxpdGllcywgd2Ugc2hvdWxkIGRlZmluZSBpdCBp biBXVEYuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9IRVZDVXRpbGl0aWVzLmNwcDoK KyAgICAgICAgKFdlYkNvcmU6OnBhcnNlSEVWQ0NvZGVjUGFyYW1ldGVycyk6CisgICAgICAgIChX ZWJDb3JlOjpjcmVhdGVIRVZDQ29kZWNQYXJhbWV0ZXJzU3RyaW5nKToKKyAgICAgICAgKFdlYkNv cmU6OnJldmVyc2VCaXRzKTogRGVsZXRlZC4KKwogMjAyMi0wMS0xNSAgRGFyaW4gQWRsZXIgIDxk YXJpbkBhcHBsZS5jb20+CiAKICAgICAgICAgRmlyc3Qgc3RlcCB0byBmaXggaGFja2VkIGlzUmVw bGFjZWQ6IFJlbmFtZSBpc1JlcGxhY2VkIGZ1bmN0aW9ucyB0byBpc1JlcGxhY2VkT3JJbmxpbmVC bG9jawpkaWZmIC0tZ2l0IGEvU291cmNlL1dURi93dGYvTWF0aEV4dHJhcy5oIGIvU291cmNlL1dU Ri93dGYvTWF0aEV4dHJhcy5oCmluZGV4IDA4ZDRlZWRlM2NlNDIyMmI0ZWI4OTFiMTliODU3ZWVm ZmNjNDFmYWMuLjFjYzMzODM2Njg3ZTIzYmYyMzcwYzBhMGE2YzYzYzliYTEwZDllOWUgMTAwNjQ0 Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL01hdGhFeHRyYXMuaAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9N YXRoRXh0cmFzLmgKQEAgLTc1Myw2ICs3NTMsMzcgQEAgaW5saW5lIHNpemVfdCBjb3VudFRyYWls aW5nWmVyb3ModWludDY0X3QgdikKICAgICByZXR1cm4gTW9kNjdQb3NpdGlvblsoKDEgKyB+dikg JiB2KSAlIDY3XTsKIH0KIAoraW5saW5lIHVpbnQzMl90IHJldmVyc2VCaXRzMzIodWludDMyX3Qg dmFsdWUpCit7CisjaWYgQ09NUElMRVIoR0NDX0NPTVBBVElCTEUpICYmIENQVShBUk02NCkKKyAg ICB1aW50MzJfdCByZXN1bHQ7CisgICAgYXNtICgicmJpdCAldzAsICV3MSIKKyAgICAgICAgOiAi PXIiKHJlc3VsdCkKKyAgICAgICAgOiAiciIodmFsdWUpKTsKKyAgICByZXR1cm4gcmVzdWx0Owor I2Vsc2UKKyAgICB2YWx1ZSA9ICgodmFsdWUgJiAweGFhYWFhYWFhKSA+PiAxKSB8ICgodmFsdWUg JiAweDU1NTU1NTU1KSA8PCAxKTsKKyAgICB2YWx1ZSA9ICgodmFsdWUgJiAweGNjY2NjY2NjKSA+ PiAyKSB8ICgodmFsdWUgJiAweDMzMzMzMzMzKSA8PCAyKTsKKyAgICB2YWx1ZSA9ICgodmFsdWUg JiAweGYwZjBmMGYwKSA+PiA0KSB8ICgodmFsdWUgJiAweDBmMGYwZjBmKSA8PCA0KTsKKyAgICB2 YWx1ZSA9ICgodmFsdWUgJiAweGZmMDBmZjAwKSA+PiA4KSB8ICgodmFsdWUgJiAweDAwZmYwMGZm KSA8PCA4KTsKKyAgICByZXR1cm4gKHZhbHVlID4+IDE2KSB8ICh2YWx1ZSA8PCAxNik7CisjZW5k aWYKK30KKworaW5saW5lIHVpbnQ2NF90IHJldmVyc2VCaXRzNjQodWludDY0X3QgdmFsdWUpCit7 CisjaWYgQ09NUElMRVIoR0NDX0NPTVBBVElCTEUpICYmIENQVShBUk02NCkKKyAgICB1aW50NjRf dCByZXN1bHQ7CisgICAgYXNtICgicmJpdCAlMCwgJTEiCisgICAgICAgIDogIj1yIihyZXN1bHQp CisgICAgICAgIDogInIiKHZhbHVlKSk7CisgICAgcmV0dXJuIHJlc3VsdDsKKyNlbHNlCisgICAg cmV0dXJuICgodWludDY0X3QpcmV2ZXJzZUJpdHMzMigodW5zaWduZWQpdmFsdWUpIDw8ICh1aW50 NjRfdCkzMikKKyAgICAgICAgfCAodWludDY0X3QpcmV2ZXJzZUJpdHMzMigodW5zaWduZWQpKHZh bHVlID4+ICh1aW50NjRfdCkzMikpOworI2VuZGlmCit9CisKIH0gLy8gbmFtZXNwYWNlIFdURgog CiB1c2luZyBXVEY6OnNodWZmbGVWZWN0b3I7CkBAIC03NjAsMyArNzkxLDUgQEAgdXNpbmcgV1RG OjpjbHo7CiB1c2luZyBXVEY6OmN0ejsKIHVzaW5nIFdURjo6Z2V0TFNCU2V0OwogdXNpbmcgV1RG OjpnZXRNU0JTZXQ7Cit1c2luZyBXVEY6OnJldmVyc2VCaXRzMzI7Cit1c2luZyBXVEY6OnJldmVy c2VCaXRzNjQ7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9I RVZDVXRpbGl0aWVzLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0hFVkNV dGlsaXRpZXMuY3BwCmluZGV4IGE5MGQ0MWZmNDIzMjc5NjNhNDdlNWM0M2U3NWYzM2NhZDk0YzQ5 YmUuLjk4YmUzYzYzN2ZlZDQ4MDhkOTVkOTNlOGJmY2ZkNTQzODVkN2M1ODEgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0hFVkNVdGlsaXRpZXMuY3BwCisrKyBi L1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0hFVkNVdGlsaXRpZXMuY3BwCkBAIC0z MCwzMCArMzAsMTMgQEAKICNpbmNsdWRlICJTaGFyZWRCdWZmZXIuaCIKICNpbmNsdWRlIDxKYXZh U2NyaXB0Q29yZS9EYXRhVmlldy5oPgogI2luY2x1ZGUgPHd0Zi9IZXhOdW1iZXIuaD4KKyNpbmNs dWRlIDx3dGYvTWF0aEV4dHJhcy5oPgogI2luY2x1ZGUgPHd0Zi9OZXZlckRlc3Ryb3llZC5oPgog I2luY2x1ZGUgPHd0Zi9Tb3J0ZWRBcnJheU1hcC5oPgogI2luY2x1ZGUgPHd0Zi90ZXh0L1N0cmlu Z1RvSW50ZWdlckNvbnZlcnNpb24uaD4KIAotI2lmIF9faGFzX2luY2x1ZGUoPGJtYWxsb2MvcGFz X3V0aWxzLmg+KQotI2luY2x1ZGUgPGJtYWxsb2MvcGFzX3V0aWxzLmg+Ci0jZW5kaWYKLQogbmFt ZXNwYWNlIFdlYkNvcmUgewogCi1zdGF0aWMgaW5saW5lIHVpbnQzMl90IHJldmVyc2VCaXRzKHVp bnQzMl90IHZhbHVlKQotewotI2lmIF9faGFzX2luY2x1ZGUoPGJtYWxsb2MvcGFzX3V0aWxzLmg+ KQotICAgIHJldHVybiBwYXNfcmV2ZXJzZSh2YWx1ZSk7Ci0jZWxzZQotICAgIC8vIEZyb20gcGFz X3JldmVyc2UoKToKLSAgICB2YWx1ZSA9ICgodmFsdWUgJiAweGFhYWFhYWFhKSA+PiAxKSB8ICgo dmFsdWUgJiAweDU1NTU1NTU1KSA8PCAxKTsKLSAgICB2YWx1ZSA9ICgodmFsdWUgJiAweGNjY2Nj Y2NjKSA+PiAyKSB8ICgodmFsdWUgJiAweDMzMzMzMzMzKSA8PCAyKTsKLSAgICB2YWx1ZSA9ICgo dmFsdWUgJiAweGYwZjBmMGYwKSA+PiA0KSB8ICgodmFsdWUgJiAweDBmMGYwZjBmKSA8PCA0KTsK LSAgICB2YWx1ZSA9ICgodmFsdWUgJiAweGZmMDBmZjAwKSA+PiA4KSB8ICgodmFsdWUgJiAweDAw ZmYwMGZmKSA8PCA4KTsKLSAgICByZXR1cm4gKHZhbHVlID4+IDE2KSB8ICh2YWx1ZSA8PCAxNik7 Ci0jZW5kaWYKLX0KLQogc3RkOjpvcHRpb25hbDxBVkNQYXJhbWV0ZXJzPiBwYXJzZUFWQ0NvZGVj UGFyYW1ldGVycyhTdHJpbmdWaWV3IGNvZGVjU3RyaW5nKQogewogICAgIC8vIFRoZSBmb3JtYXQg b2YgdGhlICdhdmMxJyBjb2RlYyBzdHJpbmcgaXMgc3BlY2lmaWVkIGluIElTTy9JRUMgMTQ0OTYt MTU6MjAxNCwgQW5uZXggRTIuCkBAIC0xODUsNyArMTY4LDcgQEAgc3RkOjpvcHRpb25hbDxIRVZD UGFyYW1ldGVycz4gcGFyc2VIRVZDQ29kZWNQYXJhbWV0ZXJzKFN0cmluZ1ZpZXcgY29kZWNTdHJp bmcpCiAgICAgYXV0byBjb21wYXRpYmlsaXR5RmxhZ3MgPSBwYXJzZUludGVnZXI8dWludDMyX3Q+ KCpuZXh0RWxlbWVudCwgMTYpOwogICAgIGlmICghY29tcGF0aWJpbGl0eUZsYWdzKQogICAgICAg ICByZXR1cm4gc3RkOjpudWxsb3B0OwotICAgIHBhcmFtZXRlcnMuZ2VuZXJhbFByb2ZpbGVDb21w YXRpYmlsaXR5RmxhZ3MgPSByZXZlcnNlQml0cygqY29tcGF0aWJpbGl0eUZsYWdzKTsKKyAgICBw YXJhbWV0ZXJzLmdlbmVyYWxQcm9maWxlQ29tcGF0aWJpbGl0eUZsYWdzID0gcmV2ZXJzZUJpdHMz MigqY29tcGF0aWJpbGl0eUZsYWdzKTsKIAogICAgIGlmICgrK25leHRFbGVtZW50ID09IGNvZGVj U3BsaXQuZW5kKCkpCiAgICAgICAgIHJldHVybiBzdGQ6Om51bGxvcHQ7CkBAIC0yMzEsNyArMjE0 LDcgQEAgU3RyaW5nIGNyZWF0ZUhFVkNDb2RlY1BhcmFtZXRlcnNTdHJpbmcoY29uc3QgSEVWQ1Bh cmFtZXRlcnMmIHBhcmFtZXRlcnMpCiAgICAgLy8gZ2VuZXJhbF9wcm9maWxlX2NvbXBhdGliaWxp dHlfZmxhZ1sgMzEgXSBhcyB0aGUgbW9zdCBzaWduaWZpY2FudCBiaXQsIGZvbGxvd2VkIGJ5LCBn ZW5lcmFsX3Byb2ZpbGVfY29tcGF0aWJpbGl0eV9mbGFnWyAzMCBdLAogICAgIC8vIGFuZCBkb3du IHRvIGdlbmVyYWxfcHJvZmlsZV9jb21wYXRpYmlsaXR5X2ZsYWdbIDAgXSBhcyB0aGUgbGVhc3Qg c2lnbmlmaWNhbnQgYml0LCB3aGVyZSBnZW5lcmFsX3Byb2ZpbGVfY29tcGF0aWJpbGl0eV9mbGFn WyBpIF0KICAgICAvLyBmb3IgaSBpbiB0aGUgcmFuZ2Ugb2YgMCB0byAzMSwgaW5jbHVzaXZlLCBh cmUgc3BlY2lmaWVkIGluIElTTy9JRUMgMjMwMDjigJAyLCBlbmNvZGVkIGluIGhleGFkZWNpbWFs IChsZWFkaW5nIHplcm9lcyBtYXkgYmUgb21pdHRlZCkKLSAgICBhdXRvIGNvbXBhdEZsYWdQYXJh bWV0ZXIgPSBoZXgocmV2ZXJzZUJpdHMocGFyYW1ldGVycy5nZW5lcmFsUHJvZmlsZUNvbXBhdGli aWxpdHlGbGFncykpOworICAgIGF1dG8gY29tcGF0RmxhZ1BhcmFtZXRlciA9IGhleChyZXZlcnNl Qml0czMyKHBhcmFtZXRlcnMuZ2VuZXJhbFByb2ZpbGVDb21wYXRpYmlsaXR5RmxhZ3MpKTsKIAog ICAgIC8vICogZWFjaCBvZiB0aGUgNiBieXRlcyBvZiB0aGUgY29uc3RyYWludCBmbGFncywgc3Rh cnRpbmcgZnJvbSB0aGUgYnl0ZSBjb250YWluaW5nIHRoZQogICAgIC8vIGdlbmVyYWxfcHJvZ3Jl c3NpdmVfc291cmNlX2ZsYWcsIGVhY2ggZW5jb2RlZCBhcyBhIGhleGFkZWNpbWFsIG51bWJlciwg YW5kIHRoZSBlbmNvZGluZwo=