From eec41fd039b809ac67fe4b0a872d03f1b8490773 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 6 Jan 2024 18:49:19 -0500 Subject: [PATCH] Fix lint_off disables on preprocessor warnings (#4703). --- Changes | 1 + src/V3PreLex.h | 4 ++ src/V3PreLex.l | 51 +++++++++++++++++++---- test_regress/t/t_pp_dupdef_pragma_bad.out | 13 ++++++ test_regress/t/t_pp_dupdef_pragma_bad.pl | 19 +++++++++ test_regress/t/t_pp_dupdef_pragma_bad.v | 32 ++++++++++++++ 6 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 test_regress/t/t_pp_dupdef_pragma_bad.out create mode 100755 test_regress/t/t_pp_dupdef_pragma_bad.pl create mode 100644 test_regress/t/t_pp_dupdef_pragma_bad.v diff --git a/Changes b/Changes index c06d4479f..e5f0e02b2 100644 --- a/Changes +++ b/Changes @@ -16,6 +16,7 @@ Verilator 5.021 devel * Add predicted stack overflow warning (#4799). * Remove deprecated 32-bit pointer mode (`gcc -m32`). * Fix delays using wrong timeunits when modules inlined (#4806). [Paul Wright] +* Fix lint_off disables on preprocessor warnings (#4703). [Srinivasan Venkataramanan] Verilator 5.020 2024-01-01 diff --git a/src/V3PreLex.h b/src/V3PreLex.h index 95ae573f0..64e49ffcc 100644 --- a/src/V3PreLex.h +++ b/src/V3PreLex.h @@ -162,6 +162,7 @@ public: // Used only by V3PreLex.cpp and V3PreProc.cpp int m_streamDepth = 0; // Depth of stream processing YY_BUFFER_STATE m_bufferState; // Flex state FileLine* m_tokFilelinep; // Starting position of current token + std::deque m_lexLintState; // Current lint state for save/restore // State to lexer static V3PreLex* s_currentLexp; ///< Current lexing point @@ -209,6 +210,9 @@ public: // Used only by V3PreLex.cpp and V3PreProc.cpp } } void warnBackslashSpace(); + void verilatorCmtLint(const char* textp, bool warnOff); + void verilatorCmtLintRestore(); + void verilatorCmtLintSave(); // Called by V3PreProc.cpp to inform lexer void pushStateDefArg(int level); void pushStateDefForm(); diff --git a/src/V3PreLex.l b/src/V3PreLex.l index e8d882da4..429d1c3cc 100644 --- a/src/V3PreLex.l +++ b/src/V3PreLex.l @@ -66,7 +66,6 @@ static void appendDefValue(const char* t, size_t l) { LEXP->appendDefValue(t, l) %} %x ARGMODE -%x CMTBEGM %x CMTMODE %x CMTONEM %x DEFCMT @@ -420,6 +419,16 @@ bom [\357\273\277] [^\/\*\n\r\\(,){}\[\]\"`]+ | . { FL_FWDC; appendDefValue(yytext, yyleng); FL_BRK; } + /* Pragma comments. */ +"//"{ws}*"verilator lint_off"[^\n\r]* { FL_FWDC; LEXP->verilatorCmtLint(yytext + 2, true); return VP_COMMENT; } +"//"{ws}*"verilator lint_on"[^\n\r]* { FL_FWDC; LEXP->verilatorCmtLint(yytext + 2, false); return VP_COMMENT; } +"//"{ws}*"verilator lint_restore"[^\n\r]* { FL_FWDC; LEXP->verilatorCmtLintRestore(); return VP_COMMENT; } +"//"{ws}*"verilator lint_save"[^\n\r]* { FL_FWDC; LEXP->verilatorCmtLintSave(); return VP_COMMENT; } +"/*"{ws}*"verilator lint_off"[^*]*"*/" { FL_FWDC; LEXP->verilatorCmtLint(yytext + 2, true); return VP_COMMENT; } +"/*"{ws}*"verilator lint_on"[^*]*"*/" { FL_FWDC; LEXP->verilatorCmtLint(yytext + 2, false); return VP_COMMENT; } +"/*"{ws}*"verilator lint_restore"[^*]*"*/" { FL_FWDC; LEXP->verilatorCmtLintRestore(); return VP_COMMENT; } +"/*"{ws}*"verilator lint_save"[^*]*"*/" { FL_FWDC; LEXP->verilatorCmtLintSave(); return VP_COMMENT; } + /* One line comments. */ "//"{ws}*{crnl} { FL_FWDC; linenoInc(); yytext=(char*)"\n"; yyleng=1; return VP_WHITE; } "//" { yy_push_state(CMTONEM); yymore(); } @@ -427,16 +436,13 @@ bom [\357\273\277] /* C-style comments. */ /**** See also DEFCMT */ - /* We distinguish between the start of a comment, and later, to look for prefix comments (deprecated) */ "/*" { yy_push_state(CMTMODE); yymore(); } -{ws}+ { yymore(); } -"*/" { FL_FWDC; yy_pop_state(); return VP_COMMENT; } -{crnl} { linenoInc(); yymore(); } -<> { FL_FWDC; yyerrorf("EOF in '/* ... */' block comment\n"); +"*/" { FL_FWDC; yy_pop_state(); return VP_COMMENT; } +{crnl} { linenoInc(); yymore(); } +<> { FL_FWDC; yyerrorf("EOF in '/* ... */' block comment\n"); yyleng=0; return VP_EOF_ERROR; } -{word} { yymore(); } -. { yymore(); BEGIN CMTMODE; } /* beginning in comment */ -. { yymore(); } +{word} { yymore(); } +. { yymore(); } /* Define calls */ /* symbdef prevents normal lex rules from making `\`"foo a symbol {`"foo} instead of a BACKQUOTE */ @@ -735,6 +741,33 @@ void V3PreLex::unused() { } // LCOV_EXCL_STOP } +void V3PreLex::verilatorCmtLintSave() { + m_lexLintState.push_back(*curFilelinep()); +} +void V3PreLex::verilatorCmtLintRestore() { + // No error here on restore without save - the verilog.y parse will report as appropriate + if (m_lexLintState.empty()) return; + curFilelinep()->warnStateFrom(m_lexLintState.back()); + m_lexLintState.pop_back(); +} +void V3PreLex::verilatorCmtLint(const char* textp, bool warnOff) { + const char* sp = textp; + while (*sp && std::isspace(*sp)) ++sp; + while (*sp && !std::isspace(*sp)) ++sp; // "verilator" + while (*sp && std::isspace(*sp)) ++sp; + while (*sp && !std::isspace(*sp)) ++sp; // "lint_on/lint_off" + while (*sp && std::isspace(*sp)) ++sp; + string msg = sp; + for (auto pos = msg.begin(); pos != msg.end(); ++pos) { + if (std::isspace(*pos) || *pos == '*') { + msg.erase(pos, msg.end()); + break; + } + } + // No warnings on bad warning codes - the verilog.y parse will report as appropriate + curFilelinep()->warnOff(msg, warnOff); +} + /*################################################################### * Local Variables: * mode: C++ diff --git a/test_regress/t/t_pp_dupdef_pragma_bad.out b/test_regress/t/t_pp_dupdef_pragma_bad.out new file mode 100644 index 000000000..f9df6cc95 --- /dev/null +++ b/test_regress/t/t_pp_dupdef_pragma_bad.out @@ -0,0 +1,13 @@ +%Warning-REDEFMACRO: t/t_pp_dupdef_pragma_bad.v:11:19: Redefining existing define: 'DUP', with different value: 'b_bad' + t/t_pp_dupdef_pragma_bad.v:11:19: ... Location of previous definition, with value: 'a' + ... For warning description see https://verilator.org/warn/REDEFMACRO?v=latest + ... Use "/* verilator lint_off REDEFMACRO */" and lint_on around source to disable this message. +%Warning-REDEFMACRO: t/t_pp_dupdef_pragma_bad.v:16:19: Redefining existing define: 'DUP', with different value: 'd_bad' + t/t_pp_dupdef_pragma_bad.v:16:19: ... Location of previous definition, with value: 'c_nowarn' +%Warning-REDEFMACRO: t/t_pp_dupdef_pragma_bad.v:21:19: Redefining existing define: 'DUP', with different value: 'f_bad' + t/t_pp_dupdef_pragma_bad.v:21:19: ... Location of previous definition, with value: 'e_nowarn' +%Warning-REDEFMACRO: t/t_pp_dupdef_pragma_bad.v:26:19: Redefining existing define: 'DUP', with different value: 'k_bad' + t/t_pp_dupdef_pragma_bad.v:26:19: ... Location of previous definition, with value: 'j_nowarn' +%Warning-REDEFMACRO: t/t_pp_dupdef_pragma_bad.v:31:19: Redefining existing define: 'DUP', with different value: 'm_bad' + t/t_pp_dupdef_pragma_bad.v:31:19: ... Location of previous definition, with value: 'l_nowarn' +%Error: Exiting due to diff --git a/test_regress/t/t_pp_dupdef_pragma_bad.pl b/test_regress/t/t_pp_dupdef_pragma_bad.pl new file mode 100755 index 000000000..b5861b2ab --- /dev/null +++ b/test_regress/t/t_pp_dupdef_pragma_bad.pl @@ -0,0 +1,19 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2008 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); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_pp_dupdef_pragma_bad.v b/test_regress/t/t_pp_dupdef_pragma_bad.v new file mode 100644 index 000000000..3541805eb --- /dev/null +++ b/test_regress/t/t_pp_dupdef_pragma_bad.v @@ -0,0 +1,32 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2008 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +module t; + +`define DUP a +`define DUP b_bad + +// verilator lint_off REDEFMACRO +`define DUP c_nowarn +// verilator lint_on REDEFMACRO +`define DUP d_bad +// verilator lint_save +// verilator lint_off REDEFMACRO +`define DUP e_nowarn +// verilator lint_restore +`define DUP f_bad + +/* verilator lint_off REDEFMACRO */ +`define DUP j_nowarn +/* verilator lint_on REDEFMACRO */ +`define DUP k_bad +/* verilator lint_save */ +/* verilator lint_off REDEFMACRO */ +`define DUP l_nowarn +/* verilator lint_restore */ +`define DUP m_bad + +endmodule