luke
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix SEGV when $fgets, $sscanf, or $fscanf is used with string (#2604) 

* Add a test to use string for $fgets

* Use dedicated function for $fgets to std::string

* share the implementation of $fgets

* Pass -1 for bitwidth of std::string to distinguish from POD

* add checks for scanf with string

* apply clang-format
This commit is contained in:
Yutetsu TAKATSUKASA 2020-10-28 08:37:12 +09:00 committed by GitHub
parent 77ac9bfcc6
commit 05ff96bea3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 79 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
include/verilated.cpp
View File

@ -1079,7 +1079,17 @@ IData _vl_vsscanf(FILE* fp, // If a fscanf
WData qowp[VL_WQ_WORDS_E];
VL_SET_WQ(qowp, 0ULL);
WDataOutP owp = qowp;
if (obits > VL_QUADSIZE) owp = va_arg(ap, WDataOutP);
if (obits == -1) { // string
owp = nullptr;
if (VL_UNCOVERABLE(fmt != 's')) {
VL_FATAL_MT(
__FILE__, __LINE__, "",
"Internal: format other than %s is passed to string"); // LCOV_EXCL_LINE
}
} else if (obits > VL_QUADSIZE) {
owp = va_arg(ap, WDataOutP);
}
for (int i = 0; i < VL_WORDS_I(obits); ++i) owp[i] = 0;
switch (fmt) {
case 'c': {
@ -1093,11 +1103,13 @@ IData _vl_vsscanf(FILE* fp, // If a fscanf
_vl_vsss_skipspace(fp, floc, fromp, fstr);
_vl_vsss_read_str(fp, floc, fromp, fstr, tmp, nullptr);
if (!tmp[0]) goto done;
int lpos = (static_cast<int>(strlen(tmp))) - 1;
int lsb = 0;
for (int i = 0; i < obits && lpos >= 0; --lpos) {
_vl_vsss_setbit(owp, obits, lsb, 8, tmp[lpos]);
lsb += 8;
if (owp) {
int lpos = (static_cast<int>(strlen(tmp))) - 1;
int lsb = 0;
for (int i = 0; i < obits && lpos >= 0; --lpos) {
_vl_vsss_setbit(owp, obits, lsb, 8, tmp[lpos]);
lsb += 8;
}
}
break;
}
@ -1194,6 +1206,9 @@ IData _vl_vsscanf(FILE* fp, // If a fscanf
if (!inIgnore) ++got;
// Reload data if non-wide (if wide, we put it in the right place directly)
if (obits == 0) { // Due to inIgnore
} else if (obits == -1) { // string
std::string* p = va_arg(ap, std::string*);
*p = tmp;
} else if (obits <= VL_BYTESIZE) {
CData* p = va_arg(ap, CData*);
*p = owp[0];
@ -1252,36 +1267,44 @@ void _VL_STRING_TO_VINT(int obits, void* destp, size_t srclen, const char* srcp)
for (; i < bytes; ++i) { *op++ = 0; }
}
IData VL_FGETS_IXI(int obits, void* destp, IData fpi) VL_MT_SAFE {
static IData getLine(std::string& str, IData fpi, size_t maxLen) VL_MT_SAFE {
str.clear();
// While threadsafe, each thread can only access different file handles
FILE* fp = VL_CVT_I_FP(fpi);
if (VL_UNLIKELY(!fp)) return 0;
// The string needs to be padded with 0's in unused spaces in front of
// any read data. This means we can't know in what location the first
// character will finally live, so we need to copy. Yuk.
IData bytes = VL_BYTES_I(obits);
char buffer[VL_TO_STRING_MAX_WORDS * VL_EDATASIZE + 1];
// We don't use fgets, as we must read \0s.
while (str.size() < maxLen) {
const int c = getc(fp); // getc() is threadsafe
if (c == EOF) break;
str.push_back(c);
if (c == '\n') break;
}
return str.size();
}
IData VL_FGETS_IXI(int obits, void* destp, IData fpi) VL_MT_SAFE {
std::string str;
const IData bytes = VL_BYTES_I(obits);
IData got = getLine(str, fpi, bytes);
if (VL_UNLIKELY(str.empty())) return 0;
// V3Emit has static check that bytes < VL_TO_STRING_MAX_WORDS, but be safe
if (VL_UNCOVERABLE(bytes > VL_TO_STRING_MAX_WORDS * VL_EDATASIZE)) {
if (VL_UNCOVERABLE(bytes < str.size())) {
VL_FATAL_MT(__FILE__, __LINE__, "", "Internal: fgets buffer overrun"); // LCOV_EXCL_LINE
}
// We don't use fgets, as we must read \0s.
IData got = 0;
char* cp = buffer;
while (got < bytes) {
int c = getc(fp); // getc() is threadsafe
if (c == EOF) break;
*cp++ = c;
got++;
if (c == '\n') break;
}
_VL_STRING_TO_VINT(obits, destp, got, buffer);
_VL_STRING_TO_VINT(obits, destp, got, str.data());
return got;
}
// declared in verilated_heavy.h
IData VL_FGETS_NI(std::string& str, IData fpi) VL_MT_SAFE {
return getLine(str, fpi, std::numeric_limits<size_t>::max());
}
IData VL_FERROR_IN(IData, std::string& outputr) VL_MT_SAFE {
// We ignore lhs/fpi - IEEE says "most recent error" so probably good enough
IData ret = errno;

2
include/verilated_heavy.h
View File

@ -546,6 +546,8 @@ inline IData VL_CMP_NN(const std::string& lhs, const std::string& rhs, bool igno
extern IData VL_ATOI_N(const std::string& str, int base) VL_PURE;
extern IData VL_FGETS_NI(std::string& destp, IData fpi);
//======================================================================
// Dumping

5
src/V3AstNodes.h
View File

@ -7959,7 +7959,10 @@ public:
V3ERROR_NA;
}
virtual string emitVerilog() override { return "%f$fgets(%l,%r)"; }
virtual string emitC() override { return "VL_FGETS_%nqX%rq(%lw, %P, &(%li), %ri)"; }
virtual string emitC() override {
return strgp()->dtypep()->basicp()->isString() ? "VL_FGETS_NI(%li, %ri)"
: "VL_FGETS_%nqX%rq(%lw, %P, &(%li), %ri)";
}
virtual bool cleanOut() const override { return false; }
virtual bool cleanLhs() const override { return true; }
virtual bool cleanRhs() const override { return true; }

7
src/V3EmitC.cpp
View File

@ -2199,7 +2199,12 @@ void EmitCStmts::displayArg(AstNode* dispp, AstNode** elistp, bool isScan, const
}
emitDispState.pushFormat(pfmt);
if (!ignore) {
emitDispState.pushArg(' ', nullptr, cvtToStr(argp->widthMin()));
if (argp->dtypep()->basicp()->keyword() == AstBasicDTypeKwd::STRING) {
// string in SystemVerilog is std::string in C++ which is not POD
emitDispState.pushArg(' ', nullptr, "-1");
} else {
emitDispState.pushArg(' ', nullptr, cvtToStr(argp->widthMin()));
}
emitDispState.pushArg(fmtLetter, argp, "");
} else {
emitDispState.pushArg(fmtLetter, nullptr, "");

17
test_regress/t/t_sys_file_basic.v
View File

@ -134,6 +134,12 @@ module t;
if (chars != 10) $stop;
if (letterw != "\0\0\0\0\0\0widestuff\n") $stop;
s = "";
chars = $fgets(s, file);
if (`verbose) $write("c=%0d w=%s", chars, s); // Output includes newline
if (chars != 7) $stop;
if (s != "string\n") $stop;
// $sscanf
if ($sscanf("x","")!=0) $stop;
if ($sscanf("z","z")!=0) $stop;
@ -173,6 +179,12 @@ module t;
`checkr(r, 0.1);
if (letterq != 64'hfffffffffffc65a5) $stop;
chars = $sscanf("scan from string",
"scan %s string", s);
if (`verbose) $write("c=%0d s=%s\n", chars, s);
if (chars != 1) $stop;
if (s != "from") $stop;
// Cover quad and %e/%f
chars = $sscanf("r=0.2",
"r=%e", r);
@ -245,6 +257,11 @@ module t;
if (chars != 1) $stop;
if (letterl != "\n") $stop;
chars = $fscanf(file, "%c%s not_included\n", letterl, s);
if (`verbose) $write("c=%0d l=%s\n", chars, s);
if (chars != 2) $stop;
if (s != "BCD") $stop;
// msg1229
v_a = $fgetc(file);
v_b = $fgetc(file);

2
test_regress/t/t_sys_file_basic_input.dat
View File

@ -1,10 +1,12 @@
hi
lquad
widestuff
string
*xa=1f xb=237904689_02348923
*ba=10 bb=11010010101001010101 note_the_two
*oa=23 ob=12563
*d=-236123
*u=-236124
*fredfishblah
aBCD not_included
12346789