From 4901668f136e29a52e894e602bd58d1e846ed8fc Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 26 Dec 2017 21:35:08 -0500 Subject: [PATCH] Add INFINITELOOP warning, bug1254. --- Changes | 2 ++ bin/verilator | 12 ++++++++++++ src/V3Const.cpp | 17 ++++++++++++++++- src/V3Error.h | 3 ++- test_regress/t/t_lint_infinite.pl | 26 ++++++++++++++++++++++++++ test_regress/t/t_lint_infinite.v | 13 +++++++++++++ test_regress/t/t_repeat.v | 2 ++ 7 files changed, 73 insertions(+), 2 deletions(-) create mode 100755 test_regress/t/t_lint_infinite.pl create mode 100644 test_regress/t/t_lint_infinite.v diff --git a/Changes b/Changes index 1fa05c188..f5f035f74 100644 --- a/Changes +++ b/Changes @@ -8,6 +8,8 @@ The contributors that suggested a given feature are shown in []. Thanks! *** Support DPI open arrays, bug909, bug1245. [David Pierce, Victor Besyakov] +*** Add INFINITELOOP warning, bug1254. [Alex Solomatnikov] + **** Support > 64 bit decimal $display. **** Support DPI time and svLogicVal. [Victor Besyakov] diff --git a/bin/verilator b/bin/verilator index e714a029d..6582faa00 100755 --- a/bin/verilator +++ b/bin/verilator @@ -3275,6 +3275,18 @@ command line to specify the top include source directories. Disabled by default as this is a code style warning; it will simulate correctly. +=item INFINITELOOP + +Warns that a while or for statement has a condition that is always true. +and thus result in an infinite loop if the statement ever executes. + +This might be unintended behavior if the loop body contains statements that +in other statements that would make time pass, which Verilator is ignoring +due to e.g. STMTDLY warnings being disabled. + +Ignoring this warning will only suppress the lint check, it will simulate +correctly (i.e. hang due to the infinite loop). + =item INITIALDLY Warns that you have a delayed assignment inside of an initial or final diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 3759ab547..d14a7ad77 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -107,6 +107,7 @@ private: bool m_doShort; // Remove expressions that short circuit bool m_doV; // Verilog, not C++ conversion bool m_doGenerate; // Postpone width checking inside generate + bool m_hasJumpGo; // JumpGo under this while AstNodeModule* m_modp; // Current module AstArraySel* m_selp; // Current select AstNode* m_scopep; // Current scope @@ -2039,7 +2040,13 @@ private: nodep->iterateChildren(*this); } virtual void visit(AstWhile* nodep) { - nodep->iterateChildren(*this); + bool oldHasJumpGo = m_hasJumpGo; + m_hasJumpGo = false; + { + nodep->iterateChildren(*this); + } + bool thisWhileHasJumpGo = m_hasJumpGo; + m_hasJumpGo = thisWhileHasJumpGo || oldHasJumpGo; if (m_doNConst) { if (nodep->condp()->isZero()) { UINFO(4,"WHILE(0) => nop "<unlinkFrBack(); nodep->deleteTree(); VL_DANGLING(nodep); } + else if (nodep->condp()->isNeqZero()) { + if (!thisWhileHasJumpGo) { + nodep->v3warn(INFINITELOOP, "Infinite loop (condition always true)"); + nodep->fileline()->modifyWarnOff(V3ErrorCode::INFINITELOOP, true); // Complain just once + } + } else if (operandBoolShift(nodep->condp())) { replaceBoolShift(nodep->condp()); } @@ -2080,6 +2093,7 @@ private: virtual void visit(AstJumpGo* nodep) { nodep->iterateChildren(*this); + m_hasJumpGo = true; if (m_doExpensive) { nodep->labelp()->user4(true); } } @@ -2460,6 +2474,7 @@ public: m_doShort = true; // Presently always done m_doV = false; m_doGenerate = false; // Inside generate conditionals + m_hasJumpGo = false; m_warn = false; m_wremove = true; // Overridden in visitors m_modp = NULL; diff --git a/src/V3Error.h b/src/V3Error.h index 1a9fafd37..ede2da71e 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -80,6 +80,7 @@ public: IMPLICIT, // Implicit wire IMPURE, // Impure function not being inlined INCABSPATH, // Include has absolute path + INFINITELOOP, // Infinite loop INITIALDLY, // Initial delayed statement LITENDIAN, // Little bit endian vector MODDUP, // Duplicate module @@ -132,7 +133,7 @@ public: "CMPCONST", "COLONPLUS", "COMBDLY", "DEFPARAM", "DECLFILENAME", "ENDLABEL", "GENCLK", "IFDEPTH", "IMPERFECTSCH", "IMPLICIT", "IMPURE", - "INCABSPATH", "INITIALDLY", + "INCABSPATH", "INFINITELOOP", "INITIALDLY", "LITENDIAN", "MODDUP", "MULTIDRIVEN", "PINMISSING", "PINNOCONNECT", "PINCONNECTEMPTY", diff --git a/test_regress/t/t_lint_infinite.pl b/test_regress/t/t_lint_infinite.pl new file mode 100755 index 000000000..21a15f49f --- /dev/null +++ b/test_regress/t/t_lint_infinite.pl @@ -0,0 +1,26 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003-2009 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. + +$Self->{vlt} or $Self->skip("Verilator only test"); + +compile ( + v_flags2 => ["--lint-only"], + make_top_shell => 0, + make_main => 0, + verilator_make_gcc => 0, + fails=>1, + expect=> +'%Warning-INFINITELOOP: t/t_lint_infinite.v:\d+: Infinite loop \(condition always true\) +%Warning-INFINITELOOP: Use .* +%Warning-INFINITELOOP: t/t_lint_infinite.v:\d+: Infinite loop \(condition always true\) +.*', + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_infinite.v b/test_regress/t/t_lint_infinite.v new file mode 100644 index 000000000..7afebb5c4 --- /dev/null +++ b/test_regress/t/t_lint_infinite.v @@ -0,0 +1,13 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2017 by Wilson Snyder. + +module t (); + + initial begin + forever begin end + // verilator lint_off UNSIGNED + for (reg [31:0] i=0; i>=0; i=i+1) begin end + end +endmodule diff --git a/test_regress/t/t_repeat.v b/test_regress/t/t_repeat.v index b49007203..9d0c1a356 100644 --- a/test_regress/t/t_repeat.v +++ b/test_regress/t/t_repeat.v @@ -25,7 +25,9 @@ module t (/*AUTOARG*/); end if (times != 10) $stop; // + // verilator lint_off INFINITELOOP forever begin + // verilator lint_on INFINITELOOP $write("*-* All Finished *-*\n"); $finish; end