From ee152be17b5492ce500a6c7bfb3edaf7c73bb0a5 Mon Sep 17 00:00:00 2001 From: Bartosz Skorowski <110765576+bskorowski@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:58:28 +0200 Subject: [PATCH] Fix lost writes when select width exceeds variable width (#7975) Signed-off-by: Bartosz Skorowski --- src/V3Width.cpp | 52 +++++++++++++++----------- test_regress/t/t_mem_multi_ref_bad.out | 12 ++++-- test_regress/t/t_sel_write_oob_high.py | 18 +++++++++ test_regress/t/t_sel_write_oob_high.v | 51 +++++++++++++++++++++++++ test_regress/t/t_select_bad_range5.out | 2 +- 5 files changed, 109 insertions(+), 26 deletions(-) create mode 100755 test_regress/t/t_sel_write_oob_high.py create mode 100644 test_regress/t/t_sel_write_oob_high.v diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 84d66c260..d608a9228 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -1106,6 +1106,8 @@ class WidthVisitor final : public VNVisitor { } UASSERT_OBJ(nodep->dtypep(), nodep, "dtype wasn't set"); // by V3WidthSel + AstNodeVarRef* lrefp = AstNodeVarRef::varRefLValueRecurse(nodep); + const bool isWriteSelect = lrefp && lrefp->access().isWriteOrRW(); // Suppress SELRANGE in parameterized template modules where // parameter-dependent widths haven't been resolved yet. const bool inParameterizedTemplate @@ -1124,13 +1126,21 @@ class WidthVisitor final : public VNVisitor { } // We're extracting, so just make sure the expression is at least wide enough. if (nodep->fromp()->width() < width && !inParameterizedTemplate) { - nodep->v3warn(SELRANGE, "Extracting " << width << " bits from only " - << nodep->fromp()->width() << " bit number"); - // Extend it. - AstNodeDType* const subDTypep - = nodep->findLogicDType(width, width, nodep->fromp()->dtypep()->numeric()); - widthCheckSized(nodep, "errorless...", nodep->fromp(), subDTypep, EXTEND_EXP, - false /*noerror*/); + // If it is not a lvalue, extend it + if (!isWriteSelect) { + nodep->v3warn(SELRANGE, "Extracting " << width << " bits from only " + << nodep->fromp()->width() + << " bit number"); + AstNodeDType* const subDTypep + = nodep->findLogicDType(width, width, nodep->fromp()->dtypep()->numeric()); + widthCheckSized(nodep, "errorless...", nodep->fromp(), subDTypep, EXTEND_EXP, + false /*noerror*/); + } else { + // Partial assignment + nodep->v3warn(SELRANGE, "Assigning " << width << " bits to only " + << nodep->fromp()->width() + << " bit number"); + } } // Check bit indexes. // What is the MSB? We want the true MSB, not one starting at @@ -1175,26 +1185,26 @@ class WidthVisitor final : public VNVisitor { // evaluating type sizes for a generate block condition. We // should only trigger the error if the out-of-range access is // actually generated. - AstNodeVarRef* lrefp = AstNodeVarRef::varRefLValueRecurse(nodep); if (m_doGenerate) { UINFO(5, "Selection index out of range inside generate"); } else if (!inParameterizedTemplate) { - nodep->v3warn(SELRANGE, "Selection index out of range: " - << nodep->msbConst() << ":" << nodep->lsbConst() - << " outside " << frommsb << ":" << fromlsb); + if (nodep->declRange().ranged()) { + nodep->v3warn(SELRANGE, "Selection index out of range: " + << nodep->msbConst() << ":" + << nodep->lsbConst() << " outside " << frommsb + << ":" << fromlsb); + } else { + nodep->v3warn(SELRANGE, + "Selection " + << nodep->msbConst() << ":" << nodep->lsbConst() + << " performed on an object without declared range"); + } UINFO(1, " Related node: " << nodep); } if (lrefp) UINFO(9, " Select extend lrefp " << lrefp); - if (lrefp && lrefp->access().isWriteOrRW()) { - // lvarref[X] = ..., the expression assigned is too wide - // WTF to do - // Don't change the width of this lhsp, instead propagate up - // to upper assign/expression the correct width - AstNodeDType* const subDTypep - = nodep->findLogicDType(width, width, nodep->fromp()->dtypep()->numeric()); - widthCheckSized(nodep, "errorless...", nodep->fromp(), subDTypep, EXTEND_EXP, - false /*noerror*/); - } else { + // Extend unless it's a lvalue, + // because extending lvalue would lose write access. + if (!isWriteSelect) { // Extend it const int extendTo = nodep->msbConst() + 1; AstNodeDType* const subDTypep = nodep->findLogicDType( diff --git a/test_regress/t/t_mem_multi_ref_bad.out b/test_regress/t/t_mem_multi_ref_bad.out index 7991320bf..33c3be7e8 100644 --- a/test_regress/t/t_mem_multi_ref_bad.out +++ b/test_regress/t/t_mem_multi_ref_bad.out @@ -3,17 +3,21 @@ 15 | dimn[1:0] = 0; | ^ ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. -%Warning-SELRANGE: t/t_mem_multi_ref_bad.v:15:9: Extracting 2 bits from only 1 bit number +%Warning-SELRANGE: t/t_mem_multi_ref_bad.v:15:9: Assigning 2 bits to only 1 bit number : ... note: In instance 't' 15 | dimn[1:0] = 0; | ^ ... For warning description see https://verilator.org/warn/SELRANGE?v=latest ... Use "/* verilator lint_off SELRANGE */" and lint_on around source to disable this message. +%Warning-SELRANGE: t/t_mem_multi_ref_bad.v:15:9: Selection 1:0 performed on an object without declared range + : ... note: In instance 't' + 15 | dimn[1:0] = 0; + | ^ %Error: t/t_mem_multi_ref_bad.v:16:12: Illegal bit or array select; type does not have a bit range, or bad dimension: data type is 'logic' : ... note: In instance 't' 16 | dim0[1][1] = 0; | ^ -%Warning-SELRANGE: t/t_mem_multi_ref_bad.v:16:12: Selection index out of range: 1:1 outside 0:0 +%Warning-SELRANGE: t/t_mem_multi_ref_bad.v:16:12: Selection 1:1 performed on an object without declared range : ... note: In instance 't' 16 | dim0[1][1] = 0; | ^ @@ -21,7 +25,7 @@ : ... note: In instance 't' 17 | dim1[1][1][1] = 0; | ^ -%Warning-SELRANGE: t/t_mem_multi_ref_bad.v:17:15: Selection index out of range: 1:1 outside 0:0 +%Warning-SELRANGE: t/t_mem_multi_ref_bad.v:17:15: Selection 1:1 performed on an object without declared range : ... note: In instance 't' 17 | dim1[1][1][1] = 0; | ^ @@ -33,7 +37,7 @@ : ... note: In instance 't' 23 | dim0nv[1][1] = 0; | ^ -%Warning-SELRANGE: t/t_mem_multi_ref_bad.v:23:14: Selection index out of range: 1:1 outside 0:0 +%Warning-SELRANGE: t/t_mem_multi_ref_bad.v:23:14: Selection 1:1 performed on an object without declared range : ... note: In instance 't' 23 | dim0nv[1][1] = 0; | ^ diff --git a/test_regress/t/t_sel_write_oob_high.py b/test_regress/t/t_sel_write_oob_high.py new file mode 100755 index 000000000..31fc821ce --- /dev/null +++ b/test_regress/t/t_sel_write_oob_high.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 + +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# 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-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile() +test.execute() + +test.passes() diff --git a/test_regress/t/t_sel_write_oob_high.v b/test_regress/t/t_sel_write_oob_high.v new file mode 100644 index 000000000..c78c93ef0 --- /dev/null +++ b/test_regress/t/t_sel_write_oob_high.v @@ -0,0 +1,51 @@ +// DESCRIPTION: Check that a blocking partial write with select outside of MSB +// of a 2-state vector doesn't corrup lvalue write refs. +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +// verilog_format: off +`define stop $stop +`define checkh(gotv, expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: $time=%0t got='h%x exp='h%x\n", `__FILE__,`__LINE__, $time, (gotv), (expv)); `stop; end while(0) +// verilog_format: on + +module t; + + initial begin + bit [5:0] x; + integer i; + + // verilator lint_off SELRANGE + x = 'h0; + x[8:4] = 5'b10001; + `checkh(x, 6'b010000); // Const, partially OOB high + + x = 'h0; + x[11:6] = 6'b111111; + `checkh(x, '0); // Const, fully OOB high + + x = 'h0; + x[9:3] = 7'b1011010; + `checkh(x, 6'b010000); // Const, select width > declared width, OOB high + + i = 4; + x = 'h0; + x[i+:5] = 5'b10001; + `checkh(x, 6'b010000); // Var, partially OOB high + + i = 6; + x = 'h0; + x[i+:6] = 6'b111111; + `checkh(x, '0); // Var, fully OOB high + + i = 3; + x = 'h0; + x[i+:7] = 7'b1011010; + `checkh(x, 6'b010000); // Var, select width > declared width, OOB high + // verilator lint_on SELRANGE + + $write("*-* All finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_select_bad_range5.out b/test_regress/t/t_select_bad_range5.out index 6a6d8c075..b17a1f2d3 100644 --- a/test_regress/t/t_select_bad_range5.out +++ b/test_regress/t/t_select_bad_range5.out @@ -9,7 +9,7 @@ | ^ ... For warning description see https://verilator.org/warn/SELRANGE?v=latest ... Use "/* verilator lint_off SELRANGE */" and lint_on around source to disable this message. -%Warning-SELRANGE: t/t_select_bad_range5.v:18:18: Selection index out of range: 3:2 outside 1:0 +%Warning-SELRANGE: t/t_select_bad_range5.v:18:18: Selection 3:2 performed on an object without declared range : ... note: In instance 't' 18 | assign mi = unk[3:2]; | ^