From f39e56111e6a0efe8d9326310721eeac79259578 Mon Sep 17 00:00:00 2001 From: Christian Hecken Date: Tue, 16 Dec 2025 11:50:36 +0100 Subject: [PATCH] Add Verilation time check for forceable strings --- include/verilated_vpi.cpp | 8 ----- src/V3Force.cpp | 7 +++++ test_regress/t/t_forceable_string_bad.out | 5 ++++ test_regress/t/t_forceable_string_bad.py | 16 ++++++++++ test_regress/t/t_forceable_string_bad.v | 9 ++++++ test_regress/t/t_vpi_force.cpp | 36 ++--------------------- test_regress/t/t_vpi_force.v | 30 +------------------ test_regress/t/t_vpi_forceable_bad.out | 2 +- 8 files changed, 42 insertions(+), 71 deletions(-) create mode 100644 test_regress/t/t_forceable_string_bad.out create mode 100755 test_regress/t/t_forceable_string_bad.py create mode 100644 test_regress/t/t_forceable_string_bad.v diff --git a/include/verilated_vpi.cpp b/include/verilated_vpi.cpp index bacc82ff7..e1baa6f43 100644 --- a/include/verilated_vpi.cpp +++ b/include/verilated_vpi.cpp @@ -2837,14 +2837,6 @@ void vl_vpi_get_value(const VerilatedVpioVarBase* vop, p_vpi_value valuep) { return; } else if (valuep->format == vpiStringVal) { if (varp->vltype() == VLVT_STRING) { - if (VL_UNLIKELY(varp->isForceable())) { - VL_VPI_ERROR_( - __FILE__, __LINE__, - "Attempting to retrieve value of forceable signal '%s' with data type " - "VLVT_STRING, but strings cannot be forced.", - vop->fullname()); - } - if (varp->isParam()) { valuep->value.str = reinterpret_cast(varDatap); return; diff --git a/src/V3Force.cpp b/src/V3Force.cpp index 700c38325..ca588fe17 100644 --- a/src/V3Force.cpp +++ b/src/V3Force.cpp @@ -464,6 +464,13 @@ class ForceConvertVisitor final : public VNVisitor { return; } + const AstBasicDType* const bdtypep = nodep->varp()->basicp(); + const bool strtype = bdtypep && bdtypep->keyword() == VBasicDTypeKwd::STRING; + if (strtype) { + nodep->varp()->v3error( + "Forcing strings is not permitted: " << nodep->varp()->name()); + } + const ForceState::ForceComponentsVarScope& fc = m_state.getForceComponents(nodep); fc.m_enVscp->varp()->sigUserRWPublic(true); fc.m_valVscp->varp()->sigUserRWPublic(true); diff --git a/test_regress/t/t_forceable_string_bad.out b/test_regress/t/t_forceable_string_bad.out new file mode 100644 index 000000000..6fb21ca01 --- /dev/null +++ b/test_regress/t/t_forceable_string_bad.out @@ -0,0 +1,5 @@ +%Error: t/t_forceable_string_bad.v:8:10: Forcing strings is not permitted: t__DOT__str + 8 | string str /*verilator forceable*/; + | ^~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: Exiting due to diff --git a/test_regress/t/t_forceable_string_bad.py b/test_regress/t/t_forceable_string_bad.py new file mode 100755 index 000000000..e30916148 --- /dev/null +++ b/test_regress/t/t_forceable_string_bad.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2025 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 + +import vltest_bootstrap + +test.scenarios('vlt') + +test.lint(fails=test.vlt_all, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_forceable_string_bad.v b/test_regress/t/t_forceable_string_bad.v new file mode 100644 index 000000000..668ccbfcc --- /dev/null +++ b/test_regress/t/t_forceable_string_bad.v @@ -0,0 +1,9 @@ +// ====================================================================== +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty. +// SPDX-License-Identifier: CC0-1.0 +// ====================================================================== + +module t; + string str /*verilator forceable*/; +endmodule diff --git a/test_regress/t/t_vpi_force.cpp b/test_regress/t/t_vpi_force.cpp index a20ba2d23..e24e33236 100644 --- a/test_regress/t/t_vpi_force.cpp +++ b/test_regress/t/t_vpi_force.cpp @@ -585,43 +585,13 @@ extern "C" int checkValuesReleased(void) { } #ifdef VERILATOR -// This function only makes sense with Verilator, because other simulators fail at elaboration time -// when trying to force a string. The error message check is specific to verilated_vpi.cpp. - -extern "C" int tryCheckingForceableString(void) { - const std::string forceableStringName = std::string{scopeName} + ".str1"; - TestVpiHandle const stringSignalHandle //NOLINT(misc-misplaced-const) - = vpi_handle_by_name(const_cast(forceableStringName.c_str()), nullptr); - CHECK_RESULT_NZ(stringSignalHandle); // NOLINT(concurrency-mt-unsafe) - - s_vpi_value value_s{.format = vpiStringVal, .value = {}}; - - // Prevent program from terminating, so error message can be collected - Verilated::fatalOnVpiError(false); - vpi_get_value(stringSignalHandle, &value_s); - // Re-enable so tests that should pass properly terminate the simulation on failure - Verilated::fatalOnVpiError(true); - - std::pair receivedError = vpiGetErrorMessage(); - const bool errorOccurred = receivedError.second; - const std::string receivedErrorMessage = receivedError.first; - CHECK_RESULT_NZ(errorOccurred); // NOLINT(concurrency-mt-unsafe) - - const std::string expectedErrorMessage - = "Attempting to retrieve value of forceable signal '" + forceableStringName - + "' with data type VLVT_STRING, but strings cannot be forced."; - // NOLINTNEXTLINE(concurrency-mt-unsafe,performance-avoid-endl) - CHECK_RESULT(receivedErrorMessage, expectedErrorMessage); - return 0; -} - // This function only makes sense with Verilator, because its purpose is testing error messages // emitted from verilated_vpi. extern "C" int tryInvalidPutOperations() { CHECK_RESULT_Z(expectVpiPutError( // NOLINT(concurrency-mt-unsafe) - "str2", {.format = vpiStringVal, .value = {.str = const_cast("foo")}}, + "str1", {.format = vpiStringVal, .value = {.str = const_cast("foo")}}, vpiForceFlag, - "vpi_put_value was used with vpiForceFlag on non-forceable signal 't.test.str2' : " + "vpi_put_value was used with vpiForceFlag on non-forceable signal 't.test.str1' : " "'Test'")); CHECK_RESULT_Z(expectVpiPutError( // NOLINT(concurrency-mt-unsafe) @@ -670,7 +640,7 @@ extern "C" int tryInvalidPutOperations() { // This function is just needed for hitting the test coverage target for verilated_vpi.cpp and // ensuring that vpi_put_value to a string without vpiForceFlag still works. extern "C" int putString() { - const std::string stringName = std::string{scopeName} + ".str2"; + const std::string stringName = std::string{scopeName} + ".str1"; TestVpiHandle const stringSignalHandle //NOLINT(misc-misplaced-const) = vpi_handle_by_name(const_cast(stringName.c_str()), nullptr); CHECK_RESULT_NZ(stringSignalHandle); // NOLINT(concurrency-mt-unsafe) diff --git a/test_regress/t/t_vpi_force.v b/test_regress/t/t_vpi_force.v index 14b37d73a..0b7f26f3c 100644 --- a/test_regress/t/t_vpi_force.v +++ b/test_regress/t/t_vpi_force.v @@ -34,7 +34,6 @@ module Test ( `elsif USE_VPI_NOT_DPI `ifdef VERILATOR `systemc_header - extern "C" int tryCheckingForceableString(); extern "C" int putString(); extern "C" int tryInvalidPutOperations(); extern "C" int putInertialDelay(); @@ -48,7 +47,6 @@ module Test ( `endif `else `ifdef VERILATOR - import "DPI-C" context function int tryCheckingForceableString(); import "DPI-C" context function int putString(); import "DPI-C" context function int tryInvalidPutOperations(); import "DPI-C" context function int putInertialDelay(); @@ -61,11 +59,8 @@ module Test ( import "DPI-C" context function int checkValuesReleased(); `endif - // Non-forceable signal should raise error - string str1 `PUBLIC_FORCEABLE; // std::string - // Verify that vpi_put_value still works for strings - string str2 /*verilator public_flat_rw*/; // std::string + string str1 /*verilator public_flat_rw*/; // std::string // Verify that vpi_put_value still works with vpiInertialDelay logic [ 31:0] delayed `PUBLIC_FORCEABLE; // IData @@ -257,27 +252,6 @@ module Test ( force decStringQContinuously[31:0] = 32'd1431655765; endtask - task automatic vpiTryCheckingForceableString(); - integer vpiStatus = 1; // Default to failed status to ensure that a function *not* getting - // called also causes simulation termination -`ifdef VERILATOR -`ifdef USE_VPI_NOT_DPI - vpiStatus = $c32("tryCheckingForceableString()"); -`else - vpiStatus = tryCheckingForceableString(); -`endif -`else - $stop; // This task only makes sense with Verilator, since other simulators ignore the "verilator forceable" metacomment. -`endif - - if (vpiStatus != 0) begin - $write("%%Error: t_vpi_force.cpp:%0d:", vpiStatus); - $display( - "C Test failed (forcing string either succeeded even though it should have failed, or produced unexpected error message)"); - $stop; - end - endtask - task automatic vpiPutString(); integer vpiStatus = 1; // Default to failed status to ensure that a function *not* getting // called also causes simulation termination @@ -626,7 +600,6 @@ $dumpfile(`STRINGIFY(`TEST_DUMPFILE)); `endif `ifdef VERILATOR - vpiTryCheckingForceableString(); vpiPutString(); vpiTryInvalidPutOperations(); vpiPutInertialDelay(); @@ -688,7 +661,6 @@ $dumpfile(`STRINGIFY(`TEST_DUMPFILE)); $display("time: %0t\tclk:%b", $time, clk); $display("str1: %s", str1); - $display("str2: %s", str2); $display("delayed: %x", delayed); $display("onebit: %x", onebit); diff --git a/test_regress/t/t_vpi_forceable_bad.out b/test_regress/t/t_vpi_forceable_bad.out index 79b3d52d6..5a6f3c927 100644 --- a/test_regress/t/t_vpi_forceable_bad.out +++ b/test_regress/t/t_vpi_forceable_bad.out @@ -1,2 +1,2 @@ -%Error: .../verilated_vpi.cpp:2939: vpi_put_value was used with vpiForceFlag on non-forceable signal 't.nonForceableSignal' : 't' +%Error: .../verilated_vpi.cpp:2931: vpi_put_value was used with vpiForceFlag on non-forceable signal 't.nonForceableSignal' : 't' Aborting...