200960 2019-08-20 18:19:34 -0700 Make it easier to pass pointers to WTFCrashWithInfo. 2019-08-20 18:38:16 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mark.lam mark.lam benjamin cdumez cmarcelo dbates ews-watchlist saam webkit-bug-importer ysuzuki oldest_to_newest 1563353 0 mark.lam 2019-08-20 18:19:34 -0700 Now, we don't have to explicitly cast them to uint64_ts first. The template wrappers will take care of it for us. 1563354 1 376837 mark.lam 2019-08-20 18:23:12 -0700 Created attachment 376837 proposed patch. 1563355 2 376837 saam 2019-08-20 18:26:58 -0700 Comment on attachment 376837 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=376837&action=review > Source/WTF/wtf/Assertions.h:625 > + static_assert(std::is_integral<T>::value || std::is_enum<T>::value || std::is_pointer<T>::value, "All types need to be bitwise_cast-able to integral type for logging"); seems like a bad name for a function that allows pointers. 1563356 3 376837 ysuzuki 2019-08-20 18:27:43 -0700 Comment on attachment 376837 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=376837&action=review r=me with comment too. > Source/WTF/wtf/Assertions.h:553 > +ALWAYS_INLINE uint64_t wtfCrashArg(T* arg) { return reinterpret_cast<uint64_t>(arg); } In ARM64_32, the pointer size is 32bit so we should cast it to 64bit by using static_cast. So, I suggest using `static_cast<uintptr_t>(arg)` here. 1563357 4 376837 ysuzuki 2019-08-20 18:29:39 -0700 Comment on attachment 376837 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=376837&action=review >> Source/WTF/wtf/Assertions.h:553 >> +ALWAYS_INLINE uint64_t wtfCrashArg(T* arg) { return reinterpret_cast<uint64_t>(arg); } > > In ARM64_32, the pointer size is 32bit so we should cast it to 64bit by using static_cast. > So, I suggest using `static_cast<uintptr_t>(arg)` here. Ah, no. reinterpret_cast<uintptr_t>(arg) is the way. 1563358 5 376837 mark.lam 2019-08-20 18:32:05 -0700 Comment on attachment 376837 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=376837&action=review >>> Source/WTF/wtf/Assertions.h:553 >>> +ALWAYS_INLINE uint64_t wtfCrashArg(T* arg) { return reinterpret_cast<uint64_t>(arg); } >> >> In ARM64_32, the pointer size is 32bit so we should cast it to 64bit by using static_cast. >> So, I suggest using `static_cast<uintptr_t>(arg)` here. > > Ah, no. reinterpret_cast<uintptr_t>(arg) is the way. Fixed. >> Source/WTF/wtf/Assertions.h:625 >> + static_assert(std::is_integral<T>::value || std::is_enum<T>::value || std::is_pointer<T>::value, "All types need to be bitwise_cast-able to integral type for logging"); > > seems like a bad name for a function that allows pointers. I'll rename it to isIntegralOrPointerType. 1563359 6 mark.lam 2019-08-20 18:37:25 -0700 Thanks for the reviews. Landed in r248930: <http://trac.webkit.org/r248930>. 1563361 7 webkit-bug-importer 2019-08-20 18:38:16 -0700 <rdar://problem/54538595> 376837 2019-08-20 18:23:12 -0700 2019-08-20 18:26:58 -0700 proposed patch. bug-200960.patch text/plain 5128 mark.lam SW5kZXg6IFNvdXJjZS9XVEYvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XVEYvQ2hh bmdlTG9nCShyZXZpc2lvbiAyNDg5MjkpCisrKyBTb3VyY2UvV1RGL0NoYW5nZUxvZwkod29ya2lu ZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDE5LTA4LTIwICBNYXJrIExhbSAgPG1hcmsubGFt QGFwcGxlLmNvbT4KKworICAgICAgICBNYWtlIGl0IGVhc2llciB0byBwYXNzIHBvaW50ZXJzIHRv IFdURkNyYXNoV2l0aEluZm8uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0yMDA5NjAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBOb3csIHdlIGRvbid0IGhhdmUgdG8gZXhwbGljaXRseSBjYXN0IHRoZW0gdG8g dWludDY0X3RzIGZpcnN0LiAgVGhlIHRlbXBsYXRlCisgICAgICAgIHdyYXBwZXJzIHdpbGwgdGFr ZSBjYXJlIG9mIGl0IGZvciB1cy4KKworICAgICAgICAqIHd0Zi9Bc3NlcnRpb25zLmg6CisgICAg ICAgICh3dGZDcmFzaEFyZyk6CisgICAgICAgIChXVEZDcmFzaFdpdGhJbmZvKToKKyAgICAgICAg KFdURjo6aXNJbnRlZ3JhbFR5cGUpOgorCiAyMDE5LTA4LTIwICBTYWFtIEJhcmF0aSAgPHNiYXJh dGlAYXBwbGUuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQuIEZvbGxvd3VwIHRvIHIyNDg5MDMu IEl0J3Mgbm90IHZhbGlkIHRvIHJlbW92ZQpJbmRleDogU291cmNlL1dURi93dGYvQXNzZXJ0aW9u cy5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XVEYvd3RmL0Fzc2VydGlvbnMuaAkocmV2aXNpb24g MjQ4OTI4KQorKysgU291cmNlL1dURi93dGYvQXNzZXJ0aW9ucy5oCSh3b3JraW5nIGNvcHkpCkBA IC0xLDUgKzEsNSBAQAogLyoKLSAqIENvcHlyaWdodCAoQykgMjAwMy0yMDE3IEFwcGxlIEluYy4g IEFsbCByaWdodHMgcmVzZXJ2ZWQuCisgKiBDb3B5cmlnaHQgKEMpIDIwMDMtMjAxOSBBcHBsZSBJ bmMuICBBbGwgcmlnaHRzIHJlc2VydmVkLgogICoKICAqIFJlZGlzdHJpYnV0aW9uIGFuZCB1c2Ug aW4gc291cmNlIGFuZCBiaW5hcnkgZm9ybXMsIHdpdGggb3Igd2l0aG91dAogICogbW9kaWZpY2F0 aW9uLCBhcmUgcGVybWl0dGVkIHByb3ZpZGVkIHRoYXQgdGhlIGZvbGxvd2luZyBjb25kaXRpb25z CkBAIC01NDksNiArNTQ5LDU0IEBAIFdURl9FWFBPUlRfUFJJVkFURSBOT19SRVRVUk5fRFVFX1RP X0NSQVMKIAogI2lmZGVmIF9fY3BsdXNwbHVzCiAKK3RlbXBsYXRlPHR5cGVuYW1lIFQ+CitBTFdB WVNfSU5MSU5FIHVpbnQ2NF90IHd0ZkNyYXNoQXJnKFQqIGFyZykgeyByZXR1cm4gcmVpbnRlcnBy ZXRfY2FzdDx1aW50NjRfdD4oYXJnKTsgfQorCit0ZW1wbGF0ZTx0eXBlbmFtZSBUPgorQUxXQVlT X0lOTElORSB1aW50NjRfdCB3dGZDcmFzaEFyZyhUIGFyZykgeyByZXR1cm4gYXJnOyB9CisKK3Rl bXBsYXRlPHR5cGVuYW1lIFQ+CitOT19SRVRVUk5fRFVFX1RPX0NSQVNIIEFMV0FZU19JTkxJTkUg dm9pZCBXVEZDcmFzaFdpdGhJbmZvKGludCBsaW5lLCBjb25zdCBjaGFyKiBmaWxlLCBjb25zdCBj aGFyKiBmdW5jdGlvbiwgaW50IGNvdW50ZXIsIFQgcmVhc29uKQoreworICAgIFdURkNyYXNoV2l0 aEluZm8obGluZSwgZmlsZSwgZnVuY3Rpb24sIGNvdW50ZXIsIHd0ZkNyYXNoQXJnKHJlYXNvbikp OworfQorCit0ZW1wbGF0ZTx0eXBlbmFtZSBULCB0eXBlbmFtZSBVPgorTk9fUkVUVVJOX0RVRV9U T19DUkFTSCBBTFdBWVNfSU5MSU5FIHZvaWQgV1RGQ3Jhc2hXaXRoSW5mbyhpbnQgbGluZSwgY29u c3QgY2hhciogZmlsZSwgY29uc3QgY2hhciogZnVuY3Rpb24sIGludCBjb3VudGVyLCBUIHJlYXNv biwgVSBtaXNjMSkKK3sKKyAgICBXVEZDcmFzaFdpdGhJbmZvKGxpbmUsIGZpbGUsIGZ1bmN0aW9u LCBjb3VudGVyLCB3dGZDcmFzaEFyZyhyZWFzb24pLCB3dGZDcmFzaEFyZyhtaXNjMSkpOworfQor Cit0ZW1wbGF0ZTx0eXBlbmFtZSBULCB0eXBlbmFtZSBVLCB0eXBlbmFtZSBWPgorTk9fUkVUVVJO X0RVRV9UT19DUkFTSCBBTFdBWVNfSU5MSU5FIHZvaWQgV1RGQ3Jhc2hXaXRoSW5mbyhpbnQgbGlu ZSwgY29uc3QgY2hhciogZmlsZSwgY29uc3QgY2hhciogZnVuY3Rpb24sIGludCBjb3VudGVyLCBU IHJlYXNvbiwgVSBtaXNjMSwgViBtaXNjMikKK3sKKyAgICBXVEZDcmFzaFdpdGhJbmZvKGxpbmUs IGZpbGUsIGZ1bmN0aW9uLCBjb3VudGVyLCB3dGZDcmFzaEFyZyhyZWFzb24pLCB3dGZDcmFzaEFy ZyhtaXNjMSksIHd0ZkNyYXNoQXJnKG1pc2MyKSk7Cit9CisKK3RlbXBsYXRlPHR5cGVuYW1lIFQs IHR5cGVuYW1lIFUsIHR5cGVuYW1lIFYsIHR5cGVuYW1lIFc+CitOT19SRVRVUk5fRFVFX1RPX0NS QVNIIEFMV0FZU19JTkxJTkUgdm9pZCBXVEZDcmFzaFdpdGhJbmZvKGludCBsaW5lLCBjb25zdCBj aGFyKiBmaWxlLCBjb25zdCBjaGFyKiBmdW5jdGlvbiwgaW50IGNvdW50ZXIsIFQgcmVhc29uLCBV IG1pc2MxLCBWIG1pc2MyLCBXIG1pc2MzKQoreworICAgIFdURkNyYXNoV2l0aEluZm8obGluZSwg ZmlsZSwgZnVuY3Rpb24sIGNvdW50ZXIsIHd0ZkNyYXNoQXJnKHJlYXNvbiksIHd0ZkNyYXNoQXJn KG1pc2MxKSwgd3RmQ3Jhc2hBcmcobWlzYzIpLCB3dGZDcmFzaEFyZyhtaXNjMykpOworfQorCit0 ZW1wbGF0ZTx0eXBlbmFtZSBULCB0eXBlbmFtZSBVLCB0eXBlbmFtZSBWLCB0eXBlbmFtZSBXLCB0 eXBlbmFtZSBYPgorTk9fUkVUVVJOX0RVRV9UT19DUkFTSCBBTFdBWVNfSU5MSU5FIHZvaWQgV1RG Q3Jhc2hXaXRoSW5mbyhpbnQgbGluZSwgY29uc3QgY2hhciogZmlsZSwgY29uc3QgY2hhciogZnVu Y3Rpb24sIGludCBjb3VudGVyLCBUIHJlYXNvbiwgVSBtaXNjMSwgViBtaXNjMiwgVyBtaXNjMywg WCBtaXNjNCkKK3sKKyAgICBXVEZDcmFzaFdpdGhJbmZvKGxpbmUsIGZpbGUsIGZ1bmN0aW9uLCBj b3VudGVyLCB3dGZDcmFzaEFyZyhyZWFzb24pLCB3dGZDcmFzaEFyZyhtaXNjMSksIHd0ZkNyYXNo QXJnKG1pc2MyKSwgd3RmQ3Jhc2hBcmcobWlzYzMpLCB3dGZDcmFzaEFyZyhtaXNjNCkpOworfQor Cit0ZW1wbGF0ZTx0eXBlbmFtZSBULCB0eXBlbmFtZSBVLCB0eXBlbmFtZSBWLCB0eXBlbmFtZSBX LCB0eXBlbmFtZSBYLCB0eXBlbmFtZSBZPgorTk9fUkVUVVJOX0RVRV9UT19DUkFTSCBBTFdBWVNf SU5MSU5FIHZvaWQgV1RGQ3Jhc2hXaXRoSW5mbyhpbnQgbGluZSwgY29uc3QgY2hhciogZmlsZSwg Y29uc3QgY2hhciogZnVuY3Rpb24sIGludCBjb3VudGVyLCBUIHJlYXNvbiwgVSBtaXNjMSwgViBt aXNjMiwgVyBtaXNjMywgWCBtaXNjNCwgWSBtaXNjNSkKK3sKKyAgICBXVEZDcmFzaFdpdGhJbmZv KGxpbmUsIGZpbGUsIGZ1bmN0aW9uLCBjb3VudGVyLCB3dGZDcmFzaEFyZyhyZWFzb24pLCB3dGZD cmFzaEFyZyhtaXNjMSksIHd0ZkNyYXNoQXJnKG1pc2MyKSwgd3RmQ3Jhc2hBcmcobWlzYzMpLCB3 dGZDcmFzaEFyZyhtaXNjNCksIHd0ZkNyYXNoQXJnKG1pc2M1KSk7Cit9CisKK3RlbXBsYXRlPHR5 cGVuYW1lIFQsIHR5cGVuYW1lIFUsIHR5cGVuYW1lIFYsIHR5cGVuYW1lIFcsIHR5cGVuYW1lIFgs IHR5cGVuYW1lIFksIHR5cGVuYW1lIFo+CitOT19SRVRVUk5fRFVFX1RPX0NSQVNIIEFMV0FZU19J TkxJTkUgdm9pZCBXVEZDcmFzaFdpdGhJbmZvKGludCBsaW5lLCBjb25zdCBjaGFyKiBmaWxlLCBj b25zdCBjaGFyKiBmdW5jdGlvbiwgaW50IGNvdW50ZXIsIFQgcmVhc29uLCBVIG1pc2MxLCBWIG1p c2MyLCBXIG1pc2MzLCBYIG1pc2M0LCBZIG1pc2M1LCBaIG1pc2M2KQoreworICAgIFdURkNyYXNo V2l0aEluZm8obGluZSwgZmlsZSwgZnVuY3Rpb24sIGNvdW50ZXIsIHd0ZkNyYXNoQXJnKHJlYXNv biksIHd0ZkNyYXNoQXJnKG1pc2MxKSwgd3RmQ3Jhc2hBcmcobWlzYzIpLCB3dGZDcmFzaEFyZyht aXNjMyksIHd0ZkNyYXNoQXJnKG1pc2M0KSwgd3RmQ3Jhc2hBcmcobWlzYzUpLCB3dGZDcmFzaEFy ZyhtaXNjNikpOworfQorCiAvLyBUaGUgY29tYmluYXRpb24gb2YgbGluZSwgZmlsZSwgZnVuY3Rp b24sIGFuZCBjb3VudGVyIHNob3VsZCBiZSBhIHVuaXF1ZSBudW1iZXIgcGVyIGNhbGwgdG8gdGhp cyBjcmFzaC4gVGhpcyB0cmlja3MgdGhlIGNvbXBpbGVyIGludG8gbm90IGNvYWxlc2NpbmcgY2Fs bHMgdG8gV1RGQ3Jhc2hXaXRoSW5mby4KIC8vIFRoZSBlYXNpZXN0IHdheSB0byBmaWxsIHRoZXNl IHZhbHVlcyBwZXIgdHJhbnNsYXRpb24gdW5pdCBpcyB0byBwYXNzIF9fTElORV9fLCBfX0ZJTEVf XywgV1RGX1BSRVRUWV9GVU5DVElPTiwgYW5kIF9fQ09VTlRFUl9fLgogV1RGX0VYUE9SVF9QUklW QVRFIE5PX1JFVFVSTl9EVUVfVE9fQ1JBU0ggTk9UX1RBSUxfQ0FMTEVEIHZvaWQgV1RGQ3Jhc2hX aXRoSW5mbyhpbnQgbGluZSwgY29uc3QgY2hhciogZmlsZSwgY29uc3QgY2hhciogZnVuY3Rpb24s IGludCBjb3VudGVyLCB1aW50NjRfdCByZWFzb24sIHVpbnQ2NF90IG1pc2MxLCB1aW50NjRfdCBt aXNjMiwgdWludDY0X3QgbWlzYzMsIHVpbnQ2NF90IG1pc2M0LCB1aW50NjRfdCBtaXNjNSwgdWlu dDY0X3QgbWlzYzYpOwpAQCAtNTc0LDcgKzYyMiw3IEBAIGlubGluZSB2b2lkIGlzSW50ZWdyYWxU eXBlKCkgeyB9CiB0ZW1wbGF0ZTx0eXBlbmFtZSBULCB0eXBlbmFtZS4uLiBUeXBlcz4KIHZvaWQg aXNJbnRlZ3JhbFR5cGUoVCwgVHlwZXMuLi4gdHlwZXMpCiB7Ci0gICAgc3RhdGljX2Fzc2VydChz dGQ6OmlzX2ludGVncmFsPFQ+Ojp2YWx1ZSB8fCBzdGQ6OmlzX2VudW08VD46OnZhbHVlLCAiQWxs IHR5cGVzIG5lZWQgdG8gYmUgaW50ZWdyYWwgYml0d2lzZV9jYXN0IHRvIGludGVncmFsIHR5cGUg Zm9yIGxvZ2dpbmciKTsKKyAgICBzdGF0aWNfYXNzZXJ0KHN0ZDo6aXNfaW50ZWdyYWw8VD46OnZh bHVlIHx8IHN0ZDo6aXNfZW51bTxUPjo6dmFsdWUgfHwgc3RkOjppc19wb2ludGVyPFQ+Ojp2YWx1 ZSwgIkFsbCB0eXBlcyBuZWVkIHRvIGJlIGJpdHdpc2VfY2FzdC1hYmxlIHRvIGludGVncmFsIHR5 cGUgZm9yIGxvZ2dpbmciKTsKICAgICBpc0ludGVncmFsVHlwZSh0eXBlcy4uLik7CiB9CiB9Cg==