From c7440b250f2feb76f871a0a568cfc17f41f7158a Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 26 Mar 2022 22:47:10 +0000 Subject: [PATCH] Validate integer run-time arguments --- include/verilated.cpp | 71 +++++++++++++++++++++--------- include/verilated_imp.h | 7 ++- test_regress/t/t_runflag_bad.out | 2 - test_regress/t/t_runflag_bad.out-a | 2 + test_regress/t/t_runflag_bad.out-b | 2 + test_regress/t/t_runflag_bad.out-c | 2 + test_regress/t/t_runflag_bad.out-d | 2 + test_regress/t/t_runflag_bad.out-e | 2 + test_regress/t/t_runflag_bad.pl | 28 +++++++++++- 9 files changed, 93 insertions(+), 25 deletions(-) delete mode 100644 test_regress/t/t_runflag_bad.out create mode 100644 test_regress/t/t_runflag_bad.out-a create mode 100644 test_regress/t/t_runflag_bad.out-b create mode 100644 test_regress/t/t_runflag_bad.out-c create mode 100644 test_regress/t/t_runflag_bad.out-d create mode 100644 test_regress/t/t_runflag_bad.out-e diff --git a/include/verilated.cpp b/include/verilated.cpp index 4401ffb45..75eaaefbf 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -55,6 +55,7 @@ #include #include #include +#include #include #include // mkdir #include @@ -2506,13 +2507,16 @@ std::pair VerilatedContextImp::argc_argv() VL_MT_SAFE_EXCLUDES(m_ar void VerilatedContextImp::commandArgVl(const std::string& arg) { if (0 == std::strncmp(arg.c_str(), "+verilator+", std::strlen("+verilator+"))) { - std::string value; + std::string str; + uint64_t u64; if (arg == "+verilator+debug") { Verilated::debug(4); - } else if (commandArgVlValue(arg, "+verilator+debugi+", value /*ref*/)) { - Verilated::debug(std::atoi(value.c_str())); - } else if (commandArgVlValue(arg, "+verilator+error+limit+", value /*ref*/)) { - errorLimit(std::atoi(value.c_str())); + } else if (commandArgVlUint64(arg, "+verilator+debugi+", u64, 0, + std::numeric_limits::max())) { + Verilated::debug(static_cast(u64)); + } else if (commandArgVlUint64(arg, "+verilator+error+limit+", u64, 0, + std::numeric_limits::max())) { + errorLimit(static_cast(u64)); } else if (arg == "+verilator+help") { VerilatedImp::versionDump(); VL_PRINTF_MT("For help, please see 'verilator --help'\n"); @@ -2520,18 +2524,19 @@ void VerilatedContextImp::commandArgVl(const std::string& arg) { "Exiting due to command line argument (not an error)"); } else if (arg == "+verilator+noassert") { assertOn(false); - } else if (commandArgVlValue(arg, "+verilator+prof+threads+start+", value /*ref*/)) { - profThreadsStart(std::atoll(value.c_str())); - } else if (commandArgVlValue(arg, "+verilator+prof+threads+window+", value /*ref*/)) { - profThreadsWindow(std::atol(value.c_str())); - } else if (commandArgVlValue(arg, "+verilator+prof+threads+file+", value /*ref*/)) { - profThreadsFilename(value); - } else if (commandArgVlValue(arg, "+verilator+prof+vlt+file+", value /*ref*/)) { - profVltFilename(value); - } else if (commandArgVlValue(arg, "+verilator+rand+reset+", value /*ref*/)) { - randReset(std::atoi(value.c_str())); - } else if (commandArgVlValue(arg, "+verilator+seed+", value /*ref*/)) { - randSeed(std::atoi(value.c_str())); + } else if (commandArgVlUint64(arg, "+verilator+prof+threads+start+", u64)) { + profThreadsStart(u64); + } else if (commandArgVlUint64(arg, "+verilator+prof+threads+window+", u64, 1)) { + profThreadsWindow(u64); + } else if (commandArgVlString(arg, "+verilator+prof+threads+file+", str)) { + profThreadsFilename(str); + } else if (commandArgVlString(arg, "+verilator+prof+vlt+file+", str)) { + profVltFilename(str); + } else if (commandArgVlUint64(arg, "+verilator+rand+reset+", u64, 0, 2)) { + randReset(static_cast(u64)); + } else if (commandArgVlUint64(arg, "+verilator+seed+", u64, 1, + std::numeric_limits::max())) { + randSeed(static_cast(u64)); } else if (arg == "+verilator+V") { VerilatedImp::versionDump(); // Someday more info too VL_FATAL_MT("COMMAND_LINE", 0, "", @@ -2541,12 +2546,14 @@ void VerilatedContextImp::commandArgVl(const std::string& arg) { VL_FATAL_MT("COMMAND_LINE", 0, "", "Exiting due to command line argument (not an error)"); } else { - VL_PRINTF_MT("%%Warning: Unknown +verilator runtime argument: '%s'\n", arg.c_str()); + const std::string msg = "Unknown runtime argument: " + arg; + VL_FATAL_MT("COMMAND_LINE", 0, "", msg.c_str()); } } } -bool VerilatedContextImp::commandArgVlValue(const std::string& arg, const std::string& prefix, - std::string& valuer) { + +bool VerilatedContextImp::commandArgVlString(const std::string& arg, const std::string& prefix, + std::string& valuer) { const size_t len = prefix.length(); if (0 == std::strncmp(prefix.c_str(), arg.c_str(), len)) { valuer = arg.substr(len); @@ -2556,6 +2563,30 @@ bool VerilatedContextImp::commandArgVlValue(const std::string& arg, const std::s } } +bool VerilatedContextImp::commandArgVlUint64(const std::string& arg, const std::string& prefix, + uint64_t& valuer, uint64_t min, uint64_t max) { + std::string str; + if (commandArgVlString(arg, prefix, str)) { + const auto fail = [&](const std::string& extra = "") { + std::stringstream ss; + ss << "Argument '" << prefix << "' must be an unsigned integer"; + if (min != std::numeric_limits::min()) ss << ", greater than " << min - 1; + if (max != std::numeric_limits::max()) ss << ", less than " << max + 1; + if (!extra.empty()) ss << ". " << extra; + const std::string& msg = ss.str(); + VL_FATAL_MT("COMMAND_LINE", 0, "", msg.c_str()); + }; + + if (std::any_of(str.begin(), str.end(), [](int c) { return !std::isdigit(c); })) fail(); + char* end; + valuer = std::strtoull(str.c_str(), &end, 10); + if (errno == ERANGE) fail("Value out of range of uint64_t"); + if (valuer < min || valuer > max) fail(); + return true; + } + return false; +} + //====================================================================== // VerilatedContext:: + VerilatedContextImp:: Methods - random diff --git a/include/verilated_imp.h b/include/verilated_imp.h index 7f07587db..c8b5ba352 100644 --- a/include/verilated_imp.h +++ b/include/verilated_imp.h @@ -34,6 +34,7 @@ #include #include +#include #include #include #include @@ -394,7 +395,11 @@ protected: // METHODS - protected void commandArgsAddGuts(int argc, const char** argv); void commandArgVl(const std::string& arg); - bool commandArgVlValue(const std::string& arg, const std::string& prefix, std::string& valuer); + bool commandArgVlString(const std::string& arg, const std::string& prefix, + std::string& valuer); + bool commandArgVlUint64(const std::string& arg, const std::string& prefix, uint64_t& valuer, + uint64_t min = std::numeric_limits::min(), + uint64_t max = std::numeric_limits::max()); void commandArgDump() const VL_MT_SAFE_EXCLUDES(m_argMutex); }; diff --git a/test_regress/t/t_runflag_bad.out b/test_regress/t/t_runflag_bad.out deleted file mode 100644 index 32330deaf..000000000 --- a/test_regress/t/t_runflag_bad.out +++ /dev/null @@ -1,2 +0,0 @@ -%Warning: Unknown +verilator runtime argument: '+verilator+bad+flag+testing' -*-* All Finished *-* diff --git a/test_regress/t/t_runflag_bad.out-a b/test_regress/t/t_runflag_bad.out-a new file mode 100644 index 000000000..f1fb8cf2c --- /dev/null +++ b/test_regress/t/t_runflag_bad.out-a @@ -0,0 +1,2 @@ +%Error: COMMAND_LINE:0: Unknown runtime argument: +verilator+bad+flag+testing +Aborting... diff --git a/test_regress/t/t_runflag_bad.out-b b/test_regress/t/t_runflag_bad.out-b new file mode 100644 index 000000000..9df23a4e7 --- /dev/null +++ b/test_regress/t/t_runflag_bad.out-b @@ -0,0 +1,2 @@ +%Error: COMMAND_LINE:0: Argument '+verilator+rand+reset+' must be an unsigned integer, less than 3 +Aborting... diff --git a/test_regress/t/t_runflag_bad.out-c b/test_regress/t/t_runflag_bad.out-c new file mode 100644 index 000000000..9df23a4e7 --- /dev/null +++ b/test_regress/t/t_runflag_bad.out-c @@ -0,0 +1,2 @@ +%Error: COMMAND_LINE:0: Argument '+verilator+rand+reset+' must be an unsigned integer, less than 3 +Aborting... diff --git a/test_regress/t/t_runflag_bad.out-d b/test_regress/t/t_runflag_bad.out-d new file mode 100644 index 000000000..f46b3bcaa --- /dev/null +++ b/test_regress/t/t_runflag_bad.out-d @@ -0,0 +1,2 @@ +%Error: COMMAND_LINE:0: Argument '+verilator+prof+threads+window+' must be an unsigned integer, greater than 0 +Aborting... diff --git a/test_regress/t/t_runflag_bad.out-e b/test_regress/t/t_runflag_bad.out-e new file mode 100644 index 000000000..7ad4052fd --- /dev/null +++ b/test_regress/t/t_runflag_bad.out-e @@ -0,0 +1,2 @@ +%Error: COMMAND_LINE:0: Argument '+verilator+prof+threads+window+' must be an unsigned integer, greater than 0. Value out of range of uint64_t +Aborting... diff --git a/test_regress/t/t_runflag_bad.pl b/test_regress/t/t_runflag_bad.pl index 7f6ecf79f..fd3637bc3 100755 --- a/test_regress/t/t_runflag_bad.pl +++ b/test_regress/t/t_runflag_bad.pl @@ -15,8 +15,32 @@ compile( execute( all_run_flags => ["+verilator+bad+flag+testing"], - #fails => 1, # doesn't fail just prints - expect_filename => $Self->{golden_filename}, + fails => 1, + expect_filename => $Self->{golden_filename} . "-a", + ); + +execute( + all_run_flags => ["+verilator+rand+reset+-1"], + fails => 1, + expect_filename => $Self->{golden_filename} . "-b" + ); + +execute( + all_run_flags => ["+verilator+rand+reset+3"], + fails => 1, + expect_filename => $Self->{golden_filename} . "-c" + ); + +execute( + all_run_flags => ["+verilator+prof+threads+window+0"], + fails => 1, + expect_filename => $Self->{golden_filename} . "-d" + ); + +execute( + all_run_flags => ["+verilator+prof+threads+window+1000000000000000000000000"], + fails => 1, + expect_filename => $Self->{golden_filename} . "-e" ); ok(1);