From b15ef49c574c0418fb92a3ccb274634e8f7d5e75 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 21 Nov 2023 21:22:35 -0500 Subject: [PATCH] Fix compilers seeing empty input due to file system races (#4708). --- Changes | 1 + src/V3File.cpp | 4 ++-- src/V3Options.cpp | 17 ----------------- src/V3Options.h | 1 - src/V3Os.cpp | 28 ++++++++++++++++++++++++++++ src/V3Os.h | 1 + src/Verilator.cpp | 10 +++++++++- 7 files changed, 41 insertions(+), 21 deletions(-) diff --git a/Changes b/Changes index 083a2c4bc..fcf059e6e 100644 --- a/Changes +++ b/Changes @@ -34,6 +34,7 @@ Verilator 5.019 devel * Fix MingW compilation (#4675). [David Ledger] * Fix trace when using SystemC with certain configurations (#4676). [Anthony Donlon] * Fix C++20 compilation errors (#4670). +* Fix compilers seeing empty input due to file system races (#4708). [Flavien Solt] Verilator 5.018 2023-10-30 diff --git a/src/V3File.cpp b/src/V3File.cpp index a51372f8f..65c44a2e6 100644 --- a/src/V3File.cpp +++ b/src/V3File.cpp @@ -191,7 +191,7 @@ void V3FileDependImp::writeTimes(const string& filename, const string& cmdlineIn // Read stats of files we create after we're done making them // (except for this file, of course) DependFile* const dfp = const_cast(&(*iter)); - V3Options::fileNfsFlush(dfp->filename()); + V3Os::filesystemFlush(dfp->filename()); dfp->loadStats(); off_t showSize = iter->size(); ino_t showIno = iter->ino(); @@ -256,7 +256,7 @@ bool V3FileDependImp::checkTimes(const string& filename, const string& cmdlineIn *ifp >> quote; const string chkFilename = V3Os::getline(*ifp, '"'); - V3Options::fileNfsFlush(chkFilename); + V3Os::filesystemFlush(chkFilename); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init) struct stat chkStat; const int err = stat(chkFilename.c_str(), &chkStat); diff --git a/src/V3Options.cpp b/src/V3Options.cpp index 919aefd96..009509d79 100644 --- a/src/V3Options.cpp +++ b/src/V3Options.cpp @@ -456,23 +456,6 @@ bool V3Options::fileStatNormal(const string& filename) { return true; } -void V3Options::fileNfsFlush(const string& filename) { - // NFS caches stat() calls so to get up-to-date information must - // do a open or opendir on the filename. - // Faster to just try both rather than check if a file is a dir. -#ifdef _MSC_VER - if (int fd = ::open(filename.c_str(), O_RDONLY)) { // LCOV_EXCL_BR_LINE - if (fd > 0) ::close(fd); - } -#else - if (DIR* const dirp = opendir(filename.c_str())) { // LCOV_EXCL_BR_LINE - closedir(dirp); // LCOV_EXCL_LINE - } else if (int fd = ::open(filename.c_str(), O_RDONLY)) { // LCOV_EXCL_BR_LINE - if (fd > 0) ::close(fd); - } -#endif -} - string V3Options::fileExists(const string& filename) { // Surprisingly, for VCS and other simulators, this process // is quite slow; presumably because of re-reading each directory diff --git a/src/V3Options.h b/src/V3Options.h index 259462fe8..77b5e5e8d 100644 --- a/src/V3Options.h +++ b/src/V3Options.h @@ -697,7 +697,6 @@ public: void filePathLookedMsg(FileLine* fl, const string& modname); V3LangCode fileLanguage(const string& filename); static bool fileStatNormal(const string& filename); - static void fileNfsFlush(const string& filename); // METHODS (other OS) static void throwSigsegv(); diff --git a/src/V3Os.cpp b/src/V3Os.cpp index 8763725c1..f966e4fe8 100644 --- a/src/V3Os.cpp +++ b/src/V3Os.cpp @@ -45,6 +45,7 @@ VL_DEFINE_DEBUG_FUNCTIONS; #define PATH_MAX MAX_PATH #else #include +#include #endif #include #include @@ -58,6 +59,7 @@ VL_DEFINE_DEBUG_FUNCTIONS; # include // BCryptGenRandom # include # include // mkdir +# include // open, read, write, close # include // GetProcessMemoryInfo # include // These macros taken from gdbsupport/gdb_wait.h in binutils-gdb @@ -74,6 +76,7 @@ VL_DEFINE_DEBUG_FUNCTIONS; # include # include // Needed on FreeBSD for WIFEXITED # include // usleep +# include #endif // clang-format on @@ -294,6 +297,31 @@ void V3Os::createDir(const string& dirname) { #endif } +void V3Os::filesystemFlush(const string& dirname) { + // NFS caches stat() calls so to get up-to-date information must + // do a open or opendir on the filename. +#ifdef _MSC_VER + if (int fd = ::open(filename.c_str(), O_RDONLY)) { // LCOV_EXCL_BR_LINE + if (fd > 0) ::close(fd); + } +#else + { + // Linux kernel may not reread from NFS unless timestamp modified + struct timeval tv; + gettimeofday(&tv, NULL); + const int err = utimes(dirname.c_str(), &tv); + // Not an error + if (err != 0) UINFO(1, "-Info: File not statable: " << dirname << endl); + } + // Faster to just try both rather than check if a file is a dir. + if (DIR* const dirp = opendir(dirname.c_str())) { // LCOV_EXCL_BR_LINE + closedir(dirp); // LCOV_EXCL_LINE + } else if (int fd = ::open(dirname.c_str(), O_RDONLY)) { // LCOV_EXCL_BR_LINE + if (fd > 0) ::close(fd); + } +#endif +} + void V3Os::unlinkRegexp(const string& dir, const string& regexp) { #ifdef _MSC_VER try { diff --git a/src/V3Os.h b/src/V3Os.h index 3bf69ca17..445ab60b1 100644 --- a/src/V3Os.h +++ b/src/V3Os.h @@ -63,6 +63,7 @@ public: // METHODS (directory utilities) static void createDir(const string& dirname); + static void filesystemFlush(const string& dirname); static void unlinkRegexp(const string& dir, const string& regexp); // METHODS (random) diff --git a/src/Verilator.cpp b/src/Verilator.cpp index 32f1108cb..cc4fb7fdf 100644 --- a/src/Verilator.cpp +++ b/src/Verilator.cpp @@ -636,9 +636,12 @@ static void verilate(const string& argString) { // --FRONTEND------------------ // Cleanup - V3Os::unlinkRegexp(v3Global.opt.hierTopDataDir(), v3Global.opt.prefix() + "_*.tree"); + // Ideally we'd do prefix + "_*.*", and prefix + ".*", but this seems + // potentially disruptive to old behavior V3Os::unlinkRegexp(v3Global.opt.hierTopDataDir(), v3Global.opt.prefix() + "_*.dot"); + V3Os::unlinkRegexp(v3Global.opt.hierTopDataDir(), v3Global.opt.prefix() + "_*.tree"); V3Os::unlinkRegexp(v3Global.opt.hierTopDataDir(), v3Global.opt.prefix() + "_*.txt"); + V3Os::unlinkRegexp(v3Global.opt.hierTopDataDir(), v3Global.opt.prefix() + "_*_DepSet_*"); // Internal tests (after option parsing as need debug() setting, // and after removing files as may make debug output) @@ -717,6 +720,9 @@ static void verilate(const string& argString) { argString); } + V3Os::filesystemFlush(v3Global.opt.makeDir()); + if (v3Global.opt.hierTop()) V3Os::filesystemFlush(v3Global.opt.hierTopDataDir()); + // Final writing shouldn't throw warnings, but... V3Error::abortIfWarnings(); } @@ -747,6 +753,7 @@ static void execBuildJob() { UINFO(1, "Start Build\n"); const string cmdStr = buildMakeCmd(v3Global.opt.prefix() + ".mk", ""); + V3Os::filesystemFlush(v3Global.opt.hierTopDataDir()); const int exit_code = V3Os::system(cmdStr); if (exit_code != 0) { v3error(cmdStr << " exited with " << exit_code << std::endl); @@ -759,6 +766,7 @@ static void execHierVerilation() { const string makefile = v3Global.opt.prefix() + "_hier.mk "; const string target = v3Global.opt.build() ? " hier_build" : " hier_verilation"; const string cmdStr = buildMakeCmd(makefile, target); + V3Os::filesystemFlush(v3Global.opt.hierTopDataDir()); const int exit_code = V3Os::system(cmdStr); if (exit_code != 0) { v3error(cmdStr << " exited with " << exit_code << std::endl);