Merge pull request #6129 from YosysHQ/nella/flock-improvements

liberty_cache: fix WASI build, add Windows lock, harden scl cache [sc-793]

kernel/io.cc

@@ -11,6 +11,12 @@
 #include <io.h>
 #endif
 
+#if !defined(_WIN32) && !defined(__wasi__)
+#include <sys/file.h>
+#include <fcntl.h>
+#include <cerrno>
+#endif
+
 YOSYS_NAMESPACE_BEGIN
 
 // Set of utilities for handling files
@@ -615,4 +621,68 @@ void format_emit_void_ptr(std::string &result, std::string_view spec, int *dynam
 	format_emit_stringf(result, spec, dynamic_ints, num_dynamic_ints, arg);
 }
 
+#if defined(_WIN32)
+ScopedFileLock::ScopedFileLock(const std::string &path)
+{
+	HANDLE h = CreateFileA(path.c_str(), GENERIC_READ | GENERIC_WRITE,
+			FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+			NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+	if (h == INVALID_HANDLE_VALUE) {
+		log_warning("Cannot open lock file %s, proceeding without lock\n", path.c_str());
+		return;
+	}
+	OVERLAPPED ov = {};
+	if (!LockFileEx(h, LOCKFILE_EXCLUSIVE_LOCK, 0, MAXDWORD, MAXDWORD, &ov)) {
+		log_warning("Cannot lock %s, proceeding without lock\n", path.c_str());
+		CloseHandle(h);
+		return;
+	}
+	handle = h;
+}
+
+ScopedFileLock::~ScopedFileLock()
+{
+	if (handle != nullptr) {
+		OVERLAPPED ov = {};
+		UnlockFileEx(handle, 0, MAXDWORD, MAXDWORD, &ov);
+		CloseHandle(handle);
+	}
+}
+#elif !defined(__wasi__)
+ScopedFileLock::ScopedFileLock(const std::string &path)
+{
+	fd = open(path.c_str(), O_CREAT | O_RDWR | O_CLOEXEC, 0666);
+	if (fd < 0)
+		fd = open(path.c_str(), O_RDONLY | O_CLOEXEC);
+	if (fd < 0) {
+		log_warning("Cannot open lock file %s, proceeding without lock\n", path.c_str());
+		return;
+	}
+	int ret;
+	while ((ret = flock(fd, LOCK_EX)) != 0 && errno == EINTR);
+	if (ret != 0)
+		log_warning("Cannot lock %s, proceeding without lock\n", path.c_str());
+}
+
+ScopedFileLock::~ScopedFileLock()
+{
+	if (fd >= 0)
+		close(fd); // releases the flock
+}
+#else
+ScopedFileLock::ScopedFileLock(const std::string &) {}
+ScopedFileLock::~ScopedFileLock() {}
+#endif
+
+unsigned get_process_id()
+{
+#if defined(_WIN32)
+	return GetCurrentProcessId();
+#elif defined(__wasi__)
+	return 0;
+#else
+	return getpid();
+#endif
+}
+
 YOSYS_NAMESPACE_END

kernel/io.h

@@ -508,6 +508,24 @@ void append_globbed(std::vector<std::string>& paths, std::string pattern);
 std::string name_from_file_path(std::string path);
 std::string parent_from_file_path(std::string path);
 
+// Exclusive inter-process file lock (flock/LockFileEx, no-op on WASI),
+// released on destruction. Never unlinks the lock file: that would reopen the race.
+struct ScopedFileLock {
+	ScopedFileLock(const std::string &path);
+	~ScopedFileLock();
+	ScopedFileLock(const ScopedFileLock &) = delete;
+	ScopedFileLock &operator=(const ScopedFileLock &) = delete;
+private:
+#if defined(_WIN32)
+	void *handle = nullptr;
+#elif !defined(__wasi__)
+	int fd = -1;
+#endif
+};
+
+// Current process id (0 on WASI).
+unsigned get_process_id();
+
 YOSYS_NAMESPACE_END
 
 #endif // YOSYS_IO_H

passes/techmap/liberty_cache.h

@@ -3,6 +3,9 @@
 
 #include "kernel/yosys.h"
 
+#include <filesystem>
+#include <system_error>
+
 #ifdef YOSYS_LINK_ABC
 namespace abc {
 	int Abc_RealMain(int argc, char *argv[]);
@@ -81,11 +84,18 @@ inline std::string convert_liberty_files_to_merged_scl(const std::vector<std::st
 	}
 
 	if (need_convert) {
+		ScopedFileLock scl_lock(merged_scl + ".lock");
+		// Recheck under the lock: another process may have built the cache while we waited.
+		if (stat(merged_scl.c_str(), &scl_stat) == 0 && scl_stat.st_mtime >= newest_mtime) {
+			log("ABC: using cached merged SCL: %s (%zu files)\n", merged_scl.c_str(), liberty_files.size());
+			return merged_scl;
+		}
 		// read_lib -X cell1 -X cell2 file1 ; read_lib -X cell1 -X cell2 -m file2 ; ... ; write_scl merged.scl
-		std::string temp_scl = merged_scl + ".tmp";
+		// Concurrent writers cannot corrupt each other even when running without the lock.
+		std::string temp_scl = stringf("%s.%u.tmp", merged_scl.c_str(), get_process_id());
 
 #ifdef YOSYS_LINK_ABC
-		std::string script_path = stringf("%s/yosys_merged_scl_convert_%08x.script", cache_dir.c_str(), hash);
+		std::string script_path = stringf("%s/yosys_merged_scl_convert_%08x_%u.script", cache_dir.c_str(), hash, get_process_id());
 		FILE *f = fopen(script_path.c_str(), "w");
 
 		if (f == NULL) {
@@ -144,7 +154,9 @@ inline std::string convert_liberty_files_to_merged_scl(const std::vector<std::st
 			return "";
 		}
 #endif
-		if (rename(temp_scl.c_str(), merged_scl.c_str()) != 0) {
+		std::error_code rename_ec;
+		std::filesystem::rename(temp_scl, merged_scl, rename_ec);
+		if (rename_ec) {
 			log_warning("ABC: failed to rename %s to %s, falling back to liberty format\n", temp_scl.c_str(), merged_scl.c_str());
 			remove(temp_scl.c_str());
 			return "";