Commit d55c644e authored by Jakob Linke's avatar Jakob Linke Committed by V8 LUCI CQ

[maglev] Fix leaks related to destructors and zone allocation

The zone-allocated objects, the destructor is never called. Such
objects must therefore never contain members that themselves have
non-trivial destructors, e.g. std containers.

Fix occurrences of this antipattern in Maglev.

Bug: v8:7700
Change-Id: I6892cf5203bb6e842397fd4292918b18134f97cc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3822672
Auto-Submit: Jakob Linke <jgruber@chromium.org>
Reviewed-by: 's avatarVictor Gomes <victorgomes@chromium.org>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82358}
parent 4e88cc71
...@@ -89,6 +89,14 @@ void MaglevCompilationInfo::set_graph_labeller( ...@@ -89,6 +89,14 @@ void MaglevCompilationInfo::set_graph_labeller(
graph_labeller_.reset(graph_labeller); graph_labeller_.reset(graph_labeller);
} }
void MaglevCompilationInfo::set_translation_array_builder(
std::unique_ptr<TranslationArrayBuilder> translation_array_builder,
std::unique_ptr<IdentityMap<int, base::DefaultAllocationPolicy>>
deopt_literals) {
translation_array_builder_ = std::move(translation_array_builder);
deopt_literals_ = std::move(deopt_literals);
}
void MaglevCompilationInfo::ReopenHandlesInNewHandleScope(Isolate* isolate) {} void MaglevCompilationInfo::ReopenHandlesInNewHandleScope(Isolate* isolate) {}
void MaglevCompilationInfo::set_persistent_handles( void MaglevCompilationInfo::set_persistent_handles(
......
...@@ -69,15 +69,15 @@ class MaglevCompilationInfo final { ...@@ -69,15 +69,15 @@ class MaglevCompilationInfo final {
Graph* graph() const { return graph_; } Graph* graph() const { return graph_; }
void set_translation_array_builder( void set_translation_array_builder(
TranslationArrayBuilder* translation_array_builder, std::unique_ptr<TranslationArrayBuilder> translation_array_builder,
IdentityMap<int, base::DefaultAllocationPolicy>* deopt_literals) { std::unique_ptr<IdentityMap<int, base::DefaultAllocationPolicy>>
translation_array_builder_ = translation_array_builder; deopt_literals);
deopt_literals_ = deopt_literals;
}
TranslationArrayBuilder& translation_array_builder() const { TranslationArrayBuilder& translation_array_builder() const {
DCHECK(translation_array_builder_);
return *translation_array_builder_; return *translation_array_builder_;
} }
IdentityMap<int, base::DefaultAllocationPolicy>& deopt_literals() const { IdentityMap<int, base::DefaultAllocationPolicy>& deopt_literals() const {
DCHECK(deopt_literals_);
return *deopt_literals_; return *deopt_literals_;
} }
...@@ -115,8 +115,9 @@ class MaglevCompilationInfo final { ...@@ -115,8 +115,9 @@ class MaglevCompilationInfo final {
// Produced off-thread during ExecuteJobImpl. // Produced off-thread during ExecuteJobImpl.
Graph* graph_ = nullptr; Graph* graph_ = nullptr;
TranslationArrayBuilder* translation_array_builder_ = nullptr; std::unique_ptr<TranslationArrayBuilder> translation_array_builder_;
IdentityMap<int, base::DefaultAllocationPolicy>* deopt_literals_ = nullptr; std::unique_ptr<IdentityMap<int, base::DefaultAllocationPolicy>>
deopt_literals_;
#define V(Name) const bool Name##_; #define V(Name) const bool Name##_;
MAGLEV_COMPILATION_FLAG_LIST(V) MAGLEV_COMPILATION_FLAG_LIST(V)
......
...@@ -239,20 +239,17 @@ class TranslationArrayProcessor { ...@@ -239,20 +239,17 @@ class TranslationArrayProcessor {
: local_isolate_(local_isolate) {} : local_isolate_(local_isolate) {}
void PreProcessGraph(MaglevCompilationInfo* compilation_info, Graph* graph) { void PreProcessGraph(MaglevCompilationInfo* compilation_info, Graph* graph) {
translation_array_builder_ = translation_array_builder_.reset(
compilation_info->zone()->New<TranslationArrayBuilder>( new TranslationArrayBuilder(compilation_info->zone()));
compilation_info->zone()); deopt_literals_.reset(new IdentityMap<int, base::DefaultAllocationPolicy>(
deopt_literals_ = local_isolate_->heap()->heap()));
compilation_info->zone()
->New<IdentityMap<int, base::DefaultAllocationPolicy>>(
local_isolate_->heap()->heap());
tagged_slots_ = graph->tagged_stack_slots(); tagged_slots_ = graph->tagged_stack_slots();
} }
void PostProcessGraph(MaglevCompilationInfo* compilation_info, Graph* graph) { void PostProcessGraph(MaglevCompilationInfo* compilation_info, Graph* graph) {
compilation_info->set_translation_array_builder(translation_array_builder_, compilation_info->set_translation_array_builder(
deopt_literals_); std::move(translation_array_builder_), std::move(deopt_literals_));
} }
void PreProcessBasicBlock(MaglevCompilationInfo*, BasicBlock* block) {} void PreProcessBasicBlock(MaglevCompilationInfo*, BasicBlock* block) {}
...@@ -528,8 +525,9 @@ class TranslationArrayProcessor { ...@@ -528,8 +525,9 @@ class TranslationArrayProcessor {
}; };
LocalIsolate* local_isolate_; LocalIsolate* local_isolate_;
TranslationArrayBuilder* translation_array_builder_; std::unique_ptr<TranslationArrayBuilder> translation_array_builder_;
IdentityMap<int, base::DefaultAllocationPolicy>* deopt_literals_; std::unique_ptr<IdentityMap<int, base::DefaultAllocationPolicy>>
deopt_literals_;
int tagged_slots_; int tagged_slots_;
}; };
......
...@@ -25,7 +25,13 @@ class Graph final : public ZoneObject { ...@@ -25,7 +25,13 @@ class Graph final : public ZoneObject {
static Graph* New(Zone* zone) { return zone->New<Graph>(zone); } static Graph* New(Zone* zone) { return zone->New<Graph>(zone); }
// Shouldn't be used directly; public so that Zone::New can access it. // Shouldn't be used directly; public so that Zone::New can access it.
explicit Graph(Zone* zone) : blocks_(zone) {} explicit Graph(Zone* zone)
: blocks_(zone),
root_(zone),
smi_(zone),
int_(zone),
float_(zone),
constants_(zone) {}
BasicBlock* operator[](int i) { return blocks_[i]; } BasicBlock* operator[](int i) { return blocks_[i]; }
const BasicBlock* operator[](int i) const { return blocks_[i]; } const BasicBlock* operator[](int i) const { return blocks_[i]; }
...@@ -54,11 +60,11 @@ class Graph final : public ZoneObject { ...@@ -54,11 +60,11 @@ class Graph final : public ZoneObject {
untagged_stack_slots_ = stack_slots; untagged_stack_slots_ = stack_slots;
} }
std::map<RootIndex, RootConstant*>& root() { return root_; } ZoneMap<RootIndex, RootConstant*>& root() { return root_; }
std::map<int, SmiConstant*>& smi() { return smi_; } ZoneMap<int, SmiConstant*>& smi() { return smi_; }
std::map<int, Int32Constant*>& int32() { return int_; } ZoneMap<int, Int32Constant*>& int32() { return int_; }
std::map<double, Float64Constant*>& float64() { return float_; } ZoneMap<double, Float64Constant*>& float64() { return float_; }
std::vector<Constant*>& constants() { return constants_; } ZoneVector<Constant*>& constants() { return constants_; }
Float64Constant* nan() const { return nan_; } Float64Constant* nan() const { return nan_; }
void set_nan(Float64Constant* nan) { void set_nan(Float64Constant* nan) {
DCHECK_NULL(nan_); DCHECK_NULL(nan_);
...@@ -71,11 +77,11 @@ class Graph final : public ZoneObject { ...@@ -71,11 +77,11 @@ class Graph final : public ZoneObject {
uint32_t tagged_stack_slots_ = kMaxUInt32; uint32_t tagged_stack_slots_ = kMaxUInt32;
uint32_t untagged_stack_slots_ = kMaxUInt32; uint32_t untagged_stack_slots_ = kMaxUInt32;
ZoneVector<BasicBlock*> blocks_; ZoneVector<BasicBlock*> blocks_;
std::map<RootIndex, RootConstant*> root_; ZoneMap<RootIndex, RootConstant*> root_;
std::map<int, SmiConstant*> smi_; ZoneMap<int, SmiConstant*> smi_;
std::map<int, Int32Constant*> int_; ZoneMap<int, Int32Constant*> int_;
std::map<double, Float64Constant*> float_; ZoneMap<double, Float64Constant*> float_;
std::vector<Constant*> constants_; ZoneVector<Constant*> constants_;
Float64Constant* nan_ = nullptr; Float64Constant* nan_ = nullptr;
}; };
......
...@@ -607,9 +607,6 @@ ...@@ -607,9 +607,6 @@
# Tests that need to run sequentially (e.g. due to memory consumption). # Tests that need to run sequentially (e.g. due to memory consumption).
'wasm/asm-wasm': [PASS, HEAVY], 'wasm/asm-wasm': [PASS, HEAVY],
# TODO(v8:7700): Fix leaks involving std containers in Zone objects.
'maglev/tier-to-ml-to-tf': [SKIP],
}], # 'asan == True' }], # 'asan == True'
############################################################################## ##############################################################################
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment