From 75993ee9fc4341d73887a91c0b5de74045f853b6 Mon Sep 17 00:00:00 2001 From: "fpizlo@apple.com" Date: Tue, 23 Feb 2016 22:17:24 +0000 Subject: [PATCH] B3::Value doesn't self-destruct virtually enough (Causes many leaks in LowerDFGToB3::appendOSRExit) https://bugs.webkit.org/show_bug.cgi?id=154592 Reviewed by Saam Barati. If Foo has a virtual destructor, then: foo->Foo::~Foo() does a non-virtual call to Foo's destructor. Even if foo points to a subclass of Foo that overrides the destructor, this syntax will not call that override. foo->~Foo() does a virtual call to the destructor, and so if foo points to a subclass, you get the subclass's override. In B3, we used this->Value::~Value() thinking that it would call the subclass's override. This caused leaks because this didn't actually call the subclass's override. This fixes the problem by using this->~Value() instead. * b3/B3ControlValue.cpp: (JSC::B3::ControlValue::convertToJump): (JSC::B3::ControlValue::convertToOops): * b3/B3Value.cpp: (JSC::B3::Value::replaceWithIdentity): (JSC::B3::Value::replaceWithNop): (JSC::B3::Value::replaceWithPhi): git-svn-id: https://svn.webkit.org/repository/webkit/trunk@196996 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/JavaScriptCore/ChangeLog | 27 +++++++++++++++++++++++++++ Source/JavaScriptCore/b3/B3ControlValue.cpp | 4 ++-- Source/JavaScriptCore/b3/B3Value.cpp | 6 +++--- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog index 801cfca..7b176d0 100644 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,30 @@ +2016-02-23 Filip Pizlo + + B3::Value doesn't self-destruct virtually enough (Causes many leaks in LowerDFGToB3::appendOSRExit) + https://bugs.webkit.org/show_bug.cgi?id=154592 + + Reviewed by Saam Barati. + + If Foo has a virtual destructor, then: + + foo->Foo::~Foo() does a non-virtual call to Foo's destructor. Even if foo points to a + subclass of Foo that overrides the destructor, this syntax will not call that override. + + foo->~Foo() does a virtual call to the destructor, and so if foo points to a subclass, you + get the subclass's override. + + In B3, we used this->Value::~Value() thinking that it would call the subclass's override. + This caused leaks because this didn't actually call the subclass's override. This fixes the + problem by using this->~Value() instead. + + * b3/B3ControlValue.cpp: + (JSC::B3::ControlValue::convertToJump): + (JSC::B3::ControlValue::convertToOops): + * b3/B3Value.cpp: + (JSC::B3::Value::replaceWithIdentity): + (JSC::B3::Value::replaceWithNop): + (JSC::B3::Value::replaceWithPhi): + 2016-02-23 Brian Burg Web Inspector: the protocol generator's Objective-C name prefix should be configurable diff --git a/Source/JavaScriptCore/b3/B3ControlValue.cpp b/Source/JavaScriptCore/b3/B3ControlValue.cpp index 2dd9f94..cbf474a 100644 --- a/Source/JavaScriptCore/b3/B3ControlValue.cpp +++ b/Source/JavaScriptCore/b3/B3ControlValue.cpp @@ -57,7 +57,7 @@ void ControlValue::convertToJump(BasicBlock* destination) Origin origin = this->origin(); BasicBlock* owner = this->owner; - this->ControlValue::~ControlValue(); + this->~ControlValue(); new (this) ControlValue(Jump, origin, FrequentedBlock(destination)); @@ -71,7 +71,7 @@ void ControlValue::convertToOops() Origin origin = this->origin(); BasicBlock* owner = this->owner; - this->ControlValue::~ControlValue(); + this->~ControlValue(); new (this) ControlValue(Oops, origin); diff --git a/Source/JavaScriptCore/b3/B3Value.cpp b/Source/JavaScriptCore/b3/B3Value.cpp index 1c6d9bc..c028dad 100644 --- a/Source/JavaScriptCore/b3/B3Value.cpp +++ b/Source/JavaScriptCore/b3/B3Value.cpp @@ -71,7 +71,7 @@ void Value::replaceWithIdentity(Value* value) RELEASE_ASSERT(type == value->type()); - this->Value::~Value(); + this->~Value(); new (this) Value(Identity, type, origin, value); @@ -85,7 +85,7 @@ void Value::replaceWithNop() Origin origin = m_origin; BasicBlock* owner = this->owner; - this->Value::~Value(); + this->~Value(); new (this) Value(Nop, Void, origin); @@ -105,7 +105,7 @@ void Value::replaceWithPhi() BasicBlock* owner = this->owner; Type type = m_type; - this->Value::~Value(); + this->~Value(); new (this) Value(Phi, type, origin); -- 1.8.3.1