167208 2017-01-19 12:06:34 -0800 The mutator needs to fire a barrier after memmoving stuff around in an object that the GC scans 2017-01-19 12:55:16 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 fpizlo fpizlo webkit-bug-importer oldest_to_newest 1268149 0 fpizlo 2017-01-19 12:06:34 -0800 We didn't used to need these kinds of barriers, but now we do! It used to be that if you moved a value from one place to another in the same object then there is no need for a barrier because the generational GC would have no need to know that the referent still continues to refer to the same referee. But the concurrent GC might scan that object as the mutator moves pointers around in it. If the ordering is right, this could mean that the collector never sees some of those pointers. This can be fixed by adding a barrier. This bug covers the most obvious cases I found. There may be more and I'll continue to audit. 1268150 1 299257 fpizlo 2017-01-19 12:09:24 -0800 Created attachment 299257 the patch 1268151 2 webkit-bug-importer 2017-01-19 12:10:21 -0800 <rdar://problem/30101860> 1268161 3 299257 saam 2017-01-19 12:39:11 -0800 Comment on attachment 299257 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=299257&action=review > Source/JavaScriptCore/runtime/JSArray.cpp:1027 > + vm.heap.writeBarrier(this); Maybe just do this if we're Contiguous? > Source/JavaScriptCore/runtime/JSArray.cpp:1179 > + vm.heap.writeBarrier(this); Maybe just do this if we're Contiguous? 1268162 4 fpizlo 2017-01-19 12:40:27 -0800 (In reply to comment #3) > Comment on attachment 299257 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=299257&action=review > > > Source/JavaScriptCore/runtime/JSArray.cpp:1027 > > + vm.heap.writeBarrier(this); > > Maybe just do this if we're Contiguous? > > > Source/JavaScriptCore/runtime/JSArray.cpp:1179 > > + vm.heap.writeBarrier(this); > > Maybe just do this if we're Contiguous? That would be a branch, and the barrier's amortized cost is about the cost of a branch. 1268165 5 fpizlo 2017-01-19 12:55:16 -0800 Landed in https://trac.webkit.org/changeset/210935 299257 2017-01-19 12:09:24 -0800 2017-01-19 12:39:11 -0800 the patch blah.patch text/plain 3045 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjEwOTMzKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI4IEBA CisyMDE3LTAxLTE5ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg VGhlIG11dGF0b3IgbmVlZHMgdG8gZmlyZSBhIGJhcnJpZXIgYWZ0ZXIgbWVtbW92aW5nIHN0dWZm IGFyb3VuZCBpbiBhbiBvYmplY3QgdGhhdCB0aGUgR0Mgc2NhbnMKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE2NzIwOAorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorICAgICAgICAKKyAgICAgICAgSXQgdXNlZCB0byBiZSB0aGF0 IGlmIHlvdSBtb3ZlZCBhIHZhbHVlIGZyb20gb25lIHBsYWNlIHRvIGFub3RoZXIgaW4gdGhlIHNh bWUgb2JqZWN0CisgICAgICAgIHRoZW4gdGhlcmUgaXMgbm8gbmVlZCBmb3IgYSBiYXJyaWVyIGJl Y2F1c2UgdGhlIGdlbmVyYXRpb25hbCBHQyB3b3VsZCBoYXZlIG5vIG5lZWQgdG8KKyAgICAgICAg a25vdyB0aGF0IHNvbWUgb2xkIG9iamVjdCBzdGlsbCBjb250aW51ZXMgdG8gcmVmZXIgdG8gdGhl IHNhbWUgb3RoZXIgb2xkIG9iamVjdC4KKworICAgICAgICBCdXQgdGhlIGNvbmN1cnJlbnQgR0Mg bWlnaHQgc2NhbiB0aGF0IG9iamVjdCBhcyB0aGUgbXV0YXRvciBtb3ZlcyBwb2ludGVycyBhcm91 bmQgaW4KKyAgICAgICAgaXQuIElmIHRoZSBvcmRlcmluZyBpcyByaWdodCwgdGhpcyBjb3VsZCBt ZWFuIHRoYXQgdGhlIGNvbGxlY3RvciBuZXZlciBzZWVzIHNvbWUgb2YKKyAgICAgICAgdGhvc2Ug cG9pbnRlcnMuIFRoaXMgY2FuIGJlIGZpeGVkIGJ5IGFkZGluZyBhIGJhcnJpZXIuCisKKyAgICAg ICAgVGhpcyBmaXhlcyB0aGUgbW9zdCBvYnZpb3VzIGNhc2VzIEkgZm91bmQuIFRoZXJlIG1heSBi ZSBtb3JlIGFuZCBJJ2xsIGNvbnRpbnVlIHRvCisgICAgICAgIGF1ZGl0LiBNb3N0IG9mIHRoZSBv dGhlciBtZW1tb3ZlIHVzZXJzIHNlZW0gdG8gYWxyZWFkeSB1c2Ugc29tZSBraW5kIG9mIHN5bmNo cm9uaXphdGlvbgorICAgICAgICB0byBwcmV2ZW50IHRoaXMuIEZvciBleGFtcGxlLCB0aGlzIGNh biBhbHNvIGJlIGZpeGVkIGJ5IGp1c3QgaG9sZGluZyB0aGUgY2VsbCBsb2NrCisgICAgICAgIGFy b3VuZCB0aGUgbWVtbW92ZSBzaW5jZSB3ZSdyZSBkZWFsaW5nIHdpdGggaW5kZXhpbmcgc3RvcmFn ZSBhbmQgdGhlIEdDIHJlYWRzIHRoYXQKKyAgICAgICAgdW5kZXIgdGhlIGNlbGwgbG9jay4KKwor ICAgICAgICAqIHJ1bnRpbWUvSlNBcnJheS5jcHA6CisgICAgICAgIChKU0M6OkpTQXJyYXk6OnNo aWZ0Q291bnRXaXRoQW55SW5kZXhpbmdUeXBlKToKKyAgICAgICAgKEpTQzo6SlNBcnJheTo6dW5z aGlmdENvdW50V2l0aEFueUluZGV4aW5nVHlwZSk6CisKIDIwMTctMDEtMTkgIE15bGVzIEMuIE1h eGZpZWxkICA8bW1heGZpZWxkQGFwcGxlLmNvbT4KIAogICAgICAgICBbQ29jb2FdIFZhcmlhdGlv biBmb250cyBhcmUgZXJyb25lb3VzbHkgZGlzYWJsZWQgb24gaU9TCkluZGV4OiBTb3VyY2UvSmF2 YVNjcmlwdENvcmUvcnVudGltZS9KU0FycmF5LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2 YVNjcmlwdENvcmUvcnVudGltZS9KU0FycmF5LmNwcAkocmV2aXNpb24gMjEwNTUzKQorKysgU291 cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNBcnJheS5jcHAJKHdvcmtpbmcgY29weSkKQEAg LTEwMjEsNiArMTAyMSwxMSBAQCBib29sIEpTQXJyYXk6OnNoaWZ0Q291bnRXaXRoQW55SW5kZXhp bmdUCiAgICAgICAgICAgICBidXR0ZXJmbHktPmNvbnRpZ3VvdXMoKVtpXS5jbGVhcigpOwogICAg ICAgICAKICAgICAgICAgYnV0dGVyZmx5LT5zZXRQdWJsaWNMZW5ndGgob2xkTGVuZ3RoIC0gY291 bnQpOworCisgICAgICAgIC8vIE91ciBtZW1tb3Zpbmcgb2YgdmFsdWVzIGFyb3VuZCBpbiB0aGUg YXJyYXkgY291bGQgaGF2ZSBjb25jZWFsZWQgc29tZSBvZiB0aGVtIGZyb20KKyAgICAgICAgLy8g dGhlIGNvbGxlY3Rvci4gTGV0J3MgbWFrZSBzdXJlIHRoYXQgdGhlIGNvbGxlY3RvciBzY2FucyB0 aGlzIG9iamVjdCBhZ2Fpbi4KKyAgICAgICAgdm0uaGVhcC53cml0ZUJhcnJpZXIodGhpcyk7Cisg ICAgICAgIAogICAgICAgICByZXR1cm4gdHJ1ZTsKICAgICB9CiAgICAgICAgIApAQCAtMTE2OSw2 ICsxMTc0LDEwIEBAIGJvb2wgSlNBcnJheTo6dW5zaGlmdENvdW50V2l0aEFueUluZGV4aW4KICAg ICAgICAgICAgIGJ1dHRlcmZseS0+Y29udGlndW91cygpW2kgKyBjb3VudF0uc2V0V2l0aG91dFdy aXRlQmFycmllcih2KTsKICAgICAgICAgfQogICAgICAgICAKKyAgICAgICAgLy8gT3VyIG1lbW1v dmluZyBvZiB2YWx1ZXMgYXJvdW5kIGluIHRoZSBhcnJheSBjb3VsZCBoYXZlIGNvbmNlYWxlZCBz b21lIG9mIHRoZW0gZnJvbQorICAgICAgICAvLyB0aGUgY29sbGVjdG9yLiBMZXQncyBtYWtlIHN1 cmUgdGhhdCB0aGUgY29sbGVjdG9yIHNjYW5zIHRoaXMgb2JqZWN0IGFnYWluLgorICAgICAgICB2 bS5oZWFwLndyaXRlQmFycmllcih0aGlzKTsKKyAgICAgICAgCiAgICAgICAgIC8vIE5PVEU6IHdl J3JlIGxlYXZpbmcgYmVpbmcgZ2FyYmFnZSBpbiB0aGUgcGFydCBvZiB0aGUgYXJyYXkgdGhhdCB3 ZSBzaGlmdGVkIG91dAogICAgICAgICAvLyBvZi4gVGhpcyBpcyBmaW5lIGJlY2F1c2UgdGhlIGNh bGxlciBpcyByZXF1aXJlZCB0byBzdG9yZSBvdmVyIHRoYXQgYXJlYSwgYW5kCiAgICAgICAgIC8v IGluIGNvbnRpZ3VvdXMgbW9kZSBzdG9yaW5nIGludG8gYSBob2xlIGlzIGd1YXJhbnRlZWQgdG8g YmVoYXZlIGV4YWN0bHkgdGhlIHNhbWUK