Merge pull request #534 from antmicro/mglb/FixLeaks
Fix memory leaks detected by ASAN.
diff --git a/systemverilog-plugin/UhdmAst.cc b/systemverilog-plugin/UhdmAst.cc
index 36c5080..f1b81a9 100644
--- a/systemverilog-plugin/UhdmAst.cc
+++ b/systemverilog-plugin/UhdmAst.cc
@@ -13,6 +13,8 @@
#include "frontends/ast/ast.h"
#include "libs/sha1/sha1.h"
+#include "utils/memory.h"
+
// UHDM
#include <uhdm/ExprEval.h>
#include <uhdm/uhdm.h>
@@ -493,21 +495,16 @@
}
if (wiretype_node == nullptr)
return;
- std::vector<AST::AstNode *> packed_ranges;
- std::vector<AST::AstNode *> unpacked_ranges;
- // First check if it has already defined ranges
- if (wire_node->attributes.count(UhdmAst::packed_ranges())) {
- for (auto r : wire_node->attributes[UhdmAst::packed_ranges()]->children) {
- packed_ranges.push_back(r->clone());
- }
- }
- if (wire_node->attributes.count(UhdmAst::unpacked_ranges())) {
- for (auto r : wire_node->attributes[UhdmAst::unpacked_ranges()]->children) {
- unpacked_ranges.push_back(r->clone());
- }
- }
+
+ unique_resource<std::vector<AST::AstNode *>> packed_ranges = wire_node->attributes.count(attr_id::packed_ranges)
+ ? std::move(wire_node->attributes[attr_id::packed_ranges]->children)
+ : std::vector<AST::AstNode *>{};
delete_attribute(wire_node, attr_id::packed_ranges);
+ unique_resource<std::vector<AST::AstNode *>> unpacked_ranges = wire_node->attributes.count(attr_id::unpacked_ranges)
+ ? std::move(wire_node->attributes[attr_id::unpacked_ranges]->children)
+ : std::vector<AST::AstNode *>{};
delete_attribute(wire_node, attr_id::unpacked_ranges);
+
AST::AstNode *wiretype_ast = nullptr;
log_assert(AST_INTERNAL::current_scope.count(wiretype_node->str));
wiretype_ast = AST_INTERNAL::current_scope[wiretype_node->str];
@@ -555,10 +552,10 @@
// add wiretype range before current wire ranges
std::reverse(packed_ranges_wiretype.begin(), packed_ranges_wiretype.end());
std::reverse(unpacked_ranges_wiretype.begin(), unpacked_ranges_wiretype.end());
- std::reverse(packed_ranges.begin(), packed_ranges.end());
- std::reverse(unpacked_ranges.begin(), unpacked_ranges.end());
- packed_ranges.insert(packed_ranges.begin(), packed_ranges_wiretype.begin(), packed_ranges_wiretype.end());
- unpacked_ranges.insert(unpacked_ranges.begin(), unpacked_ranges_wiretype.begin(), unpacked_ranges_wiretype.end());
+ std::reverse(packed_ranges->begin(), packed_ranges->end());
+ std::reverse(unpacked_ranges->begin(), unpacked_ranges->end());
+ packed_ranges->insert(packed_ranges->begin(), packed_ranges_wiretype.begin(), packed_ranges_wiretype.end());
+ unpacked_ranges->insert(unpacked_ranges->begin(), unpacked_ranges_wiretype.begin(), unpacked_ranges_wiretype.end());
AST::AstNode *value = nullptr;
if (wire_node->children[0]->type != AST::AST_RANGE) {
value = wire_node->children[0]->clone();
@@ -566,7 +563,7 @@
delete_children(wire_node);
if (value)
wire_node->children.push_back(value);
- add_multirange_wire(wire_node, packed_ranges, unpacked_ranges, false /* reverse */);
+ add_multirange_wire(wire_node, packed_ranges.release(), unpacked_ranges.release(), false /* reverse */);
}
}
@@ -2271,7 +2268,7 @@
current_node = make_ast_node(AST::AST_CELL);
std::vector<std::pair<RTLIL::IdString, RTLIL::Const>> parameters;
- std::vector<AST::AstNode *> parameter_typedefs;
+ auto parameter_typedefs = make_unique_resource<std::vector<AST::AstNode *>>();
visit_one_to_many({vpiParameter}, obj_h, [&](AST::AstNode *node) {
log_assert(node);
@@ -2300,7 +2297,7 @@
if (child->type == AST::AST_TYPEDEF || child->type == AST::AST_ENUM) {
// Copy definition of the type provided as parameter.
- parameter_typedefs.push_back(child->clone());
+ parameter_typedefs->push_back(child->clone());
}
}
delete node;
@@ -2366,7 +2363,9 @@
}
}
});
- module_node->children.insert(std::end(module_node->children), std::begin(parameter_typedefs), std::end(parameter_typedefs));
+ module_node->children.insert(std::end(module_node->children), std::begin(*parameter_typedefs), std::end(*parameter_typedefs));
+ parameter_typedefs->clear();
+ parameter_typedefs.reset();
if (module_node->attributes.count(UhdmAst::partial())) {
AST::AstNode *attr = module_node->attributes.at(UhdmAst::partial());
if (attr->type == AST::AST_CONSTANT)
@@ -3091,7 +3090,8 @@
auto top_node = find_ancestor({AST::AST_MODULE});
if (!top_node)
return;
- top_node->children.push_back(current_node->children[0]->clone());
+ top_node->children.push_back(std::move(current_node->children[0]));
+ delete current_node;
current_node = nullptr;
}
}
@@ -3974,6 +3974,7 @@
if (current_node->children.empty()) {
delete assign_node->children[0];
assign_node->children[0] = assignments[0]->children[0];
+ delete current_node;
current_node = assignments[0]->children[1];
assignments[0]->children.clear();
delete assignments[0];
diff --git a/systemverilog-plugin/utils/memory.h b/systemverilog-plugin/utils/memory.h
new file mode 100644
index 0000000..6cd43c0
--- /dev/null
+++ b/systemverilog-plugin/utils/memory.h
@@ -0,0 +1,195 @@
+#ifndef SYSTEMVERILOG_PLUGIN_UTILS_MEMORY_H_
+#define SYSTEMVERILOG_PLUGIN_UTILS_MEMORY_H_
+
+#include <cassert>
+#include <utility>
+
+namespace systemverilog_plugin
+{
+
+// `std::default_delete` equivalent for any range of pointers, e.g. `std::vector<Object *>`.
+template <class Range> struct default_delete_ptr_range {
+ void operator()(Range &range) const
+ {
+ for (auto *ptr : range)
+ delete ptr;
+ }
+};
+
+// Functor that takes a reference and does nothing. Useful as no-op deleter.
+struct noop_delete {
+ template <class T> void operator()(T &) const {}
+};
+
+namespace utils_memory_internal
+{
+
+// Unique type for detecting invalid (missing) default_resource_deleter.
+struct missing_type {
+};
+
+// Provider of default deleter functor for resource of type `R` used by `unique_resource`.
+template <class R, class AlwaysVoid_ = void> struct default_resource_deleter {
+ using type = missing_type;
+};
+
+// Type trait for detecting whether type `R` is any range of pointers.
+template <class R, class ValueType_ = std::remove_reference_t<decltype(*std::begin(std::declval<R>()))>>
+using is_range_of_pointers_t = std::enable_if_t<std::is_pointer_v<ValueType_> && !std::is_array_v<ValueType_>>;
+
+// Overload for any range of pointers.
+template <class R> struct default_resource_deleter<R, is_range_of_pointers_t<R>> {
+ using type = default_delete_ptr_range<R>;
+};
+
+// Convenience alias.
+template <class R> using default_resource_deleter_t = typename default_resource_deleter<R>::type;
+
+// Type trait for checking whether type `R` is a valid deleter of type `D`.
+template <class R, class D, class = void> struct is_valid_resource_deleter : std::false_type {
+};
+template <class R, class D> struct is_valid_resource_deleter<R, D, std::void_t<decltype(std::declval<D>()(std::declval<R &>()))>> : std::true_type {
+};
+
+// Convenience alias.
+template <class R, class D> inline constexpr bool is_valid_resource_deleter_v = is_valid_resource_deleter<R, D>::value;
+
+} // namespace utils_memory_internal
+
+// Wrapper that holds and manages resource of type `Resource`. Equivalent of `unique_ptr` for non-pointer types.
+//
+// `unique_resource` tracks initialization status of its resource. The `Destructor` is called only when the resource is in initialized state.
+// `unique_resource` constructed using default constructor is in uninitialized state. It becomes initialized when a valid resource is moved into it.
+// Moving resource out or releasing it switches state to uninitialized.
+//
+// The API is based on unique_ptr rather than `unique_resource` from Library Fundamentals TS3.
+template <class Resource, class Deleter = utils_memory_internal::default_resource_deleter_t<Resource>> class unique_resource
+{
+ // Check for errors in template parameters.
+ // Use of intermediate constexprs results in nicer error messages.
+ static constexpr bool deleter_for_resource_exists = !std::is_same_v<Deleter, utils_memory_internal::missing_type>;
+ static_assert(deleter_for_resource_exists, "'Deleter' has not been specified and no default deleter exists for type 'Resource'.");
+ static constexpr bool deleter_is_callable_with_resource_ref = utils_memory_internal::is_valid_resource_deleter_v<Resource, Deleter>;
+ static_assert(deleter_is_callable_with_resource_ref, "Object of type 'Deleter' must be callable with argument of type 'Resource &'.");
+ static constexpr bool resource_type_is_not_cvref = std::is_same_v<Resource, std::remove_cv_t<std::remove_reference_t<Resource>>>;
+ static_assert(resource_type_is_not_cvref, "'Resource' must not be a reference or have const or volatile qualifier.");
+
+ // Data members.
+
+ Resource resource = {};
+ bool initialized = false;
+
+ public:
+ // Initialize
+
+ unique_resource() = default;
+
+ template <class OtherResource> unique_resource(OtherResource &&other) : resource(std::forward<OtherResource>(other)), initialized(true) {}
+
+ // Copy
+
+ unique_resource(const unique_resource &) = delete;
+
+ unique_resource &operator=(const unique_resource &) = delete;
+
+ // Move
+
+ template <class OtherDeleter>
+ unique_resource(unique_resource<Resource, OtherDeleter> &&other) : resource(std::move(other.resource)), initialized(other.initialized)
+ {
+ other.initialized = false;
+ }
+
+ template <class OtherDeleter> unique_resource &operator=(unique_resource<Resource, OtherDeleter> &&other)
+ {
+ resource = std::move(other.resource);
+ initialized = other.initialized;
+ other.initialized = false;
+ };
+
+ // Destroy
+
+ ~unique_resource()
+ {
+ if (initialized) {
+ Deleter{}(resource);
+ }
+ }
+
+ // Data access
+
+ Resource &get()
+ {
+ assert(initialized);
+ return resource;
+ }
+
+ const Resource &get() const
+ {
+ assert(initialized);
+ return resource;
+ }
+
+ Resource &operator*()
+ {
+ assert(initialized);
+ return resource;
+ }
+
+ const Resource &operator*() const
+ {
+ assert(initialized);
+ return resource;
+ }
+
+ Resource *operator->()
+ {
+ assert(initialized);
+ return &resource;
+ }
+
+ const Resource *operator->() const
+ {
+ assert(initialized);
+ return &resource;
+ }
+
+ operator bool() const { return initialized; }
+
+ // Operations
+
+ Resource release()
+ {
+ Resource r = std::move(resource);
+ initialized = false;
+ return r;
+ }
+
+ void reset()
+ {
+ if (initialized) {
+ Deleter{}(resource);
+ initialized = false;
+ }
+ }
+
+ template <class OtherResource> void reset(OtherResource &&other)
+ {
+ if (initialized) {
+ Deleter{}(resource);
+ }
+ resource = std::forward<OtherResource>(other);
+ initialized = true;
+ }
+};
+
+// Creates `unique_resource<Resource, Deleter>` and initializes it with a resource constructed using specified arguments.
+template <class Resource, class Deleter = utils_memory_internal::default_resource_deleter_t<Resource>, class... Tn>
+inline unique_resource<Resource, Deleter> make_unique_resource(Tn &&... arg_n)
+{
+ return unique_resource<Resource, Deleter>(Resource(std::forward<Tn>(arg_n)...));
+}
+
+} // namespace systemverilog_plugin
+
+#endif // SYSTEMVERILOG_PLUGIN_UTILS_MEMORY_H_
