204763 2019-12-02 13:18:53 -0800 Only check each use...FuzzerAgent() option in VM constructor if any of the options are enabled. 2019-12-02 17:08:16 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mark.lam mark.lam ews-watchlist keith_miller msaboff saam tzagallo webkit-bug-importer oldest_to_newest 1594754 0 mark.lam 2019-12-02 13:18:53 -0800 We know that we'll never use fuzzer agents in deployment. Hence, we shouldn't spend time checking for them in the normal use case. 1594758 1 384652 mark.lam 2019-12-02 13:26:02 -0800 Created attachment 384652 proposed patch. 1594783 2 384652 tzagallo 2019-12-02 14:05:15 -0800 Comment on attachment 384652 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=384652&action=review > Source/JavaScriptCore/runtime/Options.cpp:550 > + FOR_EACH_JSC_OPTION(CHECK_IF_USING_FUZZER_AGENT) This is a bit unfortunate... how about maybe adding FOR_EACH_JSC_FUZZER_AGENT? That way you can call it here and just have FOR_EACH_JSC_OPTION call it as well. 1594786 3 384652 mark.lam 2019-12-02 14:14:04 -0800 Comment on attachment 384652 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=384652&action=review >> Source/JavaScriptCore/runtime/Options.cpp:550 >> + FOR_EACH_JSC_OPTION(CHECK_IF_USING_FUZZER_AGENT) > > This is a bit unfortunate... how about maybe adding FOR_EACH_JSC_FUZZER_AGENT? That way you can call it here and just have FOR_EACH_JSC_OPTION call it as well. Why? This code compiles down to one check each for the 4 use...FuzzerAgent options. All other options are no-ops that hey optimized away. I already verified it with a disassembler. 1594844 4 384652 keith_miller 2019-12-02 16:12:20 -0800 Comment on attachment 384652 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=384652&action=review > Source/JavaScriptCore/runtime/Options.cpp:545 > + if (nameLength > 14 && !strncmp(name, "use", 3) && !strncmp(&name[nameLength -11], "FuzzerAgent", 11)) { \ I think you just need to check for 11 but it probably doesn't matter. >>> Source/JavaScriptCore/runtime/Options.cpp:550 >>> + FOR_EACH_JSC_OPTION(CHECK_IF_USING_FUZZER_AGENT) >> >> This is a bit unfortunate... how about maybe adding FOR_EACH_JSC_FUZZER_AGENT? That way you can call it here and just have FOR_EACH_JSC_OPTION call it as well. > > Why? This code compiles down to one check each for the 4 use...FuzzerAgent options. All other options are no-ops that hey optimized away. I already verified it with a disassembler. Yeah, I don't think this matters because clang will probably (TM) compile fold this into a constant in production. 1594850 5 mark.lam 2019-12-02 16:21:18 -0800 Thanks for the review. Landed in r253015: <http://trac.webkit.org/r253015>. 1594852 6 webkit-bug-importer 2019-12-02 16:22:19 -0800 <rdar://problem/57575360> 1594861 7 384652 saam 2019-12-02 17:08:16 -0800 Comment on attachment 384652 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=384652&action=review > Source/JavaScriptCore/ChangeLog:13 > + We know that we'll never use fuzzer agents in deployment. Hence, we shouldn't > + spend time checking for them in the normal use case. This probably doesn't matter > + much for Web processes, but for clients of JSC that repeatedly spawn and kill VMs, > + it might matter more. We might want to eventually widen this idiom to include > + other debugging / development options, but for now, I'm only covering the fuzzer > + agent options. I actually think our time is better spent profiling where we spend time in VM construction and making measurable improvements. It seems super unlikely checking falsey debug options is where VM construction spends its time. 384652 2019-12-02 13:26:02 -0800 2019-12-02 16:12:20 -0800 proposed patch. bug-204763.patch text/plain 5538 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjUzMDA2KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI3IEBA CisyMDE5LTEyLTAyICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBP bmx5IGNoZWNrIGVhY2ggdXNlLi4uRnV6emVyQWdlbnQoKSBvcHRpb24gaW4gVk0gY29uc3RydWN0 b3IgaWYgYW55IG9mIHRoZSBvcHRpb25zIGFyZSBlbmFibGVkLgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjA0NzYzCisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV2Uga25vdyB0aGF0IHdlJ2xsIG5ldmVyIHVz ZSBmdXp6ZXIgYWdlbnRzIGluIGRlcGxveW1lbnQuICBIZW5jZSwgd2Ugc2hvdWxkbid0CisgICAg ICAgIHNwZW5kIHRpbWUgY2hlY2tpbmcgZm9yIHRoZW0gaW4gdGhlIG5vcm1hbCB1c2UgY2FzZS4g IFRoaXMgcHJvYmFibHkgZG9lc24ndCBtYXR0ZXIKKyAgICAgICAgbXVjaCBmb3IgV2ViIHByb2Nl c3NlcywgYnV0IGZvciBjbGllbnRzIG9mIEpTQyB0aGF0IHJlcGVhdGVkbHkgc3Bhd24gYW5kIGtp bGwgVk1zLAorICAgICAgICBpdCBtaWdodCBtYXR0ZXIgbW9yZS4gIFdlIG1pZ2h0IHdhbnQgdG8g ZXZlbnR1YWxseSB3aWRlbiB0aGlzIGlkaW9tIHRvIGluY2x1ZGUKKyAgICAgICAgb3RoZXIgZGVi dWdnaW5nIC8gZGV2ZWxvcG1lbnQgb3B0aW9ucywgYnV0IGZvciBub3csIEknbSBvbmx5IGNvdmVy aW5nIHRoZSBmdXp6ZXIKKyAgICAgICAgYWdlbnQgb3B0aW9ucy4KKworICAgICAgICAqIHJ1bnRp bWUvT3B0aW9ucy5jcHA6CisgICAgICAgIChKU0M6OmNvbXB1dGVJZlVzaW5nRnV6emVyQWdlbnQp OgorICAgICAgICAoSlNDOjpPcHRpb25zOjppbml0aWFsaXplKToKKyAgICAgICAgKiBydW50aW1l L09wdGlvbnMuaDoKKyAgICAgICAgKEpTQzo6T3B0aW9uczo6aXNVc2luZ0Z1enplckFnZW50KToK KyAgICAgICAgKiBydW50aW1lL09wdGlvbnNMaXN0Lmg6CisgICAgICAgIChKU0M6Ok9wdGlvblJh bmdlOjpvcGVyYXRvciBib29sIGNvbnN0KToKKyAgICAgICAgKiBydW50aW1lL1ZNLmNwcDoKKyAg ICAgICAgKEpTQzo6Vk06OlZNKToKKwogMjAxOS0xMi0wMSAgVHVvbWFzIEthcmtrYWluZW4gIDx0 dW9tYXMud2Via2l0QGFwcGxlLmNvbT4KIAogICAgICAgICBBZGQgRnV6emVyQWdlbnQgdGhhdCBy ZWFkcyBwcmVkaWN0aW9ucyBmcm9tIGEgZmlsZQpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3Jl L3J1bnRpbWUvT3B0aW9ucy5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3Jl L3J1bnRpbWUvT3B0aW9ucy5jcHAJKHJldmlzaW9uIDI1MzAwMSkKKysrIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9ydW50aW1lL09wdGlvbnMuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01MzYsNiArNTM2 LDIxIEBAIGlubGluZSB2b2lkKiBPcHRpb25zOjphZGRyZXNzT2ZPcHRpb25EZWYKICAgICByZXR1 cm4gcmVpbnRlcnByZXRfY2FzdDx1aW50OF90Kj4oJmdfanNjQ29uZmlnLm9wdGlvbnMpICsgb2Zm c2V0OwogfQogCitzdGF0aWMgdm9pZCBjb21wdXRlSWZVc2luZ0Z1enplckFnZW50KCkKK3sKKyAg ICBnX2pzY0NvbmZpZy5vcHRpb25zLmlzVXNpbmdGdXp6ZXJBZ2VudCA9IGZhbHNlOworI2RlZmlu ZSBDSEVDS19JRl9VU0lOR19GVVpaRVJfQUdFTlQodHlwZV8sIG5hbWVfLCBkZWZhdWx0VmFsdWVf LCBhdmFpbGFiaWxpdHlfLCBkZXNjcmlwdGlvbl8pIHsgXAorICAgICAgICBjb25zdCBjaGFyIG5h bWVbXSA9ICNuYW1lXzsgXAorICAgICAgICB1bnNpZ25lZCBuYW1lTGVuZ3RoID0gc3RybGVuKG5h bWUpOyBcCisgICAgICAgIGlmIChuYW1lTGVuZ3RoID4gMTQgJiYgIXN0cm5jbXAobmFtZSwgInVz ZSIsIDMpICYmICFzdHJuY21wKCZuYW1lW25hbWVMZW5ndGggLTExXSwgIkZ1enplckFnZW50Iiwg MTEpKSB7IFwKKyAgICAgICAgICAgIGlmIChPcHRpb25zOjpuYW1lXygpKSBcCisgICAgICAgICAg ICAgICAgZ19qc2NDb25maWcub3B0aW9ucy5pc1VzaW5nRnV6emVyQWdlbnQgPSB0cnVlOyBcCisg ICAgICAgIH0gXAorICAgIH0KKyAgICBGT1JfRUFDSF9KU0NfT1BUSU9OKENIRUNLX0lGX1VTSU5H X0ZVWlpFUl9BR0VOVCkKKyN1bmRlZiBDSEVDS19JRl9VU0lOR19GVVpaRVJfQUdFTlQKK30KKwog dm9pZCBPcHRpb25zOjppbml0aWFsaXplKCkKIHsKICAgICBzdGF0aWMgc3RkOjpvbmNlX2ZsYWcg aW5pdGlhbGl6ZU9wdGlvbnNPbmNlRmxhZzsKQEAgLTYwOSw2ICs2MjQsNyBAQCB2b2lkIE9wdGlv bnM6OmluaXRpYWxpemUoKQogCiAgICAgICAgICAgICBkdW1wT3B0aW9uc0lmTmVlZGVkKCk7CiAg ICAgICAgICAgICBlbnN1cmVPcHRpb25zQXJlQ29oZXJlbnQoKTsKKyAgICAgICAgICAgIGNvbXB1 dGVJZlVzaW5nRnV6emVyQWdlbnQoKTsKIAogI2lmIEhBVkUoTUFDSF9FWENFUFRJT05TKQogICAg ICAgICAgICAgaWYgKE9wdGlvbnM6OnVzZU1hY2hGb3JFeGNlcHRpb25zKCkpCkluZGV4OiBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9PcHRpb25zLmgKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNl L0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvT3B0aW9ucy5oCShyZXZpc2lvbiAyNTMwMDEpCisrKyBT b3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9PcHRpb25zLmgJKHdvcmtpbmcgY29weSkKQEAg LTk3LDYgKzk3LDggQEAgcHVibGljOgogCiAgICAgc3RhdGljIGJvb2wgaXNBdmFpbGFibGUoSUQs IEF2YWlsYWJpbGl0eSk7CiAKKyAgICBzdGF0aWMgYm9vbCBpc1VzaW5nRnV6emVyQWdlbnQoKSB7 IHJldHVybiBnX2pzY0NvbmZpZy5vcHRpb25zLmlzVXNpbmdGdXp6ZXJBZ2VudDsgfQorCiBwcml2 YXRlOgogICAgIHN0cnVjdCBDb25zdE1ldGFEYXRhIHsKICAgICAgICAgY29uc3QgY2hhciogbmFt ZTsKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL09wdGlvbnNMaXN0LmgKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvT3B0aW9uc0xpc3QuaAko cmV2aXNpb24gMjUzMDAxKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvT3B0aW9u c0xpc3QuaAkod29ya2luZyBjb3B5KQpAQCAtNTcyLDYgKzU3Miw4IEBAIHB1YmxpYzoKICAgICBi b29sIGlzSW5SYW5nZSh1bnNpZ25lZCk7CiAgICAgY29uc3QgY2hhciogcmFuZ2VTdHJpbmcoKSBj b25zdCB7IHJldHVybiAobV9zdGF0ZSA+IEluaXRFcnJvcikgPyBtX3JhbmdlU3RyaW5nIDogc19u dWxsUmFuZ2VTdHI7IH0KICAgICAKKyAgICBvcGVyYXRvciBib29sKCkgY29uc3QgeyByZXR1cm4g bV9zdGF0ZSAhPSBVbmluaXRpYWxpemVkOyB9CisKICAgICB2b2lkIGR1bXAoUHJpbnRTdHJlYW0m IG91dCkgY29uc3Q7CiAKIHByaXZhdGU6CkBAIC01OTgsNiArNjAwLDggQEAgc3RydWN0IE9wdGlv bnNTdG9yYWdlIHsKICAgICB0eXBlXyBuYW1lXyMjRGVmYXVsdDsKIEZPUl9FQUNIX0pTQ19PUFRJ T04oREVDTEFSRV9PUFRJT04pCiAjdW5kZWYgREVDTEFSRV9PUFRJT04KKworICAgIGJvb2wgaXNV c2luZ0Z1enplckFnZW50OyAvLyBUaGlzIHZhbHVlIGlzIGNvbXB1dGVkIGluIE9wdGlvbnM6Omlu aXRpYWxpemUoKS4KIH07CiAKIC8vIE9wdGlvbnM6Ok1ldGFkYXRhJ3Mgb2Zmc2V0T2ZPcHRpb24g YW5kIG9mZnNldE9mT3B0aW9uRGVmYXVsdCByZWxpZXMgb24gdGhpcy4KSW5kZXg6IFNvdXJjZS9K YXZhU2NyaXB0Q29yZS9ydW50aW1lL1ZNLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNj cmlwdENvcmUvcnVudGltZS9WTS5jcHAJKHJldmlzaW9uIDI1MzAwMSkKKysrIFNvdXJjZS9KYXZh U2NyaXB0Q29yZS9ydW50aW1lL1ZNLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDYxLDE0ICs0NjEs MTYgQEAgVk06OlZNKFZNVHlwZSB2bVR5cGUsIEhlYXBUeXBlIGhlYXBUeXBlKQogICAgIH0KICNl bmRpZiAvLyBFTkFCTEUoU0FNUExJTkdfUFJPRklMRVIpCiAKLSAgICBpZiAoT3B0aW9uczo6dXNl UmFuZG9taXppbmdGdXp6ZXJBZ2VudCgpKQotICAgICAgICBzZXRGdXp6ZXJBZ2VudChtYWtlVW5p cXVlPFJhbmRvbWl6aW5nRnV6emVyQWdlbnQ+KCp0aGlzKSk7Ci0gICAgaWYgKE9wdGlvbnM6OnVz ZURvdWJsZVByZWRpY3Rpb25GdXp6ZXJBZ2VudCgpKQotICAgICAgICBzZXRGdXp6ZXJBZ2VudCht YWtlVW5pcXVlPERvdWJsZVByZWRpY3Rpb25GdXp6ZXJBZ2VudD4oKnRoaXMpKTsKLSAgICBpZiAo T3B0aW9uczo6dXNlRmlsZUJhc2VkRnV6emVyQWdlbnQoKSkKLSAgICAgICAgc2V0RnV6emVyQWdl bnQobWFrZVVuaXF1ZTxGaWxlQmFzZWRGdXp6ZXJBZ2VudD4oKnRoaXMpKTsKLSAgICBpZiAoT3B0 aW9uczo6dXNlUHJlZGljdGlvbkZpbGVDcmVhdGluZ0Z1enplckFnZW50KCkpCi0gICAgICAgIHNl dEZ1enplckFnZW50KG1ha2VVbmlxdWU8UHJlZGljdGlvbkZpbGVDcmVhdGluZ0Z1enplckFnZW50 PigqdGhpcykpOworICAgIGlmIChVTkxJS0VMWShPcHRpb25zOjppc1VzaW5nRnV6emVyQWdlbnQo KSkpIHsKKyAgICAgICAgaWYgKE9wdGlvbnM6OnVzZVJhbmRvbWl6aW5nRnV6emVyQWdlbnQoKSkK KyAgICAgICAgICAgIHNldEZ1enplckFnZW50KG1ha2VVbmlxdWU8UmFuZG9taXppbmdGdXp6ZXJB Z2VudD4oKnRoaXMpKTsKKyAgICAgICAgaWYgKE9wdGlvbnM6OnVzZURvdWJsZVByZWRpY3Rpb25G dXp6ZXJBZ2VudCgpKQorICAgICAgICAgICAgc2V0RnV6emVyQWdlbnQobWFrZVVuaXF1ZTxEb3Vi bGVQcmVkaWN0aW9uRnV6emVyQWdlbnQ+KCp0aGlzKSk7CisgICAgICAgIGlmIChPcHRpb25zOjp1 c2VGaWxlQmFzZWRGdXp6ZXJBZ2VudCgpKQorICAgICAgICAgICAgc2V0RnV6emVyQWdlbnQobWFr ZVVuaXF1ZTxGaWxlQmFzZWRGdXp6ZXJBZ2VudD4oKnRoaXMpKTsKKyAgICAgICAgaWYgKE9wdGlv bnM6OnVzZVByZWRpY3Rpb25GaWxlQ3JlYXRpbmdGdXp6ZXJBZ2VudCgpKQorICAgICAgICAgICAg c2V0RnV6emVyQWdlbnQobWFrZVVuaXF1ZTxQcmVkaWN0aW9uRmlsZUNyZWF0aW5nRnV6emVyQWdl bnQ+KCp0aGlzKSk7CisgICAgfQogCiAgICAgaWYgKE9wdGlvbnM6OmFsd2F5c0dlbmVyYXRlUENU b0NvZGVPcmlnaW5NYXAoKSkKICAgICAgICAgc2V0U2hvdWxkQnVpbGRQQ1RvQ29kZU9yaWdpbk1h cHBpbmcoKTsK