227962 2021-07-14 11:38:38 -0700 Check for out of memory in JSC::globalFuncEscape() and JSC::globalFuncUnescape(). 2021-07-14 12:16:43 -0700 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 ysuzuki oldest_to_newest 1776984 0 mark.lam 2021-07-14 11:38:38 -0700 rdar://78392251 1776986 1 433514 mark.lam 2021-07-14 11:41:41 -0700 Created attachment 433514 proposed patch. 1776987 2 433514 ysuzuki 2021-07-14 11:45:14 -0700 Comment on attachment 433514 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=433514&action=review r=me > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:616 > + return vm.smallStrings.emptyString(); Why not returning `{ }`? > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:679 > + return vm.smallStrings.emptyString(); Why not returning `{ }` ? 1776992 3 mark.lam 2021-07-14 11:54:00 -0700 (In reply to Yusuke Suzuki from comment #2) > Comment on attachment 433514 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=433514&action=review > > r=me Thanks for the review. > > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:616 > > + return vm.smallStrings.emptyString(); > > Why not returning `{ }`? > > > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:679 > > + return vm.smallStrings.emptyString(); > > Why not returning `{ }` ? Because the return value is for a callback called from toStringView(). I gave returning { } a try and this is what I get: ./runtime/JSGlobalObjectFunctions.cpp:617:20: error: cannot deduce lambda return type from initializer list return { }; ^~~ ./runtime/JSGlobalObjectFunctions.cpp:580:28: error: no matching function for call to 'toStringView' return JSValue::encode(toStringView(globalObject, callFrame->argument(0), [&] (StringView view) { ^~~~~~~~~~~~ I'll just stick with returning the empty string for simplicity. 1776994 4 mark.lam 2021-07-14 12:05:47 -0700 (In reply to Mark Lam from comment #3) > > > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:616 > > > + return vm.smallStrings.emptyString(); > > > > Why not returning `{ }`? ... > Because the return value is for a callback called from toStringView(). I > gave returning { } a try and this is what I get: > > ./runtime/JSGlobalObjectFunctions.cpp:617:20: error: cannot deduce lambda > return type from initializer list > return { }; > ^~~ On second thought, I can explicitly declare the lambdas as returning JSString*. It will result in a roundabout way of returning an empty JSValue() on 64-bit and a null cell JSValue on 32-bit ... which is kind of inconsistent, but we already apply this treatment when throwing exceptions elsewhere in the code. I'll just do that. 1776999 5 mark.lam 2021-07-14 12:16:43 -0700 Landed in r279915: <http://trac.webkit.org/r279915>. 433514 2021-07-14 11:41:41 -0700 2021-07-14 11:45:14 -0700 proposed patch. bug-227962.patch text/plain 4770 mark.lam SW5kZXg6IEpTVGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEpTVGVzdHMvQ2hhbmdlTG9n CShyZXZpc2lvbiAyNzk5MTMpCisrKyBKU1Rlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpA QCAtMSwzICsxLDEzIEBACisyMDIxLTA3LTE0ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNv bT4KKworICAgICAgICBDaGVjayBmb3Igb3V0IG9mIG1lbW9yeSBpbiBKU0M6Omdsb2JhbEZ1bmNF c2NhcGUoKSBhbmQgSlNDOjpnbG9iYWxGdW5jVW5lc2NhcGUoKS4KKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIyNzk2MgorICAgICAgICByZGFyOi8vNzgz OTIyNTEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAq IHN0cmVzcy9vdXQtb2YtbWVtb3J5LWluLWdsb2JhbEZ1bmNVbmVzY2FwZS5qczogQWRkZWQuCisK IDIwMjEtMDctMTQgIE1hcmsgTGFtICA8bWFyay5sYW1AYXBwbGUuY29tPgogCiAgICAgICAgIFBs YWNhdGUgZXhjZXB0aW9uIGNoZWNrZXIgdmFsaWRhdGlvbiBpbiBvcGVyYXRpb25PYmplY3RBc3Np Z25VbnR5cGVkLgpJbmRleDogSlNUZXN0cy9zdHJlc3Mvb3V0LW9mLW1lbW9yeS1pbi1nbG9iYWxG dW5jVW5lc2NhcGUuanMKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gSlNUZXN0cy9zdHJlc3Mvb3V0LW9mLW1lbW9y eS1pbi1nbG9iYWxGdW5jVW5lc2NhcGUuanMJKG5vbmV4aXN0ZW50KQorKysgSlNUZXN0cy9zdHJl c3Mvb3V0LW9mLW1lbW9yeS1pbi1nbG9iYWxGdW5jVW5lc2NhcGUuanMJKHdvcmtpbmcgY29weSkK QEAgLTAsMCArMSwxMSBAQAorLy9AIGlmICRidWlsZFR5cGUgPT0gInJlbGVhc2UiICYmICEkbWVt b3J5TGltaXRlZCB0aGVuIHJ1bkRlZmF1bHQgZWxzZSBza2lwIGVuZAorCit2YXIgZXhjZXB0aW9u OwordHJ5IHsKKyAgICB1bmVzY2FwZSgnXHUwMTAwJy5yZXBlYXQoMioqMzApKTsKK30gY2F0Y2gg KGUpIHsKKyAgICBleGNlcHRpb24gPSBlOworfQorCitpZiAoZXhjZXB0aW9uICE9ICJSYW5nZUVy cm9yOiBPdXQgb2YgbWVtb3J5IikKKyAgICB0aHJvdyAiRkFJTEVEIjsKSW5kZXg6IFNvdXJjZS9K YXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjc5ODYxKQorKysgU291cmNlL0phdmFTY3JpcHRDb3Jl L0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDIxLTA3LTE0ICBN YXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBDaGVjayBmb3Igb3V0IG9m IG1lbW9yeSBpbiBKU0M6Omdsb2JhbEZ1bmNFc2NhcGUoKSBhbmQgSlNDOjpnbG9iYWxGdW5jVW5l c2NhcGUoKS4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTIyNzk2MgorICAgICAgICByZGFyOi8vNzgzOTIyNTEKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHJ1bnRpbWUvSlNHbG9iYWxPYmplY3RGdW5jdGlv bnMuY3BwOgorICAgICAgICAoSlNDOjpKU0NfREVGSU5FX0hPU1RfRlVOQ1RJT04pOgorCiAyMDIx LTA3LTEyICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZlcnQg cjI3NzAyNzogYnJlYWtzIEdDLgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUv SlNHbG9iYWxPYmplY3RGdW5jdGlvbnMuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9ydW50aW1lL0pTR2xvYmFsT2JqZWN0RnVuY3Rpb25zLmNwcAkocmV2aXNpb24gMjc5 ODYwKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNHbG9iYWxPYmplY3RGdW5j dGlvbnMuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01ODYsNyArNTg2LDkgQEAgSlNDX0RFRklORV9I T1NUX0ZVTkNUSU9OKGdsb2JhbEZ1bmNFc2NhcAogICAgICAgICApOwogCiAgICAgICAgIFZNJiB2 bSA9IGdsb2JhbE9iamVjdC0+dm0oKTsKLSAgICAgICAgU3RyaW5nQnVpbGRlciBidWlsZGVyOwor ICAgICAgICBhdXRvIHNjb3BlID0gREVDTEFSRV9USFJPV19TQ09QRSh2bSk7CisKKyAgICAgICAg U3RyaW5nQnVpbGRlciBidWlsZGVyKFN0cmluZ0J1aWxkZXI6Ok92ZXJmbG93SGFuZGxlcjo6UmVj b3JkT3ZlcmZsb3cpOwogICAgICAgICBpZiAodmlldy5pczhCaXQoKSkgewogICAgICAgICAgICAg Y29uc3QgTENoYXIqIGMgPSB2aWV3LmNoYXJhY3RlcnM4KCk7CiAgICAgICAgICAgICBmb3IgKHVu c2lnbmVkIGsgPSAwOyBrIDwgdmlldy5sZW5ndGgoKTsgaysrLCBjKyspIHsKQEAgLTU5NiwyMCAr NTk4LDIzIEBAIEpTQ19ERUZJTkVfSE9TVF9GVU5DVElPTihnbG9iYWxGdW5jRXNjYXAKICAgICAg ICAgICAgICAgICBlbHNlCiAgICAgICAgICAgICAgICAgICAgIGJ1aWxkZXIuYXBwZW5kKCclJywg aGV4KHUsIDIpKTsKICAgICAgICAgICAgIH0KLSAgICAgICAgICAgIHJldHVybiBqc1N0cmluZyh2 bSwgYnVpbGRlci50b1N0cmluZygpKTsKKyAgICAgICAgfSBlbHNlIHsKKyAgICAgICAgICAgIGNv bnN0IFVDaGFyKiBjID0gdmlldy5jaGFyYWN0ZXJzMTYoKTsKKyAgICAgICAgICAgIGZvciAodW5z aWduZWQgayA9IDA7IGsgPCB2aWV3Lmxlbmd0aCgpOyBrKyssIGMrKykgeworICAgICAgICAgICAg ICAgIFVDaGFyIHUgPSBjWzBdOworICAgICAgICAgICAgICAgIGlmICh1ID49IGRvTm90RXNjYXBl LnNpemUoKSkKKyAgICAgICAgICAgICAgICAgICAgYnVpbGRlci5hcHBlbmQoIiV1IiwgaGV4KHN0 YXRpY19jYXN0PHVpbnQ4X3Q+KHUgPj4gOCksIDIpLCBoZXgoc3RhdGljX2Nhc3Q8dWludDhfdD4o dSksIDIpKTsKKyAgICAgICAgICAgICAgICBlbHNlIGlmIChkb05vdEVzY2FwZS5nZXQoc3RhdGlj X2Nhc3Q8TENoYXI+KHUpKSkKKyAgICAgICAgICAgICAgICAgICAgYnVpbGRlci5hcHBlbmQoKmMp OworICAgICAgICAgICAgICAgIGVsc2UKKyAgICAgICAgICAgICAgICAgICAgYnVpbGRlci5hcHBl bmQoJyUnLCBoZXgodSwgMikpOworICAgICAgICAgICAgfQogICAgICAgICB9CiAKLSAgICAgICAg Y29uc3QgVUNoYXIqIGMgPSB2aWV3LmNoYXJhY3RlcnMxNigpOwotICAgICAgICBmb3IgKHVuc2ln bmVkIGsgPSAwOyBrIDwgdmlldy5sZW5ndGgoKTsgaysrLCBjKyspIHsKLSAgICAgICAgICAgIFVD aGFyIHUgPSBjWzBdOwotICAgICAgICAgICAgaWYgKHUgPj0gZG9Ob3RFc2NhcGUuc2l6ZSgpKQot ICAgICAgICAgICAgICAgIGJ1aWxkZXIuYXBwZW5kKCIldSIsIGhleChzdGF0aWNfY2FzdDx1aW50 OF90Pih1ID4+IDgpLCAyKSwgaGV4KHN0YXRpY19jYXN0PHVpbnQ4X3Q+KHUpLCAyKSk7Ci0gICAg ICAgICAgICBlbHNlIGlmIChkb05vdEVzY2FwZS5nZXQoc3RhdGljX2Nhc3Q8TENoYXI+KHUpKSkK LSAgICAgICAgICAgICAgICBidWlsZGVyLmFwcGVuZCgqYyk7Ci0gICAgICAgICAgICBlbHNlCi0g ICAgICAgICAgICAgICAgYnVpbGRlci5hcHBlbmQoJyUnLCBoZXgodSwgMikpOworICAgICAgICBp ZiAoVU5MSUtFTFkoYnVpbGRlci5oYXNPdmVyZmxvd2VkKCkpKSB7CisgICAgICAgICAgICB0aHJv d091dE9mTWVtb3J5RXJyb3IoZ2xvYmFsT2JqZWN0LCBzY29wZSk7CisgICAgICAgICAgICByZXR1 cm4gdm0uc21hbGxTdHJpbmdzLmVtcHR5U3RyaW5nKCk7CiAgICAgICAgIH0KLQogICAgICAgICBy ZXR1cm4ganNTdHJpbmcodm0sIGJ1aWxkZXIudG9TdHJpbmcoKSk7CiAgICAgfSkpOwogfQpAQCAt NjIyLDcgKzYyNywxMCBAQCBKU0NfREVGSU5FX0hPU1RfRlVOQ1RJT04oZ2xvYmFsRnVuY1VuZXNj CiAgICAgICAgIGludCBrID0gMDsKICAgICAgICAgaW50IGxlbmd0aCA9IHZpZXcubGVuZ3RoKCk7 CiAKLSAgICAgICAgU3RyaW5nQnVpbGRlciBidWlsZGVyOworICAgICAgICBWTSYgdm0gPSBnbG9i YWxPYmplY3QtPnZtKCk7CisgICAgICAgIGF1dG8gc2NvcGUgPSBERUNMQVJFX1RIUk9XX1NDT1BF KHZtKTsKKworICAgICAgICBTdHJpbmdCdWlsZGVyIGJ1aWxkZXIoU3RyaW5nQnVpbGRlcjo6T3Zl cmZsb3dIYW5kbGVyOjpSZWNvcmRPdmVyZmxvdyk7CiAgICAgICAgIGJ1aWxkZXIucmVzZXJ2ZUNh cGFjaXR5KGxlbmd0aCk7CiAKICAgICAgICAgaWYgKHZpZXcuaXM4Qml0KCkpIHsKQEAgLTY2Niw3 ICs2NzQsMTEgQEAgSlNDX0RFRklORV9IT1NUX0ZVTkNUSU9OKGdsb2JhbEZ1bmNVbmVzYwogICAg ICAgICAgICAgfQogICAgICAgICB9CiAKLSAgICAgICAgcmV0dXJuIGpzU3RyaW5nKGdsb2JhbE9i amVjdC0+dm0oKSwgYnVpbGRlci50b1N0cmluZygpKTsKKyAgICAgICAgaWYgKFVOTElLRUxZKGJ1 aWxkZXIuaGFzT3ZlcmZsb3dlZCgpKSkgeworICAgICAgICAgICAgdGhyb3dPdXRPZk1lbW9yeUVy cm9yKGdsb2JhbE9iamVjdCwgc2NvcGUpOworICAgICAgICAgICAgcmV0dXJuIHZtLnNtYWxsU3Ry aW5ncy5lbXB0eVN0cmluZygpOworICAgICAgICB9CisgICAgICAgIHJldHVybiBqc1N0cmluZyh2 bSwgYnVpbGRlci50b1N0cmluZygpKTsKICAgICB9KSk7CiB9CiAK