From 3990376c5783c4796c476036192d7bb9727cb6cf Mon Sep 17 00:00:00 2001 From: Sumanth Kadiyala Date: Sat, 5 Sep 2026 15:33:06 -0400 Subject: [PATCH] Internals: resolved incorrect array bounds guarding (#8224) Co-authored-by: Sumanth Kadiyala --- include/verilated_funcs.h | 4 +- src/V3Unknown.cpp | 2 +- .../t/t_math_insert_bound_unaligned.py | 19 +++++ .../t/t_math_insert_bound_unaligned.v | 74 +++++++++++++++++++ 4 files changed, 96 insertions(+), 3 deletions(-) create mode 100755 test_regress/t/t_math_insert_bound_unaligned.py create mode 100644 test_regress/t/t_math_insert_bound_unaligned.v diff --git a/include/verilated_funcs.h b/include/verilated_funcs.h index c4b483d35..420da03d3 100644 --- a/include/verilated_funcs.h +++ b/include/verilated_funcs.h @@ -1438,8 +1438,8 @@ inline void _vl_insert_WI(WDataOutP iowp, IData ld, int hbit, int lbit, int rbit const int nbitsonright = VL_EDATASIZE - loffset; // bits that end up in lword iowp[lword] = (iowp[lword] & ~linsmask) | ((lde << loffset) & linsmask); // Prevent unsafe write where lword was final writable location and hword is - // out-of-bounds. - if (VL_LIKELY(!(hword == rword && roffset == 0))) { + // out-of-bounds. rbits==0 means the caller guarantees bounds. + if (VL_LIKELY(!(rbits && hword >= VL_WORDS_I(rbits)))) { iowp[hword] = (iowp[hword] & ~hinsmask) | ((lde >> nbitsonright) & (hinsmask & cleanmask)); } diff --git a/src/V3Unknown.cpp b/src/V3Unknown.cpp index 2fdc04394..6f989a889 100644 --- a/src/V3Unknown.cpp +++ b/src/V3Unknown.cpp @@ -146,7 +146,7 @@ class UnknownVisitor final : public VNVisitor { // Returns true if it is known at compile time that `msbConstp` is greater than or equal // `exprp` static bool isStaticlyGte(AstConst* const msbConstp, const AstNodeExpr* const exprp) { - if (msbConstp->width() >= exprp->width() + if (msbConstp->width() >= exprp->width() && exprp->width() < 32 && msbConstp->num().toSInt() >= (1 << exprp->width()) - 1) { return true; } diff --git a/test_regress/t/t_math_insert_bound_unaligned.py b/test_regress/t/t_math_insert_bound_unaligned.py new file mode 100755 index 000000000..02b343dfa --- /dev/null +++ b/test_regress/t/t_math_insert_bound_unaligned.py @@ -0,0 +1,19 @@ +#!/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') + +# Utilizes AddressSanitizer to catch out of bounds writes +test.compile(verilator_flags2=["-runtime-debug"]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_math_insert_bound_unaligned.v b/test_regress/t/t_math_insert_bound_unaligned.v new file mode 100644 index 000000000..f48dd8386 --- /dev/null +++ b/test_regress/t/t_math_insert_bound_unaligned.v @@ -0,0 +1,74 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +// Validates two out-of-bounds write guards: +// - The _vl_insert_WI() guard for a destination whose width is not a multiple +// of 32. t_math_insert_bound.v covers the aligned case. +// - The V3Unknown guard for a select whose index expression is around 32 bits +// wide, where computing the index's maximum value can overflow. + +module t ( + clk +); + + input clk; + + // Each struct is 480 bits (15 words) with `dat` last, so a write to the + // nonexistent bit 500 lands on word 15, one past the end of the struct. + typedef struct packed { + logic [30:0] idx; + logic [448:0] dat; + } s31_t; + + typedef struct packed { + logic [31:0] idx; + logic [447:0] dat; + } s32_t; + + typedef struct packed { + logic [32:0] idx; + logic [446:0] dat; + } s33_t; + + s31_t s31; + s32_t s32; + s33_t s33; + + always_ff @(posedge clk) begin : blk + logic [443:0] v; + int sel; + + sel = 440; + void'($value$plusargs("SEL=%d", sel)); + + // Writes bits 471:440: the top four bits of `v`, then 28 that do not exist. + v = '0; + v[sel +: 32] = 32'hcafef00d; + + $write("v=%h\n", v[443:412]); + + // Bit 500 of `dat` does not exist. Index it with widths around 32 bits. + s31 = '0; + s32 = '0; + s33 = '0; + + // verilator lint_off WIDTHTRUNC + s31.idx = 500; + s31.dat[s31.idx] = 1'b1; + + s32.idx = 500; + s32.dat[s32.idx] = 1'b1; + + s33.idx = 500; + s33.dat[s33.idx] = 1'b1; + // verilator lint_on WIDTHTRUNC + + $write("dat=%h %h %h\n", s31.dat[446:415], s32.dat[445:414], s33.dat[444:413]); + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule