From b069315832877bc258aa5ad3beae27f1c196f239 Mon Sep 17 00:00:00 2001 From: Franziska Hinkelmann <franzih@chromium.org> Date: Mon, 11 Sep 2017 12:47:20 +0200 Subject: [PATCH] [type-profile] Use shared_ptr instead of raw pointer If TypeProfile goes out of scope, ScriptData and Entry still rely on TypeProfiles's type_profile_. Make type_profile_ a shared_ptr owned by all three classes to prevent use after free. Bug: v8:5933 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: Ida7d66dadc17a816cf4439a25e6f714edccffa2c Reviewed-on: https://chromium-review.googlesource.com/659937 Reviewed-by: Yang Guo <yangguo@chromium.org> Commit-Queue: Franziska Hinkelmann <franzih@chromium.org> Cr-Commit-Position: refs/heads/master@{#48013} --- src/api.cc | 17 ++++++++--------- src/debug/debug-interface.h | 23 +++++++++++++++-------- src/debug/debug-type-profile.cc | 4 ++-- src/debug/debug-type-profile.h | 2 +- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/api.cc b/src/api.cc index 24507138df..fe02f56790 100644 --- a/src/api.cc +++ b/src/api.cc @@ -10182,9 +10182,6 @@ debug::Coverage::FunctionData debug::Coverage::ScriptData::GetFunctionData( return FunctionData(&script_->functions.at(i), coverage_); } -debug::Coverage::Coverage(std::shared_ptr<i::Coverage> coverage) - : coverage_(std::move(coverage)) {} - debug::Coverage::ScriptData::ScriptData(size_t index, std::shared_ptr<i::Coverage> coverage) : script_(&coverage->at(index)), coverage_(std::move(coverage)) {} @@ -10216,11 +10213,16 @@ int debug::TypeProfile::Entry::SourcePosition() const { std::vector<MaybeLocal<String>> debug::TypeProfile::Entry::Types() const { std::vector<MaybeLocal<String>> result; for (const internal::Handle<internal::String>& type : entry_->types) { - result.push_back(ToApiHandle<String>(type)); + result.emplace_back(ToApiHandle<String>(type)); } return result; } +debug::TypeProfile::ScriptData::ScriptData( + size_t index, std::shared_ptr<i::TypeProfile> type_profile) + : script_(&type_profile->at(index)), + type_profile_(std::move(type_profile)) {} + Local<debug::Script> debug::TypeProfile::ScriptData::GetScript() const { return ToApiHandle<debug::Script>(script_->script); } @@ -10229,7 +10231,7 @@ std::vector<debug::TypeProfile::Entry> debug::TypeProfile::ScriptData::Entries() const { std::vector<debug::TypeProfile::Entry> result; for (const internal::TypeProfileEntry& entry : script_->entries) { - result.push_back(debug::TypeProfile::Entry(&entry)); + result.push_back(debug::TypeProfile::Entry(&entry, type_profile_)); } return result; } @@ -10248,12 +10250,9 @@ size_t debug::TypeProfile::ScriptCount() const { return type_profile_->size(); } debug::TypeProfile::ScriptData debug::TypeProfile::GetScriptData( size_t i) const { - // TODO(franzih): ScriptData is invalid after ~TypeProfile. Same in Coverage. - return ScriptData(&type_profile_->at(i)); + return ScriptData(i, type_profile_); } -debug::TypeProfile::~TypeProfile() { delete type_profile_; } - const char* CpuProfileNode::GetFunctionNameStr() const { const i::ProfileNode* node = reinterpret_cast<const i::ProfileNode*>(this); return node->entry()->name(); diff --git a/src/debug/debug-interface.h b/src/debug/debug-interface.h index 151b0e75db..cc321ebfa2 100644 --- a/src/debug/debug-interface.h +++ b/src/debug/debug-interface.h @@ -341,7 +341,8 @@ class V8_EXPORT_PRIVATE Coverage { bool IsEmpty() const { return coverage_ == nullptr; } private: - explicit Coverage(std::shared_ptr<i::Coverage> coverage); + explicit Coverage(std::shared_ptr<i::Coverage> coverage) + : coverage_(std::move(coverage)) {} std::shared_ptr<i::Coverage> coverage_; }; @@ -366,8 +367,12 @@ class V8_EXPORT_PRIVATE TypeProfile { std::vector<MaybeLocal<String>> Types() const; private: - explicit Entry(const i::TypeProfileEntry* entry) : entry_(entry) {} + explicit Entry(const i::TypeProfileEntry* entry, + std::shared_ptr<i::TypeProfile> type_profile) + : entry_(entry), type_profile_(std::move(type_profile)) {} + const i::TypeProfileEntry* entry_; + std::shared_ptr<i::TypeProfile> type_profile_; friend class v8::debug::TypeProfile::ScriptData; }; @@ -380,8 +385,11 @@ class V8_EXPORT_PRIVATE TypeProfile { std::vector<Entry> Entries() const; private: - explicit ScriptData(i::TypeProfileScript* script) : script_(script) {} + explicit ScriptData(size_t index, + std::shared_ptr<i::TypeProfile> type_profile); + i::TypeProfileScript* script_; + std::shared_ptr<i::TypeProfile> type_profile_; friend class v8::debug::TypeProfile; }; @@ -393,12 +401,11 @@ class V8_EXPORT_PRIVATE TypeProfile { size_t ScriptCount() const; ScriptData GetScriptData(size_t i) const; - ~TypeProfile(); - private: - explicit TypeProfile(i::TypeProfile* type_profile) - : type_profile_(type_profile) {} - i::TypeProfile* type_profile_; + explicit TypeProfile(std::shared_ptr<i::TypeProfile> type_profile) + : type_profile_(std::move(type_profile)) {} + + std::shared_ptr<i::TypeProfile> type_profile_; }; class ScopeIterator { diff --git a/src/debug/debug-type-profile.cc b/src/debug/debug-type-profile.cc index 1a8d4178ce..ef4f5ba3d7 100644 --- a/src/debug/debug-type-profile.cc +++ b/src/debug/debug-type-profile.cc @@ -12,8 +12,8 @@ namespace v8 { namespace internal { -TypeProfile* TypeProfile::Collect(Isolate* isolate) { - TypeProfile* result = new TypeProfile(); +std::unique_ptr<TypeProfile> TypeProfile::Collect(Isolate* isolate) { + std::unique_ptr<TypeProfile> result(new TypeProfile()); // Collect existing feedback vectors. std::vector<Handle<FeedbackVector>> feedback_vectors; diff --git a/src/debug/debug-type-profile.h b/src/debug/debug-type-profile.h index 221a047b28..de18951381 100644 --- a/src/debug/debug-type-profile.h +++ b/src/debug/debug-type-profile.h @@ -32,7 +32,7 @@ struct TypeProfileScript { class TypeProfile : public std::vector<TypeProfileScript> { public: - static TypeProfile* Collect(Isolate* isolate); + static std::unique_ptr<TypeProfile> Collect(Isolate* isolate); static void SelectMode(Isolate* isolate, debug::TypeProfile::Mode mode); private: