73982 2011-12-06 21:10:20 -0800 Zapping a block that is Marked leads to dead objects being mistaken for live ones 2011-12-06 21:26:50 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 fpizlo webkit-unassigned oldest_to_newest 516484 0 fpizlo 2011-12-06 21:10:20 -0800 Consider the following chain of state transitions for some MarkedBlock M, containing object O, and two collection cycles C1 and C2. 1) Allocation causes M to be swept, and ending up in the FreeListed state. 2) GC cycle C1 starts with M still being the currentBlock for its size class. M first transitions FreeListed->Zapped and then Zapped->Marked. C1 does not mark O, leading it to have 0 in the mark bit. M happens to also be the first block in its size class, so currentBlock still refers to M. 3) GC cycle C2 starts with M still being the currentBlock. No allocations have occurred in M between C1 and C2. So, M is zapped again. But this time, its free list is empty and its non-free-listed objects may be dead from the previous collection. This is true of O, which has a clear mark bit, but is not on the free list, since M could not possibly have a free list. (Free lists are only created when a sweep occurs, and a sweep only occurs if you allocate, and nobody allocated in M or M's size class.) 4) We now have a scenario that is ripe for disaster: O has a non-zero vtable pointer (hence, it is not zapped) and a clear mark bit, but the block thinks it is Zapped. Hence conservative marking logic believes that O's non-zero vtable is proof that O is a valid object that was marked in the previous collection cycle, and hence must have valid outgoing references. But neither of these things is true; O is not a valid object (it is in fact dead) and does not have valid outgoing references, because, since it was not marked in C1, its outgoing references were not marked in C1, and hence its outgoing references may point to nonsense (free memory, other objects, or zapped objects). At this point, we are lucky of the GC completes safely. The solution is that zapping a block in the Marked state should not do anything. Only zapping a block in the FreeListed state should lead to the block becoming Zapped. 516488 1 118168 fpizlo 2011-12-06 21:15:22 -0800 Created attachment 118168 the patch 516489 2 118168 ggaren 2011-12-06 21:16:56 -0800 Comment on attachment 118168 the patch r=me 516490 3 fpizlo 2011-12-06 21:20:28 -0800 <rdar://problem/10539602> 516491 4 fpizlo 2011-12-06 21:26:29 -0800 Landed in http://trac.webkit.org/changeset/102220 516492 5 118168 fpizlo 2011-12-06 21:26:50 -0800 Comment on attachment 118168 the patch Clearing flags 118168 2011-12-06 21:15:22 -0800 2011-12-06 21:26:50 -0800 the patch fixzap_patch_1.diff text/plain 3751 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTAyMjE5KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI2IEBA CisyMDExLTEyLTA2ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg WmFwcGluZyBhIGJsb2NrIHRoYXQgaXMgTWFya2VkIGxlYWRzIHRvIGRlYWQgb2JqZWN0cyBiZWlu ZyBtaXN0YWtlbiBmb3IgbGl2ZSBvbmVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD03Mzk4MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorICAgICAgICAKKyAgICAgICAgQ2hhbmdlZCB0aGUgemFwcGluZyBjb2RlIHRvIGlnbm9y ZSBibG9ja3MgdGhhdCBhcmUgTWFya2VkIG9yIFphcHBlZC4gQWRkaXRpb25hbGx5LAorICAgICAg ICB0aGUgY29kZSBhc3NlcnRzIHRoYXQ6CisgICAgICAgIAorICAgICAgICAtIElmIHdlIHphcCBh IE1hcmtlZCBvciBaYXBwZWQgYmxvY2sgdGhlbiB0aGUgZnJlZSBsaXN0IGlzIGVtcHR5LCBiZWNh dXNlIHRoaXMKKyAgICAgICAgICBjYW4gb25seSBoYXBwZW4gaWYgdGhlIGJsb2NrIHdhcyBuZXZl ciBmcmVlLWxpc3RlZC4KKyAgICAgICAgICAKKyAgICAgICAgLSBaYXBwaW5nIGNhbiBvbmx5IGhh cHBlbiBmb3IgTWFya2VkLCBaYXBwZWQsIG9yIEZyZWVMaXN0ZWQgYmxvY2tzLCBzaW5jZSBBbGxv Y2F0ZWQKKyAgICAgICAgICBibG9ja3MgYXJlIHRob3NlIHRoYXQgY2Fubm90IGJlIHJlZmVycmVk IHRvIGJ5IFNpemVDbGFzczo6Y3VycmVudEJsb2NrIChzaW5jZQorICAgICAgICAgIFNpemVDbGFz czo6Y3VycmVudEJsb2NrIG9ubHkgcmVmZXJzIHRvIGJsb2NrcyB0aGF0IGFyZSBjYW5kaWRhdGVz IGZvciBhbGxvY2F0aW9uLAorICAgICAgICAgIGFuZCBBbGxvY2F0ZWQgYmxvY2tzIGFyZSB0aG9z ZSB3aG8gaGF2ZSBiZWVuIGV4aGF1c3RlZCBieSBhbGxvY2F0aW9uIGFuZCB3aWxsIG5vdAorICAg ICAgICAgIGJlIGFsbG9jYXRlZCBmcm9tIGFnYWluKSwgYW5kIE5ldyBibG9ja3MgY2Fubm90IGJl IHJlZmVycmVkIHRvIGJ5IGFueXRoaW5nIGV4Y2VwdAorICAgICAgICAgIGR1cmluZyBhIGJyaWVm IHdpbmRvdyBpbnNpZGUgdGhlIGFsbG9jYXRpb24gc2xvdy1wYXRoLgorCisgICAgICAgICogaGVh cC9NYXJrZWRCbG9jay5jcHA6CisgICAgICAgIChKU0M6Ok1hcmtlZEJsb2NrOjp6YXBGcmVlTGlz dCk6CisKIDIwMTEtMTItMDYgIEZpbGlwIFBpemxvICA8ZnBpemxvQGFwcGxlLmNvbT4KIAogICAg ICAgICBERkcgMzJfNjQgY2FsbCBsaW5raW5nIGRvZXMgbm90IGhhbmRsZSBub24tY2VsbCBjYWxs ZWVzIGNvcnJlY3RseQpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvTWFya2VkQmxv Y2suY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL01hcmtlZEJs b2NrLmNwcAkocmV2aXNpb24gMTAyMDM5KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAv TWFya2VkQmxvY2suY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNDEsMTYgKzE0MSw0NyBAQCB2b2lk IE1hcmtlZEJsb2NrOjp6YXBGcmVlTGlzdChGcmVlQ2VsbCogCiB7CiAgICAgSEVBUF9MT0dfQkxP Q0tfU1RBVEVfVFJBTlNJVElPTih0aGlzKTsKIAorICAgIGlmIChtX3N0YXRlID09IE1hcmtlZCkg eworICAgICAgICAvLyBJZiB0aGUgYmxvY2sgaXMgaW4gdGhlIE1hcmtlZCBzdGF0ZSB0aGVuIHdl IGtub3cgdGhhdDoKKyAgICAgICAgLy8gMSkgSXQgd2FzIG5vdCB1c2VkIGZvciBhbGxvY2F0aW9u IGR1cmluZyB0aGUgcHJldmlvdXMgYWxsb2NhdGlvbiBjeWNsZS4KKyAgICAgICAgLy8gMikgSXQg bWF5IGhhdmUgZGVhZCBvYmplY3RzLCBhbmQgd2Ugb25seSBrbm93IHRoZW0gdG8gYmUgZGVhZCBi eSB0aGUKKyAgICAgICAgLy8gICAgZmFjdCB0aGF0IHRoZWlyIG1hcmsgYml0cyBhcmUgdW5zZXQu CisgICAgICAgIC8vIEhlbmNlIGlmIHRoZSBibG9jayBpcyBNYXJrZWQgd2UgbmVlZCB0byBsZWF2 ZSBpdCBNYXJrZWQuCisgICAgICAgIAorICAgICAgICBBU1NFUlQoIWZpcnN0RnJlZUNlbGwpOwor ICAgICAgICAKKyAgICAgICAgcmV0dXJuOworICAgIH0KKyAgICAKKyAgICBpZiAobV9zdGF0ZSA9 PSBaYXBwZWQpIHsKKyAgICAgICAgLy8gSWYgdGhlIGJsb2NrIGlzIGluIHRoZSBaYXBwZWQgc3Rh dGUgdGhlbiB3ZSBrbm93IHRoYXQgc29tZW9uZSBhbHJlYWR5CisgICAgICAgIC8vIHphcHBlZCBp dCBmb3IgdXMuIFRoaXMgY291bGQgbm90IGhhdmUgaGFwcGVuZWQgZHVyaW5nIGEgR0MsIGJ1dCBt aWdodAorICAgICAgICAvLyBiZSB0aGUgcmVzdWx0IG9mIHNvbWVvbmUgaGF2aW5nIGRvbmUgYSBH QyBzY2FuIHRvIHBlcmZvcm0gc29tZSBvcGVyYXRpb24KKyAgICAgICAgLy8gb3ZlciBhbGwgbGl2 ZSBvYmplY3RzIChvciBhbGwgbGl2ZSBibG9ja3MpLiBJdCBhbHNvIG1lYW5zIHRoYXQgc29tZWJv ZHkKKyAgICAgICAgLy8gaGFkIGFsbG9jYXRlZCBpbiB0aGlzIGJsb2NrIHNpbmNlIHRoZSBsYXN0 IEdDLCBzd2VwdCBhbGwgZGVhZCBvYmplY3RzCisgICAgICAgIC8vIG9udG8gdGhlIGZyZWUgbGlz dCwgbGVmdCB0aGUgYmxvY2sgaW4gdGhlIEZyZWVMaXN0ZWQgc3RhdGUsIHRoZW4gdGhlIGhlYXAK KyAgICAgICAgLy8gc2NhbiBoYXBwZW5lZCwgYW5kIGNhbm9uaWNhbGl6ZWQgdGhlIGJsb2NrLCBs ZWFkaW5nIHRvIGFsbCBkZWFkIG9iamVjdHMKKyAgICAgICAgLy8gYmVpbmcgemFwcGVkLiBUaGVy ZWZvcmUsIGl0IGlzIHNhZmUgZm9yIHVzIHRvIHNpbXBseSBkbyBub3RoaW5nLCBzaW5jZQorICAg ICAgICAvLyBkZWFkIG9iamVjdHMgd2lsbCBoYXZlIDAgaW4gdGhlaXIgdnRhYmxlcyBhbmQgbGl2 ZSBvYmplY3RzIHdpbGwgaGF2ZQorICAgICAgICAvLyBub24temVybyB2dGFibGVzLCB3aGljaCBp cyBjb25zaXN0ZW50IHdpdGggdGhlIGJsb2NrIGJlaW5nIHphcHBlZC4KKyAgICAgICAgCisgICAg ICAgIEFTU0VSVCghZmlyc3RGcmVlQ2VsbCk7CisgICAgICAgIAorICAgICAgICByZXR1cm47Cisg ICAgfQorICAgIAorICAgIEFTU0VSVChtX3N0YXRlID09IEZyZWVMaXN0ZWQpOworICAgIAogICAg IC8vIFJvbGwgYmFjayB0byBhIGNvaGVyZW50IHN0YXRlIGZvciBIZWFwIGludHJvc3BlY3Rpb24u IENlbGxzIG5ld2x5CiAgICAgLy8gYWxsb2NhdGVkIGZyb20gb3VyIGZyZWUgbGlzdCBhcmUgbm90 IGN1cnJlbnRseSBtYXJrZWQsIHNvIHdlIG5lZWQgYW5vdGhlcgogICAgIC8vIHdheSB0byB0ZWxs IHdoYXQncyBsaXZlIHZzIGRlYWQuIFdlIHVzZSB6YXBwaW5nIGZvciB0aGF0LgotCisgICAgCiAg ICAgRnJlZUNlbGwqIG5leHQ7CiAgICAgZm9yIChGcmVlQ2VsbCogY3VycmVudCA9IGZpcnN0RnJl ZUNlbGw7IGN1cnJlbnQ7IGN1cnJlbnQgPSBuZXh0KSB7CiAgICAgICAgIG5leHQgPSBjdXJyZW50 LT5uZXh0OwogICAgICAgICByZWludGVycHJldF9jYXN0PEpTQ2VsbCo+KGN1cnJlbnQpLT56YXAo KTsKICAgICB9Ci0KKyAgICAKICAgICBtX3N0YXRlID0gWmFwcGVkOwogfQogCg==