From ff702174d8f77202203bbdb2d7713e1263f3c24d Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 16 Dec 2020 19:10:17 -0500 Subject: [PATCH] Report double calls to vpi_release_handle when using VL_DEBUG --- include/verilated_vpi.cpp | 26 +++++++++++++++++--- test_regress/t/TestVpi.h | 2 +- test_regress/t/t_vpi_release_dup_bad.out | 2 ++ test_regress/t/t_vpi_release_dup_bad.pl | 25 +++++++++++++++++++ test_regress/t/t_vpi_release_dup_bad.v | 15 ++++++++++++ test_regress/t/t_vpi_release_dup_bad_c.cpp | 28 ++++++++++++++++++++++ 6 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 test_regress/t/t_vpi_release_dup_bad.out create mode 100755 test_regress/t/t_vpi_release_dup_bad.pl create mode 100644 test_regress/t/t_vpi_release_dup_bad.v create mode 100644 test_regress/t/t_vpi_release_dup_bad_c.cpp diff --git a/include/verilated_vpi.cpp b/include/verilated_vpi.cpp index bd5dc3abe..8693bca46 100644 --- a/include/verilated_vpi.cpp +++ b/include/verilated_vpi.cpp @@ -56,6 +56,13 @@ constexpr unsigned VL_VPI_LINE_SIZE = 8192; // Base VPI handled object class VerilatedVpio VL_NOT_FINAL { + // CONSTANTS + /// Magic value stored in front of object to detect double free etc + /// Must be odd, as aligned pointer can never be odd +#ifdef VL_DEBUG + static constexpr vluint32_t activeMagic() { return 0xfeed100f; } +#endif + // MEM MANGLEMENT static VL_THREAD_LOCAL vluint8_t* t_freeHead; @@ -73,14 +80,27 @@ public: if (VL_LIKELY(t_freeHead)) { vluint8_t* newp = t_freeHead; t_freeHead = *(reinterpret_cast(newp)); +#ifdef VL_DEBUG + *(reinterpret_cast(newp)) = activeMagic(); +#endif return newp + 8; } // +8: 8 bytes for next vluint8_t* newp = reinterpret_cast(::operator new(chunk + 8)); +#ifdef VL_DEBUG + *(reinterpret_cast(newp)) = activeMagic(); +#endif return newp + 8; } static void operator delete(void* obj, size_t /*size*/)VL_MT_SAFE { vluint8_t* oldp = (static_cast(obj)) - 8; +#ifdef VL_DEBUG + if (VL_UNLIKELY(*(reinterpret_cast(oldp)) != activeMagic())) { + VL_FATAL_MT(__FILE__, __LINE__, "", + "vpi_release_handle() called on same object twice, or on non-Verilator " + "VPI object"); + } +#endif *(reinterpret_cast(oldp)) = t_freeHead; t_freeHead = oldp; } @@ -2003,9 +2023,8 @@ PLI_INT32 vpi_chk_error(p_vpi_error_info error_info_p) { } PLI_INT32 vpi_free_object(vpiHandle object) { - VerilatedVpiImp::assertOneCheck(); - _VL_VPI_ERROR_RESET(); - return vpi_release_handle(object); // Deprecated + // vpi_free_object is IEEE deprecated, use vpi_release_handle + return vpi_release_handle(object); } PLI_INT32 vpi_release_handle(vpiHandle object) { @@ -2014,6 +2033,7 @@ PLI_INT32 vpi_release_handle(vpiHandle object) { VerilatedVpio* vop = VerilatedVpio::castp(object); _VL_VPI_ERROR_RESET(); if (VL_UNLIKELY(!vop)) return 0; + vpi_remove_cb(object); // May not be a callback, but that's ok VL_DO_DANGLING(delete vop, vop); return 1; diff --git a/test_regress/t/TestVpi.h b/test_regress/t/TestVpi.h index e6371c6c1..555e97a18 100644 --- a/test_regress/t/TestVpi.h +++ b/test_regress/t/TestVpi.h @@ -29,7 +29,7 @@ public: if (m_handle && m_free) { // Below not VL_DO_DANGLING so is portable { - vpi_free_object(m_handle); + vpi_release_handle(m_handle); m_handle = NULL; } } diff --git a/test_regress/t/t_vpi_release_dup_bad.out b/test_regress/t/t_vpi_release_dup_bad.out new file mode 100644 index 000000000..fdb02d67b --- /dev/null +++ b/test_regress/t/t_vpi_release_dup_bad.out @@ -0,0 +1,2 @@ +%Error: /svaha/wsnyder/SandBox/homecvs/v4/verilator/test_regress/../include/verilated_vpi.cpp:99: vpi_release_handle() called on same object twice, or on non-Verilator VPI object +Aborting... diff --git a/test_regress/t/t_vpi_release_dup_bad.pl b/test_regress/t/t_vpi_release_dup_bad.pl new file mode 100755 index 000000000..ee70f0752 --- /dev/null +++ b/test_regress/t/t_vpi_release_dup_bad.pl @@ -0,0 +1,25 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. This program is free software; you +# can redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(vlt => 1); + +compile( + v_flags2 => ["t/$Self->{name}_c.cpp"], + verilator_flags2 => ['--vpi'], + ); + +execute( + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); + +1; diff --git a/test_regress/t/t_vpi_release_dup_bad.v b/test_regress/t/t_vpi_release_dup_bad.v new file mode 100644 index 000000000..1ba1097e0 --- /dev/null +++ b/test_regress/t/t_vpi_release_dup_bad.v @@ -0,0 +1,15 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under The Creative Commons Public Domain, for +// any use, without warranty, 2020 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +import "DPI-C" context function void dpii_check(); + +module t (/*AUTOARG*/); + initial begin + dpii_check(); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_vpi_release_dup_bad_c.cpp b/test_regress/t/t_vpi_release_dup_bad_c.cpp new file mode 100644 index 000000000..2d05cb61d --- /dev/null +++ b/test_regress/t/t_vpi_release_dup_bad_c.cpp @@ -0,0 +1,28 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +//************************************************************************* +// +// Copyright 2009-2011 by Wilson Snyder. This program is free software; you +// can redistribute it and/or modify it under the terms of either the GNU +// Lesser General Public License Version 3 or the Perl Artistic License +// Version 2.0. +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 +// +//************************************************************************* + +#include +#include "svdpi.h" +#include "vpi_user.h" +//#include "verilated.h" + +#include "Vt_vpi_release_dup_bad__Dpi.h" + +//====================================================================== + +void dpii_check() { + vpiHandle mod; // Not TestVpiHandle as testing double free + // Verilated::scopesDump(); + mod = vpi_handle_by_name((PLI_BYTE8*)"top.t", NULL); + if (!mod) vpi_printf(const_cast("-- Cannot vpi_find module\n")); + vpi_free_object(mod); // using vpi_free_object instead of vpi_release_handle for coverage + vpi_free_object(mod); // error: double free +}