192795 2018-12-17 20:27:17 -0800 Array unshift/shift should not race against the AI in the compiler thread. 2018-12-17 22:57:00 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mark.lam mark.lam commit-queue fpizlo keith_miller msaboff rmorisset saam tzagallo webkit-bug-importer ysuzuki oldest_to_newest 1488754 0 mark.lam 2018-12-17 20:27:17 -0800 <rdar://problem/46724263> 1488759 1 357527 mark.lam 2018-12-17 20:48:07 -0800 Created attachment 357527 proposed patch. 1488772 2 357527 saam 2018-12-17 21:30:46 -0800 Comment on attachment 357527 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=357527&action=review > Source/JavaScriptCore/ChangeLog:16 > + operations can move values around in the butterfly. Hence, the fact that AI has Do any operations do this without grabbing the lock? Or does this kind of moving around require holding the lock? 1488795 3 357527 mark.lam 2018-12-17 22:14:52 -0800 Comment on attachment 357527 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=357527&action=review Thanks for the review. >> Source/JavaScriptCore/ChangeLog:16 >> + operations can move values around in the butterfly. Hence, the fact that AI has > > Do any operations do this without grabbing the lock? Or does this kind of moving around require holding the lock? I can't think of any other operations that keeps the moves the m_sparseMap around the butterfly like this. I also did a survey of setButterfly(), nukeStructureAndSetButterfly(), the setting of arrayStorage()->m_indexBias, and the setting of arrayStorage()->m_sparseMap. AFAICT, in all cases, we either (1) allocating a new butterfly and fully initializing it before setting it in the object, which means just fetching the butterfly pointer is sufficient to guarantee we can read a meaningful value from it if the structure after is still the same, or (2) the butterfly is already in ArrayStorage format, and we're setting the m_sparseMap either to a new sparse map or to null. Other than unshift and shift, I didn't see anything that moves the location of m_sparseMap in the butterfly. So, I think we're good to go. 1488808 4 357527 commit-queue 2018-12-17 22:56:59 -0800 Comment on attachment 357527 proposed patch. Clearing flags on attachment: 357527 Committed r239325: <https://trac.webkit.org/changeset/239325> 1488809 5 commit-queue 2018-12-17 22:57:00 -0800 All reviewed patches have been landed. Closing bug. 357527 2018-12-17 20:48:07 -0800 2018-12-17 22:56:59 -0800 proposed patch. bug-192795.patch text/plain 7115 mark.lam SW5kZXg6IEpTVGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEpTVGVzdHMvQ2hhbmdlTG9n CShyZXZpc2lvbiAyMzkzMTYpCisrKyBKU1Rlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpA QCAtMSwzICsxLDEzIEBACisyMDE4LTEyLTE3ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNv bT4KKworICAgICAgICBBcnJheSB1bnNoaWZ0L3NoaWZ0IHNob3VsZCBub3QgcmFjZSBhZ2FpbnN0 IHRoZSBBSSBpbiB0aGUgY29tcGlsZXIgdGhyZWFkLgorICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTkyNzk1CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS80 NjcyNDI2Mz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICAqIHN0cmVzcy9hcnJheS11bnNoaWZ0LXNob3VsZC1ub3QtcmFjZS1hZ2FpbnN0LWNvbXBpbGVy LXRocmVhZC5qczogQWRkZWQuCisKIDIwMTgtMTItMTcgIE1hcmsgTGFtICA8bWFyay5sYW1AYXBw bGUuY29tPgogCiAgICAgICAgIFNhbXBsaW5nUHJvZmlsZXIncyBpc1ZhbGlkRnJhbWVQb2ludGVy KCkgc2hvdWxkIHJlamVjdCBhZGRyZXNzIGF0IHN0YWNrIG9yaWdpbi4KSW5kZXg6IEpTVGVzdHMv c3RyZXNzL2FycmF5LXVuc2hpZnQtc2hvdWxkLW5vdC1yYWNlLWFnYWluc3QtY29tcGlsZXItdGhy ZWFkLmpzCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIEpTVGVzdHMvc3RyZXNzL2FycmF5LXVuc2hpZnQtc2hvdWxk LW5vdC1yYWNlLWFnYWluc3QtY29tcGlsZXItdGhyZWFkLmpzCShub25leGlzdGVudCkKKysrIEpT VGVzdHMvc3RyZXNzL2FycmF5LXVuc2hpZnQtc2hvdWxkLW5vdC1yYWNlLWFnYWluc3QtY29tcGls ZXItdGhyZWFkLmpzCSh3b3JraW5nIGNvcHkpCkBAIC0wLDAgKzEsNyBAQAorbGV0IHggPSBbXTsK K2ZvciAobGV0IGkgPSAwOyBpIDwgMzA7ICsraSkgeworICAgIGZvciAobGV0IGogPSAwOyBqIDwg MjAwMDA7ICsraikgeworICAgICAgICB4WzBdCisgICAgICAgIHgudW5zaGlmdCh1bmRlZmluZWQp OworICAgIH0KK30KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjM5MzEy KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAt MSwzICsxLDM2IEBACisyMDE4LTEyLTE3ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4K KworICAgICAgICBBcnJheSB1bnNoaWZ0L3NoaWZ0IHNob3VsZCBub3QgcmFjZSBhZ2FpbnN0IHRo ZSBBSSBpbiB0aGUgY29tcGlsZXIgdGhyZWFkLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTkyNzk1CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS80Njcy NDI2Mz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBU aGUgQXJyYXkgdW5zaGlmdCBhbmQgc2hpZnQgb3BlcmF0aW9ucyBmb3IgQXJyYXlTdG9yYWdlIHR5 cGUgYXJyYXlzIGFyZSBwcm90ZWN0ZWQKKyAgICAgICAgdXNpbmcgdGhlIGNlbGxMb2NrLiAgVGhl IEFic3RyYWN0SW50ZXJwcmV0ZXIncyBmb2xkR2V0QnlWYWxPbkNvbnN0YW50UHJvcGVydHkoKQor ICAgICAgICBmdW5jdGlvbiBkb2VzIGdyYWIgdGhlIGNlbGxMb2NrIGJlZm9yZSByZWFkaW5nIGEg dmFsdWUgZnJvbSB0aGUgYXJyYXkncyBBcnJheVN0b3JhZ2UsCisgICAgICAgIGJ1dCBkb2VzIG5v dCBnZXQgdGhlIGFycmF5IGJ1dHRlcmZseSB1bmRlciB0aGUgcHJvdGVjdGlvbiBvZiB0aGUgY2Vs bExvY2suCisKKyAgICAgICAgVGhpcyBpcyBpbnN1ZmZpY2llbnQgYW5kIHJhY3kuICBGb3IgQXJy YXlTdG9yYWdlIHR5cGUgYXJyYXlzLCB0aGUgZmV0Y2hpbmcgb2YgdGhlCisgICAgICAgIGJ1dHRl cmZseSBhbHNvIG5lZWRzIHRvIGJlIHByb3RlY3RlZCBieSB0aGUgY2VsbExvY2suICBUaGUgdW5z aGlmdCAvIHNoaWZ0CisgICAgICAgIG9wZXJhdGlvbnMgY2FuIG1vdmUgdmFsdWVzIGFyb3VuZCBp biB0aGUgYnV0dGVyZmx5LiAgSGVuY2UsIHRoZSBmYWN0IHRoYXQgQUkgaGFzCisgICAgICAgIGZl dGNoZWQgYSBidXR0ZXJmbHkgcG9pbnRlciAod2hpbGUgZW5zdXJpbmcgbm8gc3RydWN0dXJlIGNo YW5nZSkgaXMgaW5zdWZmaWNpZW50CisgICAgICAgIHRvIGd1YXJhbnRlZSB0aGF0IHRoZSB2YWx1 ZXMgaW4gdGhlIGJ1dHRlcmZseSBoYXZlbid0IHNoaWZ0ZWQuCisKKyAgICAgICAgSGF2aW5nIEFJ IGhvbGQgdGhlIGNlbGxMb2NrIHRoZSB3aG9sZSB0aW1lIChmcm9tIGJlZm9yZSBmZXRjaGluZyB0 aGUgYnV0dGVyZmx5CisgICAgICAgIHRpbGwgYWZ0ZXIgcmVhZGluZyB0aGUgdmFsdWUgZnJvbSBp dCkgZWxpbWluYXRlcyB0aGlzIHJhY2UuICBOb3RlOiB3ZSBvbmx5IG5lZWQKKyAgICAgICAgdG8g ZG8gdGhpcyBmb3IgQXJyYXlTdG9yYWdlIHR5cGUgYXJyYXlzLgorCisgICAgICAgIE5vdGUgYWxz byB0aGF0IHRob3VnaCBBSSBpcyBob2xkaW5nIHRoZSBjZWxsTG9jayBpbiB0aGlzIGNhc2UsIHdl IHN0aWxsIG5lZWQgdG8KKyAgICAgICAgZW5zdXJlIHRoYXQgdGhlIGFycmF5IHN0cnVjdHVyZSBo YXNuJ3QgY2hhbmdlZCBhcm91bmQgdGhlIGZldGNoaW5nIG9mIHRoZSBidXR0ZXJmbHkuCisgICAg ICAgIFRoaXMgaXMgYmVjYXVzZSBvcGVyYXRpb25zIG90aGVyIHRoYW4gdW5zaGlmdCBhbmQgc2hp ZnQgYXJlIGd1YXJkZWQgYnkgdGhpcworICAgICAgICBwcm90b2NvbCwgYW5kIG5vdCB0aGUgY2Vs bExvY2suCisKKyAgICAgICAgKiBkZmcvREZHQWJzdHJhY3RJbnRlcnByZXRlcklubGluZXMuaDoK KyAgICAgICAgKEpTQzo6REZHOjpBYnN0cmFjdEludGVycHJldGVyPEFic3RyYWN0U3RhdGVUeXBl Pjo6ZXhlY3V0ZUVmZmVjdHMpOgorICAgICAgICAqIHJ1bnRpbWUvSlNBcnJheS5jcHA6CisgICAg ICAgIChKU0M6OkpTQXJyYXk6OnVuc2hpZnRDb3VudFNsb3dDYXNlKToKKwogMjAxOC0xMi0xNyAg TWFyayBMYW0gIDxtYXJrLmxhbUBhcHBsZS5jb20+CiAKICAgICAgICAgU2FtcGxpbmdQcm9maWxl cidzIGlzVmFsaWRGcmFtZVBvaW50ZXIoKSBzaG91bGQgcmVqZWN0IGFkZHJlc3MgYXQgc3RhY2sg b3JpZ2luLgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdBYnN0cmFjdEludGVy cHJldGVySW5saW5lcy5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcv REZHQWJzdHJhY3RJbnRlcnByZXRlcklubGluZXMuaAkocmV2aXNpb24gMjM5MzEyKQorKysgU291 cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdBYnN0cmFjdEludGVycHJldGVySW5saW5lcy5oCSh3 b3JraW5nIGNvcHkpCkBAIC0xOTAwLDE3ICsxOTAwLDE5IEBAIGJvb2wgQWJzdHJhY3RJbnRlcnBy ZXRlcjxBYnN0cmFjdFN0YXRlVHkKICAgICAgICAgICAgICAgICBpZiAoaXNOdWtlZChzdHJ1Y3R1 cmVJREVhcmx5KSkKICAgICAgICAgICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwogCi0gICAgICAg ICAgICAgICAgV1RGOjpsb2FkTG9hZEZlbmNlKCk7Ci0gICAgICAgICAgICAgICAgQnV0dGVyZmx5 KiBidXR0ZXJmbHkgPSBhcnJheS0+YnV0dGVyZmx5KCk7CisgICAgICAgICAgICAgICAgaWYgKG5v ZGUtPmFycmF5TW9kZSgpLmFycmF5Q2xhc3MoKSA9PSBBcnJheTo6T3JpZ2luYWxDb3B5T25Xcml0 ZUFycmF5KSB7CiAKLSAgICAgICAgICAgICAgICBXVEY6OmxvYWRMb2FkRmVuY2UoKTsKLSAgICAg ICAgICAgICAgICBTdHJ1Y3R1cmVJRCBzdHJ1Y3R1cmVJRExhdGUgPSBhcnJheS0+c3RydWN0dXJl SUQoKTsKKyAgICAgICAgICAgICAgICAgICAgV1RGOjpsb2FkTG9hZEZlbmNlKCk7CisgICAgICAg ICAgICAgICAgICAgIEJ1dHRlcmZseSogYnV0dGVyZmx5ID0gYXJyYXktPmJ1dHRlcmZseSgpOwog Ci0gICAgICAgICAgICAgICAgaWYgKHN0cnVjdHVyZUlERWFybHkgIT0gc3RydWN0dXJlSURMYXRl KQotICAgICAgICAgICAgICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgICAgICAgICAgICAgICAg IFdURjo6bG9hZExvYWRGZW5jZSgpOworICAgICAgICAgICAgICAgICAgICBTdHJ1Y3R1cmVJRCBz dHJ1Y3R1cmVJRExhdGUgPSBhcnJheS0+c3RydWN0dXJlSUQoKTsKKworICAgICAgICAgICAgICAg ICAgICBpZiAoc3RydWN0dXJlSURFYXJseSAhPSBzdHJ1Y3R1cmVJRExhdGUpCisgICAgICAgICAg ICAgICAgICAgICAgICByZXR1cm4gZmFsc2U7CisKKyAgICAgICAgICAgICAgICAgICAgU3RydWN0 dXJlKiBzdHJ1Y3R1cmUgPSBtX3ZtLmdldFN0cnVjdHVyZShzdHJ1Y3R1cmVJRExhdGUpOwogCi0g ICAgICAgICAgICAgICAgU3RydWN0dXJlKiBzdHJ1Y3R1cmUgPSBtX3ZtLmdldFN0cnVjdHVyZShz dHJ1Y3R1cmVJRExhdGUpOwotICAgICAgICAgICAgICAgIGlmIChub2RlLT5hcnJheU1vZGUoKS5h cnJheUNsYXNzKCkgPT0gQXJyYXk6Ok9yaWdpbmFsQ29weU9uV3JpdGVBcnJheSkgewogICAgICAg ICAgICAgICAgICAgICBpZiAoIWlzQ29weU9uV3JpdGUoc3RydWN0dXJlLT5pbmRleGluZ01vZGUo KSkpCiAgICAgICAgICAgICAgICAgICAgICAgICByZXR1cm4gZmFsc2U7CiAKQEAgLTE5NDMsMTcg KzE5NDUsMjcgQEAgYm9vbCBBYnN0cmFjdEludGVycHJldGVyPEFic3RyYWN0U3RhdGVUeQogICAg ICAgICAgICAgICAgIH0KIAogICAgICAgICAgICAgICAgIGlmIChub2RlLT5hcnJheU1vZGUoKS50 eXBlKCkgPT0gQXJyYXk6OkFycmF5U3RvcmFnZSB8fCBub2RlLT5hcnJheU1vZGUoKS50eXBlKCkg PT0gQXJyYXk6OlNsb3dQdXRBcnJheVN0b3JhZ2UpIHsKLSAgICAgICAgICAgICAgICAgICAgaWYg KCFoYXNBbnlBcnJheVN0b3JhZ2Uoc3RydWN0dXJlLT5pbmRleGluZ01vZGUoKSkpCi0gICAgICAg ICAgICAgICAgICAgICAgICByZXR1cm4gZmFsc2U7Ci0KLSAgICAgICAgICAgICAgICAgICAgaWYg KHN0cnVjdHVyZS0+dHlwZUluZm8oKS5pbnRlcmNlcHRzR2V0T3duUHJvcGVydHlTbG90QnlJbmRl eEV2ZW5XaGVuTGVuZ3RoSXNOb3RaZXJvKCkpCi0gICAgICAgICAgICAgICAgICAgICAgICByZXR1 cm4gZmFsc2U7Ci0KICAgICAgICAgICAgICAgICAgICAgSlNWYWx1ZSB2YWx1ZTsKICAgICAgICAg ICAgICAgICAgICAgewogICAgICAgICAgICAgICAgICAgICAgICAgLy8gQXJyYXlTdG9yYWdlJ3Mg QnV0dGVyZmx5IGNhbiBiZSBoYWxmLWJyb2tlbiBzdGF0ZS4KICAgICAgICAgICAgICAgICAgICAg ICAgIGF1dG8gbG9ja2VyID0gaG9sZExvY2soYXJyYXktPmNlbGxMb2NrKCkpOwogCisgICAgICAg ICAgICAgICAgICAgICAgICBXVEY6OmxvYWRMb2FkRmVuY2UoKTsKKyAgICAgICAgICAgICAgICAg ICAgICAgIEJ1dHRlcmZseSogYnV0dGVyZmx5ID0gYXJyYXktPmJ1dHRlcmZseSgpOworCisgICAg ICAgICAgICAgICAgICAgICAgICBXVEY6OmxvYWRMb2FkRmVuY2UoKTsKKyAgICAgICAgICAgICAg ICAgICAgICAgIFN0cnVjdHVyZUlEIHN0cnVjdHVyZUlETGF0ZSA9IGFycmF5LT5zdHJ1Y3R1cmVJ RCgpOworCisgICAgICAgICAgICAgICAgICAgICAgICBpZiAoc3RydWN0dXJlSURFYXJseSAhPSBz dHJ1Y3R1cmVJRExhdGUpCisgICAgICAgICAgICAgICAgICAgICAgICAgICAgcmV0dXJuIGZhbHNl OworCisgICAgICAgICAgICAgICAgICAgICAgICBTdHJ1Y3R1cmUqIHN0cnVjdHVyZSA9IG1fdm0u Z2V0U3RydWN0dXJlKHN0cnVjdHVyZUlETGF0ZSk7CisgICAgICAgICAgICAgICAgICAgICAgICBp ZiAoIWhhc0FueUFycmF5U3RvcmFnZShzdHJ1Y3R1cmUtPmluZGV4aW5nTW9kZSgpKSkKKyAgICAg ICAgICAgICAgICAgICAgICAgICAgICByZXR1cm4gZmFsc2U7CisKKyAgICAgICAgICAgICAgICAg ICAgICAgIGlmIChzdHJ1Y3R1cmUtPnR5cGVJbmZvKCkuaW50ZXJjZXB0c0dldE93blByb3BlcnR5 U2xvdEJ5SW5kZXhFdmVuV2hlbkxlbmd0aElzTm90WmVybygpKQorICAgICAgICAgICAgICAgICAg ICAgICAgICAgIHJldHVybiBmYWxzZTsKKwogICAgICAgICAgICAgICAgICAgICAgICAgQXJyYXlT dG9yYWdlKiBzdG9yYWdlID0gYnV0dGVyZmx5LT5hcnJheVN0b3JhZ2UoKTsKICAgICAgICAgICAg ICAgICAgICAgICAgIGlmIChpbmRleCA+PSBzdG9yYWdlLT5sZW5ndGgoKSkKICAgICAgICAgICAg ICAgICAgICAgICAgICAgICByZXR1cm4gZmFsc2U7CkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENv cmUvcnVudGltZS9KU0FycmF5LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENv cmUvcnVudGltZS9KU0FycmF5LmNwcAkocmV2aXNpb24gMjM5MzEyKQorKysgU291cmNlL0phdmFT Y3JpcHRDb3JlL3J1bnRpbWUvSlNBcnJheS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTMzNCw2ICsz MzQsOCBAQCB2b2lkIEpTQXJyYXk6OmdldE93bk5vbkluZGV4UHJvcGVydHlOYW1lCiAvLyBUaGlz IG1ldGhvZCBtYWtlcyByb29tIGluIHRoZSB2ZWN0b3IsIGJ1dCBsZWF2ZXMgdGhlIG5ldyBzcGFj ZSBmb3IgY291bnQgc2xvdHMgdW5jbGVhcmVkLgogYm9vbCBKU0FycmF5Ojp1bnNoaWZ0Q291bnRT bG93Q2FzZShjb25zdCBBYnN0cmFjdExvY2tlciYsIFZNJiB2bSwgRGVmZXJHQyYsIGJvb2wgYWRk VG9Gcm9udCwgdW5zaWduZWQgY291bnQpCiB7CisgICAgQVNTRVJUKGNlbGxMb2NrKCkuaXNMb2Nr ZWQoKSk7CisKICAgICBBcnJheVN0b3JhZ2UqIHN0b3JhZ2UgPSBlbnN1cmVBcnJheVN0b3JhZ2Uo dm0pOwogICAgIEJ1dHRlcmZseSogYnV0dGVyZmx5ID0gc3RvcmFnZS0+YnV0dGVyZmx5KCk7CiAg ICAgU3RydWN0dXJlKiBzdHJ1Y3R1cmUgPSB0aGlzLT5zdHJ1Y3R1cmUodm0pOwo=