From baa6a2c31ad88b6f5f5f7c8aca135121f7e7e11d Mon Sep 17 00:00:00 2001 From: Yves Mathieu Date: Thu, 24 Oct 2019 07:33:19 -0400 Subject: [PATCH] Support quoted arguments in -f files, bug1535. Signed-off-by: Wilson Snyder --- Changes | 2 + src/V3Options.cpp | 90 +++++++++++++++++++++++++----- test_regress/t/t_flag_define.v | 52 +++++++++++++++++ test_regress/t/t_flag_define.vc | 6 ++ test_regress/t/t_flag_parameter.pl | 2 +- test_regress/t/t_flag_parameter.v | 32 +++++++++++ test_regress/t/t_flag_parameter.vc | 10 ++++ 7 files changed, 178 insertions(+), 16 deletions(-) diff --git a/Changes b/Changes index 9ceb29a6e..8e7470ac5 100644 --- a/Changes +++ b/Changes @@ -16,6 +16,8 @@ The contributors that suggested a given feature are shown in []. Thanks! **** Increase case duplicate/incomplete to 16 bit tables, bug1545. [Yossi Nivin] +**** Support quoted arguments in -f files, bug1535. [Yves Mathieu] + **** Fix multithreaded yield behavior when no work. [Patrick Stewart] **** Fix bad-syntax crashes, bug1548, bug1550-1553, bug1557-1560, bug1563. [Eric Rippey] diff --git a/src/V3Options.cpp b/src/V3Options.cpp index fc4903091..fbe391fc9 100644 --- a/src/V3Options.cpp +++ b/src/V3Options.cpp @@ -1236,25 +1236,85 @@ void V3Options::parseOptsFile(FileLine* fl, const string& filename, bool rel) { fl = new FileLine(filename); // Split into argument list and process - // Note we don't respect quotes. It seems most simulators dont. - // Woez those that expect it; we'll at least complain. - if (whole_file.find('\"') != string::npos) { - fl->v3error("Double quotes in -f files cause unspecified behavior."); - } + // Note we try to respect escaped char, double/simple quoted strings + // Other simulators don't respect a common syntax... // Strip off arguments and parse into words std::vector args; - string::size_type startpos = 0; - while (startpos < whole_file.length()) { - while (isspace(whole_file[startpos])) ++startpos; - string::size_type endpos = startpos; - while (endpos < whole_file.length() && !isspace(whole_file[endpos])) ++endpos; - if (startpos != endpos) { - string arg (whole_file, startpos, endpos-startpos); - args.reserve(args.size()+1); - args.push_back(arg); + + // Parse file using a state machine, taking into account quoted strings and escaped chars + enum state {ST_IN_OPTION, + ST_ESCAPED_CHAR, + ST_IN_QUOTED_STR, + ST_IN_DOUBLE_QUOTED_STR}; + + state st = ST_IN_OPTION; + state last_st; + string arg; + for (string::size_type pos = 0; + pos < whole_file.length(); ++pos) { + char curr_char = whole_file[pos]; + switch (st) { + case ST_IN_OPTION: // Get all chars up to a white space or a "=" + if (isspace(curr_char)) { // End of option + if (!arg.empty()) { // End of word + args.push_back(arg); + } + arg = ""; + break; + } + if (curr_char == '\\') { // Escape char, we wait for next char + last_st = st; // Memorize current state + st = ST_ESCAPED_CHAR; + break; + } + if (curr_char == '\'') { // Find begin of quoted string + // Examine next char in order to decide between + // a string or a base specifier for integer literal + ++pos; + if (pos < whole_file.length()) curr_char = whole_file[pos]; + if (curr_char == '"') { // String + st = ST_IN_QUOTED_STR; + } else { // Base specifier + arg += '\''; + } + arg += curr_char; + break; + } + if (curr_char == '"') { // Find begin of double quoted string + // Doesn't insert the quote + st = ST_IN_DOUBLE_QUOTED_STR; + break; + } + arg += curr_char; + break; + case ST_IN_QUOTED_STR: // Just store all chars inside string + if (curr_char != '\'') { + arg += curr_char; + } else { // End of quoted string + st = ST_IN_OPTION; + } + break; + case ST_IN_DOUBLE_QUOTED_STR: // Take into account escaped chars + if (curr_char != '"') { + if (curr_char == '\\') { + last_st = st; + st = ST_ESCAPED_CHAR; + } else { + arg += curr_char; + } + } else { // End of double quoted string + st = ST_IN_OPTION; + } + break; + case ST_ESCAPED_CHAR: // Just add the escaped char + arg += curr_char; + st = last_st; + break; } - startpos = endpos; + } + if (!arg.empty()) { // Add last word + args.push_back(arg); } // Path diff --git a/test_regress/t/t_flag_define.v b/test_regress/t/t_flag_define.v index d26cc1854..11a163184 100644 --- a/test_regress/t/t_flag_define.v +++ b/test_regress/t/t_flag_define.v @@ -2,6 +2,22 @@ // // This file ONLY is placed into the Public Domain, for any use, // without warranty, 2014 by Wilson Snyder +// +// Special cases of "string parameters" : +// This table compares obtain results from big-3 simulators to Verilator +// expected behavior. Base specified integer literals are also included as +// string detection may impact results for such cases. +// +// | In the options file | simulator 1 | simulator 2 | simulator 3 | verilator | +// |----------------------- ---|-------------|-------------|-------------|-------------| +// | +define+C0='"AB CD"' | AB CD | UNSUPPORTED | AB CD | AB CD | +// | +define+C1=\"AB\ CD\" | AB CD | UNSUPPORTED | AB CD | AB CD | +// | +define+C2="\"AB CD\"" | AB CD | AB CD | UNSUPPORTED | AB CD | +// | +define+C3="\"AB\ CD\"" | AB CD | AB CD | UNSUPPORTED | AB CD | +// | +define+C4=32'h600D600D | UNSUPPORTED | 32'h600D600D| 32'h600D600D| 32'h600D600D| +// | +define+C5=32\'h600D600D | 32'h600D600D| UNSUPPORTED | UNSUPPORTED | 32'h600D600D| +// | +define+C6="32'h600D600D" | 32'h600D600D| 32'h600D600D| 32'h600D600D| 32'h600D600D| +// | +define+C7='AB CD' | AB CD | UNSUPPORTED | UNSUPPORTED | UNSUPPORTED | `define STRINGIFY(x) `"x`" @@ -55,6 +71,42 @@ module t; $write("%%Error: Missing define\n"); $stop; `endif +`ifdef STRING1 + if (`STRING1 !== "New String") $stop; +`else + $write("%%Error: Missing define\n"); $stop; +`endif + +`ifdef STRING2 + if (`STRING2 !== "New String") $stop; +`else + $write("%%Error: Missing define\n"); $stop; +`endif + +`ifdef STRING3 + if (`STRING3 !== "New String") $stop; +`else + $write("%%Error: Missing define\n"); $stop; +`endif + +`ifdef LIT1 + if (`STRINGIFY(`LIT1) !== "32'h600D600D") $stop; +`else + $write("%%Error: Missing define\n"); $stop; +`endif + +`ifdef LIT2 + if (`STRINGIFY(`LIT2) !== "32'h600D600D") $stop; +`else + $write("%%Error: Missing define\n"); $stop; +`endif + +`ifdef LIT3 + if (`STRINGIFY(`LIT3) !== "32'h600D600D") $stop; +`else + $write("%%Error: Missing define\n"); $stop; +`endif + $write("*-* All Finished *-*\n"); $finish; end diff --git a/test_regress/t/t_flag_define.vc b/test_regress/t/t_flag_define.vc index f0b52c58f..f6ddea042 100644 --- a/test_regress/t/t_flag_define.vc +++ b/test_regress/t/t_flag_define.vc @@ -5,3 +5,9 @@ +define+D5A=VALA+D5B=VALB // Quotes do NOT escape the plus //+define+D5A="VALA+D5B"+D5C ++define+STRING1="\"New String\"" ++define+STRING2='"New String"' ++define+STRING3=\"New\ String\" ++define+LIT1=32'h600D600D ++define+LIT2=32\'h600D600D ++define+LIT3="32'h600D600D" diff --git a/test_regress/t/t_flag_parameter.pl b/test_regress/t/t_flag_parameter.pl index 2b3c896c1..6152aa8ec 100755 --- a/test_regress/t/t_flag_parameter.pl +++ b/test_regress/t/t_flag_parameter.pl @@ -11,7 +11,7 @@ scenarios(vlt => 1); compile( # It is not possible to put them into the options file - v_flags2 => ['-Gstring1="\"New String\"" -pvalue+string2="\"New String\"" -f t/t_flag_parameter.vc'], + v_flags2 => ['-f t/t_flag_parameter.vc'], ); execute( diff --git a/test_regress/t/t_flag_parameter.v b/test_regress/t/t_flag_parameter.v index 77dd585e3..faad97e97 100644 --- a/test_regress/t/t_flag_parameter.v +++ b/test_regress/t/t_flag_parameter.v @@ -2,12 +2,32 @@ // // This file ONLY is placed into the Public Domain, for any use, // without warranty, 2016 by Wilson Snyder +// +// Special cases of "string parameters" : +// This table compares obtain results from big-3 simulators to Verilator +// expected behavior. Base specified integer literals are also included as +// string detection may impact results for such cases. +// +// | Option/Param file | simulator 1 | simulator 2 | simulator 3 | verilator | +// |---------------------|-------------|-------------|-------------|-------------| +// | -gC0='"AB CD"' | AB CD | UNSUPPORTED | AB CD | AB CD | +// | -gC1=\"AB\ CD\" | AB CD | UNSUPPORTED | UNSUPPORTED | AB CD | +// | -gC2="\"AB CD\"" | AB CD | AB CD | AB CD | AB CD | +// | -gC3="\"AB\ CD\"" | AB CD | AB\\ CD | AB CD | AB CD | +// | -gC4=32'h600D600D | UNSUPPORTED | 32'h600D600D| 32'h600D600D| 32'h600D600D| +// | -gC5=32\'h600D600D | 32'h600D600D| UNSUPPORTED | UNSUPPORTED | 32'h600D600D| +// | -gC6="32'h600D600D" | 32'h600D600D| 32'h600D600D| UNSUPPORTED | 32'h600D600D| +// | -gC7='AB CD' | AB CD | UNSUPPORTED | UNSUPPORTED | UNSUPPORTED | `define check(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: Wrong parameter value", `__FILE__,`__LINE__); $stop; end while(0); module t; parameter string1 = "Original String"; parameter string2 = "Original String"; + parameter string11 = "Original String"; + parameter string12 = "Original String"; + parameter string21 = "Original String"; + parameter string22 = "Original String"; parameter real11 = 0.1; parameter real12 = 0.1; @@ -24,10 +44,18 @@ module t; parameter int32 = 1; parameter int41 = 1; parameter int42 = 1; + parameter int51 = 1; + parameter int52 = 1; + parameter int61 = 1; + parameter int62 = 1; initial begin `check(string1,"New String"); `check(string2,"New String"); + `check(string11,"New String"); + `check(string12,"New String"); + `check(string21,"New String"); + `check(string22,"New String"); `check(real11,0.2); `check(real12,0.2); `check(real21,400); @@ -42,6 +70,10 @@ module t; `check(int32,123); `check(int41,32'hdeadbeef); `check(int42,32'hdeadbeef); + `check(int51,32'hdeadbeef); + `check(int52,32'hdeadbeef); + `check(int61,32'hdeadbeef); + `check(int62,32'hdeadbeef); $write("*-* All Finished *-*\n"); $finish; diff --git a/test_regress/t/t_flag_parameter.vc b/test_regress/t/t_flag_parameter.vc index 7946f2b28..49e71571d 100644 --- a/test_regress/t/t_flag_parameter.vc +++ b/test_regress/t/t_flag_parameter.vc @@ -1,3 +1,9 @@ +-Gstring1="\"New String\"" +-pvalue+string2="\"New String\"" +-Gstring11='"New String"' +-pvalue+string12='"New String"' +-Gstring21=\"New\ String\" +-pvalue+string22=\"New\ String\" -Greal11=0.2 -pvalue+real12=0.2 -Greal21=4e2 @@ -12,3 +18,7 @@ -pvalue+int32=123 -Gint41=32'hdead_beef -pvalue+int42=32'hdead_beef +-Gint51=32\'hdead_beef +-pvalue+int52=32\'hdead_beef +-Gint61="32'hdead_beef" +-pvalue+int62="32'hdead_beef"