From ce9293fcb334d7330f5d2145d80c289d04c3ddc7 Mon Sep 17 00:00:00 2001 From: Yutetsu TAKATSUKASA Date: Thu, 17 Dec 2020 08:31:47 +0900 Subject: [PATCH] Fix memory leaks found in trace related tests (#2708) * Fix memory leak of t_trace_cat and t_trace_cat_renew * Fix memory leak of t_trace_c_api * Fix memory leak in t_trace_public_func and t_trace_public_func_vlt * Fix memory leaks in t_flat_build (and probably more). * Use unique_ptr in testcases --- include/verilated_vcd_c.cpp | 1 + test_regress/driver.pl | 16 +++++++++------- test_regress/t/t_trace_cat.cpp | 15 ++++++++------- test_regress/t/t_trace_public_func.cpp | 10 ++++++---- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/include/verilated_vcd_c.cpp b/include/verilated_vcd_c.cpp index 233f6b01e..9339eced7 100644 --- a/include/verilated_vcd_c.cpp +++ b/include/verilated_vcd_c.cpp @@ -893,6 +893,7 @@ void vcdTestMain(const char* filenamep) { } # endif vcdp->close(); + VL_DO_CLEAR(delete vcdp, vcdp = nullptr); } } #endif diff --git a/test_regress/driver.pl b/test_regress/driver.pl index 1b607579d..f4214443f 100755 --- a/test_regress/driver.pl +++ b/test_regress/driver.pl @@ -1686,6 +1686,7 @@ sub _make_main { print $fh "// Test defines\n"; print $fh "#define MAIN_TIME_MULTIPLIER ".($self->{main_time_multiplier} || 1)."\n"; + print $fh "#include \n"; print $fh "// OS header\n"; print $fh "#include \"verilatedos.h\"\n"; @@ -1701,7 +1702,7 @@ sub _make_main { print $fh "#include \"verilated_vcd_sc.h\"\n" if $self->{trace} && $self->{trace_format} eq 'vcd-sc'; print $fh "#include \"verilated_save.h\"\n" if $self->{savable}; - print $fh "$VM_PREFIX* topp;\n"; + print $fh "std::unique_ptr<$VM_PREFIX> topp;\n"; if (!$self->sc) { if ($self->{vl_time_stamp64}) { print $fh "vluint64_t main_time = 0;\n"; @@ -1749,7 +1750,7 @@ sub _make_main { print $fh " Verilated::debug(".($self->{verilated_debug}?1:0).");\n"; print $fh " srand48(5);\n"; # Ensure determinism print $fh " Verilated::randReset(".$self->{verilated_randReset}.");\n" if defined $self->{verilated_randReset}; - print $fh " topp = new $VM_PREFIX(\"top\");\n"; + print $fh " topp.reset(new $VM_PREFIX(\"top\"));\n"; print $fh " Verilated::internalsDump()\n;" if $self->{verilated_debug}; my $set; @@ -1766,10 +1767,10 @@ sub _make_main { $fh->print("\n"); $fh->print("#if VM_TRACE\n"); $fh->print(" Verilated::traceEverOn(true);\n"); - $fh->print(" VerilatedFstC* tfp = new VerilatedFstC;\n") if $self->{trace_format} eq 'fst-c'; - $fh->print(" VerilatedVcdC* tfp = new VerilatedVcdC;\n") if $self->{trace_format} eq 'vcd-c'; - $fh->print(" VerilatedVcdSc* tfp = new VerilatedVcdSc;\n") if $self->{trace_format} eq 'vcd-sc'; - $fh->print(" topp->trace(tfp, 99);\n"); + $fh->print(" std::unique_ptr tfp{new VerilatedFstC};\n") if $self->{trace_format} eq 'fst-c'; + $fh->print(" std::unique_ptr tfp{new VerilatedVcdC};\n") if $self->{trace_format} eq 'vcd-c'; + $fh->print(" std::unique_ptr tfp{new VerilatedVcdSc};\n") if $self->{trace_format} eq 'vcd-sc'; + $fh->print(" topp->trace(tfp.get(), 99);\n"); $fh->print(" tfp->open(\"".$self->trace_filename."\");\n"); if ($self->{trace} && !$self->sc) { $fh->print(" if (tfp) tfp->dump(main_time);\n"); @@ -1831,11 +1832,12 @@ sub _make_main { if ($self->{trace}) { $fh->print("#if VM_TRACE\n"); $fh->print(" if (tfp) tfp->close();\n"); + $fh->print(" tfp.reset();\n"); $fh->print("#endif // VM_TRACE\n"); } $fh->print("\n"); - print $fh " VL_DO_DANGLING(delete topp, topp);\n"; + print $fh " topp.reset();\n"; print $fh " exit(0L);\n"; print $fh "}\n"; $fh->close(); diff --git a/test_regress/t/t_trace_cat.cpp b/test_regress/t/t_trace_cat.cpp index c0374121d..8f851daf7 100644 --- a/test_regress/t/t_trace_cat.cpp +++ b/test_regress/t/t_trace_cat.cpp @@ -6,6 +6,7 @@ // any use, without warranty, 2008 by Wilson Snyder. // SPDX-License-Identifier: CC0-1.0 +#include #include #include @@ -21,13 +22,13 @@ const char* trace_name() { } int main(int argc, char** argv, char** env) { - VM_PREFIX* top = new VM_PREFIX("top"); + std::unique_ptr top{new VM_PREFIX("top")}; Verilated::debug(0); Verilated::traceEverOn(true); - VerilatedVcdC* tfp = new VerilatedVcdC; - top->trace(tfp, 99); + std::unique_ptr tfp{new VerilatedVcdC}; + top->trace(tfp.get(), 99); tfp->open(trace_name()); @@ -45,9 +46,8 @@ int main(int argc, char** argv, char** env) { tfp->open(trace_name()); #elif defined(T_TRACE_CAT_RENEW) tfp->close(); - delete tfp; - tfp = new VerilatedVcdC; - top->trace(tfp, 99); + tfp.reset(new VerilatedVcdC); + top->trace(tfp.get(), 99); tfp->open(trace_name()); #else #error "Unknown test" @@ -58,7 +58,8 @@ int main(int argc, char** argv, char** env) { } tfp->close(); top->final(); - VL_DO_DANGLING(delete top, top); + tfp.reset(); + top.reset(); printf("*-* All Finished *-*\n"); return 0; } diff --git a/test_regress/t/t_trace_public_func.cpp b/test_regress/t/t_trace_public_func.cpp index b77045813..b17540c2d 100644 --- a/test_regress/t/t_trace_public_func.cpp +++ b/test_regress/t/t_trace_public_func.cpp @@ -6,6 +6,7 @@ // any use, without warranty, 2008 by Wilson Snyder. // SPDX-License-Identifier: CC0-1.0 +#include #include #include @@ -26,13 +27,13 @@ double sc_time_stamp() { return (double)main_time; } const unsigned long long dt_2 = 3; int main(int argc, char** argv, char** env) { - VM_PREFIX* top = new VM_PREFIX("top"); + std::unique_ptr top{new VM_PREFIX("top")}; Verilated::debug(0); Verilated::traceEverOn(true); - VerilatedVcdC* tfp = new VerilatedVcdC; - top->trace(tfp, 99); + std::unique_ptr tfp{new VerilatedVcdC}; + top->trace(tfp.get(), 99); tfp->open(VL_STRINGIFY(TEST_OBJ_DIR) "/simx.vcd"); while (main_time <= 20) { @@ -46,7 +47,8 @@ int main(int argc, char** argv, char** env) { } tfp->close(); top->final(); - VL_DO_DANGLING(delete top, top); + tfp.reset(); + top.reset(); printf("*-* All Finished *-*\n"); return 0; }