From bee344d1aec2939dfe4bb9151169740eba016b92 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 26 Nov 2024 21:06:43 -0500 Subject: [PATCH] Add error on illegal `--prefix` etc. values (#5507). --- Changes | 1 + src/V3Options.cpp | 24 ++++++++++++++++++++---- src/V3Options.h | 1 + src/V3PreProc.cpp | 2 +- src/V3String.cpp | 11 +++++++++-- src/V3String.h | 6 ++++-- test_regress/t/t_flag_libcreate_bad.out | 2 ++ test_regress/t/t_flag_libcreate_bad.py | 19 +++++++++++++++++++ test_regress/t/t_flag_modprefix_bad.out | 2 ++ test_regress/t/t_flag_modprefix_bad.py | 19 +++++++++++++++++++ test_regress/t/t_flag_prefix_bad.out | 2 ++ test_regress/t/t_flag_prefix_bad.py | 17 +++++++++++++++++ 12 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 test_regress/t/t_flag_libcreate_bad.out create mode 100755 test_regress/t/t_flag_libcreate_bad.py create mode 100644 test_regress/t/t_flag_modprefix_bad.out create mode 100755 test_regress/t/t_flag_modprefix_bad.py create mode 100644 test_regress/t/t_flag_prefix_bad.out create mode 100755 test_regress/t/t_flag_prefix_bad.py diff --git a/Changes b/Changes index 1e028eac2..bd47bdfd3 100644 --- a/Changes +++ b/Changes @@ -26,6 +26,7 @@ Verilator 5.031 devel * Add error on illegal enum base type (#3010). [Iztok Jeras] * Add error on `wait` with missing `.triggered` (#4457). * Add error when improperly storing to parameter (#5147). [Gökçe Aydos] +* Add error on illegal `--prefix` etc. values (#5507). [Fabian Keßler] * Add coverage point hierarchy to coverage reports (#5575) (#5576). [Andrew Nolte] * Add warning on global constraints (#5625). [Ryszard Rozak, Antmicro Ltd.] * Add error on `solve before` or soft constraints of `randc` variable. diff --git a/src/V3Options.cpp b/src/V3Options.cpp index a5ea5f7e0..01b57af48 100644 --- a/src/V3Options.cpp +++ b/src/V3Options.cpp @@ -561,6 +561,12 @@ int V3Options::stripOptionsForChildRun(const string& opt, bool forTop) { return 0; } +void V3Options::validateIdentifier(FileLine* fl, const string& arg, const string& opt) { + if (!VString::isIdentifier(arg)) { + fl->v3error(opt << " argument must be a legal C++ identifier: '" << arg << "'"); + } +} + string V3Options::filePath(FileLine* fl, const string& modname, const string& lastpath, const string& errmsg) { // Error prefix or "" to suppress error // Find a filename to read the specified module name, @@ -1386,7 +1392,10 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, }; DECL_OPTION("-default-language", CbVal, setLang); DECL_OPTION("-language", CbVal, setLang); - DECL_OPTION("-lib-create", Set, &m_libCreate); + DECL_OPTION("-lib-create", CbVal, [this, fl](const char* valp) { + validateIdentifier(fl, valp, "--lib-create"); + m_libCreate = valp; + }); DECL_OPTION("-lint-only", OnOff, &m_lintOnly); DECL_OPTION("-localize-max-size", Set, &m_localizeMaxSize); DECL_OPTION("-main-top-name", Set, &m_mainTopName); @@ -1409,7 +1418,10 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, } }); DECL_OPTION("-max-num-width", Set, &m_maxNumWidth); - DECL_OPTION("-mod-prefix", Set, &m_modPrefix); + DECL_OPTION("-mod-prefix", CbVal, [this, fl](const char* valp) { + validateIdentifier(fl, valp, "--mod-prefix"); + m_modPrefix = valp; + }); DECL_OPTION("-O0", CbCall, [this]() { optimize(0); }); DECL_OPTION("-O1", CbCall, [this]() { optimize(1); }); @@ -1460,7 +1472,10 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, DECL_OPTION("-pins-uint8", OnOff, &m_pinsUint8); DECL_OPTION("-pipe-filter", Set, &m_pipeFilter); DECL_OPTION("-pp-comments", OnOff, &m_ppComments); - DECL_OPTION("-prefix", Set, &m_prefix); + DECL_OPTION("-prefix", CbVal, [this, fl](const char* valp) { + validateIdentifier(fl, valp, "--prefix"); + m_prefix = valp; + }); DECL_OPTION("-private", CbCall, [this]() { m_public = false; }); DECL_OPTION("-prof-c", OnOff, &m_profC); DECL_OPTION("-prof-cfuncs", CbCall, [this]() { m_profC = m_profCFuncs = true; }); @@ -1470,7 +1485,8 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, DECL_OPTION("-prof-pgo", OnOff, &m_profPgo); DECL_OPTION("-protect-ids", OnOff, &m_protectIds); DECL_OPTION("-protect-key", Set, &m_protectKey); - DECL_OPTION("-protect-lib", CbVal, [this](const char* valp) { + DECL_OPTION("-protect-lib", CbVal, [this, fl](const char* valp) { + validateIdentifier(fl, valp, "--protect-lib"); m_libCreate = valp; m_protectIds = true; }); diff --git a/src/V3Options.h b/src/V3Options.h index c60a86224..f7a05a022 100644 --- a/src/V3Options.h +++ b/src/V3Options.h @@ -428,6 +428,7 @@ private: static string parseFileArg(const string& optdir, const string& relfilename); string filePathCheckOneDir(const string& modname, const string& dirname); static int stripOptionsForChildRun(const string& opt, bool forTop); + void validateIdentifier(FileLine* fl, const string& arg, const string& opt); // CONSTRUCTORS VL_UNCOPYABLE(V3Options); diff --git a/src/V3PreProc.cpp b/src/V3PreProc.cpp index 6cb95aa28..c2cb514b3 100644 --- a/src/V3PreProc.cpp +++ b/src/V3PreProc.cpp @@ -490,7 +490,7 @@ void V3PreProcImp::comment(const string& text) { if (VString::startsWith(cmd, "public_flat_rw")) { // "/*verilator public_flat_rw @(foo) */" -> "/*verilator public_flat_rw*/ @(foo)" string::size_type endOfCmd = std::strlen("public_flat_rw"); - while (VString::isWordChar(cmd[endOfCmd])) ++endOfCmd; + while (VString::isIdentifierChar(cmd[endOfCmd])) ++endOfCmd; string baseCmd = cmd.substr(0, endOfCmd); string arg = cmd.substr(endOfCmd); while (std::isspace(arg[0])) arg = arg.substr(1); diff --git a/src/V3String.cpp b/src/V3String.cpp index d79a6ff58..c0f8f036d 100644 --- a/src/V3String.cpp +++ b/src/V3String.cpp @@ -216,6 +216,13 @@ string VString::removeWhitespace(const string& str) { return result; } +bool VString::isIdentifier(const string& str) { + for (const char c : str) { + if (!isIdentifierChar(c)) return false; + } + return true; +} + bool VString::isWhitespace(const string& str) { for (const char c : str) { if (!std::isspace(c)) return false; @@ -256,8 +263,8 @@ string VString::replaceWord(const string& str, const string& from, const string& UASSERT_STATIC(len > 0, "Cannot replace empty string"); for (size_t pos = 0; (pos = result.find(from, pos)) != string::npos; pos += len) { // Only replace whole words - if (((pos > 0) && VString::isWordChar(result[pos - 1])) || // - ((pos + len < result.size()) && VString::isWordChar(result[pos + len]))) { + if (((pos > 0) && VString::isIdentifierChar(result[pos - 1])) || // + ((pos + len < result.size()) && VString::isIdentifierChar(result[pos + len]))) { continue; } result.replace(pos, len, to); diff --git a/src/V3String.h b/src/V3String.h index a69036cc9..83db7eed9 100644 --- a/src/V3String.h +++ b/src/V3String.h @@ -114,6 +114,10 @@ public: static string spaceUnprintable(const string& str) VL_PURE; // Remove any whitespace static string removeWhitespace(const string& str); + // Return true if only identifer or "" + static bool isIdentifier(const string& str); + // Return true if char is valid character in C identifiers + static bool isIdentifierChar(char c) { return isalnum(c) || c == '_'; } // Return true if only whitespace or "" static bool isWhitespace(const string& str); // Return number of spaces/tabs leading in string @@ -128,8 +132,6 @@ public: static bool startsWith(const string& str, const string& prefix); // Predicate to check if 'str' ends with 'suffix' static bool endsWith(const string& str, const string& suffix); - // Return true if char is valid character in word - static bool isWordChar(char c) { return isalnum(c) || c == '_'; } // Return proper article (a/an) for a word. May be inaccurate for some special words static string aOrAn(const char* word); static string aOrAn(const string& word) { return aOrAn(word.c_str()); } diff --git a/test_regress/t/t_flag_libcreate_bad.out b/test_regress/t/t_flag_libcreate_bad.out new file mode 100644 index 000000000..c4912f7c5 --- /dev/null +++ b/test_regress/t/t_flag_libcreate_bad.out @@ -0,0 +1,2 @@ +%Error: --lib-create argument must be a legal C++ identifier: 'bad/name' +%Error: Exiting due to diff --git a/test_regress/t/t_flag_libcreate_bad.py b/test_regress/t/t_flag_libcreate_bad.py new file mode 100755 index 000000000..8923eb5f0 --- /dev/null +++ b/test_regress/t/t_flag_libcreate_bad.py @@ -0,0 +1,19 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2024 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.top_filename = 't/t_EXAMPLE.v' # Anything + +test.lint(verilator_flags2=["--lib-create bad/name"], + fails=True, + expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_flag_modprefix_bad.out b/test_regress/t/t_flag_modprefix_bad.out new file mode 100644 index 000000000..e20cabf72 --- /dev/null +++ b/test_regress/t/t_flag_modprefix_bad.out @@ -0,0 +1,2 @@ +%Error: --mod-prefix argument must be a legal C++ identifier: 'bad/name' +%Error: Exiting due to diff --git a/test_regress/t/t_flag_modprefix_bad.py b/test_regress/t/t_flag_modprefix_bad.py new file mode 100755 index 000000000..7d78e05da --- /dev/null +++ b/test_regress/t/t_flag_modprefix_bad.py @@ -0,0 +1,19 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2024 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.top_filename = 't/t_EXAMPLE.v' # Anything + +test.lint(verilator_flags2=["--mod-prefix bad/name"], + fails=True, + expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_flag_prefix_bad.out b/test_regress/t/t_flag_prefix_bad.out new file mode 100644 index 000000000..d87a75861 --- /dev/null +++ b/test_regress/t/t_flag_prefix_bad.out @@ -0,0 +1,2 @@ +%Error: --prefix argument must be a legal C++ identifier: 'bad/name' +%Error: Exiting due to diff --git a/test_regress/t/t_flag_prefix_bad.py b/test_regress/t/t_flag_prefix_bad.py new file mode 100755 index 000000000..70b21989f --- /dev/null +++ b/test_regress/t/t_flag_prefix_bad.py @@ -0,0 +1,17 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2024 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.top_filename = 't/t_EXAMPLE.v' # Anything + +test.lint(verilator_flags2=["--prefix bad/name"], fails=True, expect_filename=test.golden_filename) + +test.passes()