From 0855c45400f88afcaf3e4def7d5bb4abee502aa3 Mon Sep 17 00:00:00 2001 From: David Truby Date: Tue, 25 Feb 2020 15:59:50 +0000 Subject: [PATCH] [flang] Replace module writer posix file handling with llvm file handling. (flang-compiler/f18#993) NOTE: This commit introduces a dependency on LLVM HEAD Original-commit: flang-compiler/f18@97efc0194d411a17ec47edf5e27157d1a1afa3bc Reviewed-on: https://github.com/flang-compiler/f18/pull/993 --- flang/.drone.star | 20 +++- flang/include/flang/Semantics/semantics.h | 10 +- flang/lib/Semantics/CMakeLists.txt | 1 + flang/lib/Semantics/mod-file.cpp | 125 ++++++++++----------- .../test-lit/Semantics/Inputs/mod-file-changed.f90 | 5 + .../Semantics/Inputs/mod-file-unchanged.f90 | 5 + flang/test-lit/Semantics/mod-file-rewriter.f90 | 12 ++ flang/tools/f18/f18.cpp | 7 +- 8 files changed, 111 insertions(+), 74 deletions(-) create mode 100644 flang/test-lit/Semantics/Inputs/mod-file-changed.f90 create mode 100644 flang/test-lit/Semantics/Inputs/mod-file-unchanged.f90 create mode 100644 flang/test-lit/Semantics/mod-file-rewriter.f90 diff --git a/flang/.drone.star b/flang/.drone.star index 47dfca7..7d09fda 100644 --- a/flang/.drone.star +++ b/flang/.drone.star @@ -7,11 +7,17 @@ def clang(arch): "name": "test", "image": "ubuntu", "commands": [ - "apt-get update && apt-get install -y clang-8 cmake ninja-build lld-8 llvm-8-dev libc++-8-dev libc++abi-8-dev libz-dev", + "apt-get update && apt-get install -y clang-8 cmake ninja-build lld-8 llvm-8-dev libc++-8-dev libc++abi-8-dev libz-dev git", + "git clone https://github.com/llvm/llvm-project", + "mkdir llvm-project/build && cd llvm-project/build", + 'env CC=clang-8 CXX=clang++-8 CXXFLAGS="-stdlib=libc++" LDFLAGS="-fuse-ld=lld" cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=../install -DLLVM_TARGETS_TO_BUILD=host -DLLVM_ENABLE_PROJECTS="clang;mlir" ../llvm', + "ninja install", + "cd ../..", "mkdir build && cd build", - 'env CC=clang-8 CXX=clang++-8 CXXFLAGS="-UNDEBUG" LDFLAGS="-fuse-ld=lld" cmake -GNinja -DCMAKE_BUILD_TYPE=Release ..', + 'env CC=clang-8 CXX=clang++-8 CXXFLAGS="-UNDEBUG -stdlib=libc++" LDFLAGS="-fuse-ld=lld" cmake -GNinja -DCMAKE_BUILD_TYPE=Release .. -DLLVM_DIR=/drone/src/llvm-project/install/lib/cmake/llvm -DLLVM_EXTERNAL_LIT=/drone/src/llvm-project/build/bin/llvm-lit', "ninja -j8", "ctest --output-on-failure -j24", + "ninja check-all", ], }, ], @@ -27,11 +33,17 @@ def gcc(arch): "name": "test", "image": "gcc", "commands": [ - "apt-get update && apt-get install -y cmake ninja-build llvm-dev libz-dev", + "apt-get update && apt-get install -y cmake ninja-build llvm-dev libz-dev git", + "git clone https://github.com/llvm/llvm-project", + "mkdir llvm-project/build && cd llvm-project/build", + 'env CC=gcc CXX=g++ LDFLAGS="-fuse-ld=gold" cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=../install -DLLVM_TARGETS_TO_BUILD=host -DLLVM_ENABLE_PROJECTS="clang;mlir" ../llvm', + "ninja install", + "cd ../..", "mkdir build && cd build", - 'env CC=gcc CXX=g++ CXXFLAGS="-UNDEBUG" LDFLAGS="-fuse-ld=gold" cmake -GNinja -DCMAKE_BUILD_TYPE=Release ..', + 'env CC=gcc CXX=g++ CXXFLAGS="-UNDEBUG" LDFLAGS="-fuse-ld=gold" cmake -GNinja -DCMAKE_BUILD_TYPE=Release .. -DLLVM_DIR=/drone/src/llvm-project/install/lib/cmake/llvm -DLLVM_EXTERNAL_LIT=/drone/src/llvm-project/build/bin/llvm-lit', "ninja -j8", "ctest --output-on-failure -j24", + "ninja check-all", ], }, ], diff --git a/flang/include/flang/Semantics/semantics.h b/flang/include/flang/Semantics/semantics.h index dd38458..b88bcc4 100644 --- a/flang/include/flang/Semantics/semantics.h +++ b/flang/include/flang/Semantics/semantics.h @@ -80,6 +80,7 @@ public: const std::string &moduleFileSuffix() const { return moduleFileSuffix_; } bool warnOnNonstandardUsage() const { return warnOnNonstandardUsage_; } bool warningsAreErrors() const { return warningsAreErrors_; } + bool debugModuleWriter() const { return debugModuleWriter_; } const evaluate::IntrinsicProcTable &intrinsics() const { return intrinsics_; } Scope &globalScope() { return globalScope_; } parser::Messages &messages() { return messages_; } @@ -112,6 +113,11 @@ public: return *this; } + SemanticsContext &set_debugModuleWriter(bool x) { + debugModuleWriter_ = x; + return *this; + } + const DeclTypeSpec &MakeNumericType(TypeCategory, int kind = 0); const DeclTypeSpec &MakeLogicalType(int kind = 0); @@ -175,6 +181,7 @@ private: std::string moduleFileSuffix_{".mod"}; bool warnOnNonstandardUsage_{false}; bool warningsAreErrors_{false}; + bool debugModuleWriter_{false}; const evaluate::IntrinsicProcTable intrinsics_; Scope globalScope_; parser::Messages messages_; @@ -190,8 +197,9 @@ private: class Semantics { public: explicit Semantics(SemanticsContext &context, parser::Program &program, - parser::CookedSource &cooked) + parser::CookedSource &cooked, bool debugModuleWriter = false) : context_{context}, program_{program}, cooked_{cooked} { + context.set_debugModuleWriter(debugModuleWriter); context.globalScope().AddSourceRange(parser::CharBlock{cooked.data()}); } diff --git a/flang/lib/Semantics/CMakeLists.txt b/flang/lib/Semantics/CMakeLists.txt index 7900eef..0c6b612 100644 --- a/flang/lib/Semantics/CMakeLists.txt +++ b/flang/lib/Semantics/CMakeLists.txt @@ -45,6 +45,7 @@ add_library(FortranSemantics target_link_libraries(FortranSemantics FortranCommon FortranEvaluate + LLVMSupport ) install (TARGETS FortranSemantics diff --git a/flang/lib/Semantics/mod-file.cpp b/flang/lib/Semantics/mod-file.cpp index 53f72b4..2810c35 100644 --- a/flang/lib/Semantics/mod-file.cpp +++ b/flang/lib/Semantics/mod-file.cpp @@ -15,16 +15,14 @@ #include "flang/Semantics/semantics.h" #include "flang/Semantics/symbol.h" #include "flang/Semantics/tools.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/raw_ostream.h" #include -#include #include #include #include #include -#include -#include -#include -#include #include namespace Fortran::semantics { @@ -62,10 +60,10 @@ static std::ostream &PutAttrs(std::ostream &, Attrs, static std::ostream &PutAttr(std::ostream &, Attr); static std::ostream &PutType(std::ostream &, const DeclTypeSpec &); static std::ostream &PutLower(std::ostream &, const std::string &); -static int WriteFile(const std::string &, const std::string &); +static std::error_code WriteFile( + const std::string &, const std::string &, bool = true); static bool FileContentsMatch( const std::string &, const std::string &, const std::string &); -static std::size_t GetFileSize(const std::string &); static std::string CheckSum(const std::string_view &); // Collect symbols needed for a subprogram interface @@ -135,9 +133,10 @@ void ModFileWriter::Write(const Symbol &symbol) { auto path{context_.moduleDirectory() + '/' + ModFileName(symbol.name(), ancestorName, context_.moduleFileSuffix())}; PutSymbols(DEREF(symbol.scope())); - if (int error{WriteFile(path, GetAsString(symbol))}) { - context_.Say(symbol.name(), "Error writing %s: %s"_err_en_US, path, - std::strerror(error)); + if (std::error_code error{ + WriteFile(path, GetAsString(symbol), context_.debugModuleWriter())}) { + context_.Say( + symbol.name(), "Error writing %s: %s"_err_en_US, path, error.message()); } } @@ -618,53 +617,67 @@ std::ostream &PutLower(std::ostream &os, const std::string &str) { } struct Temp { - Temp() = delete; + Temp(llvm::sys::fs::file_t fd, std::string path) : fd{fd}, path{path} {} + Temp(Temp &&t) : fd{std::exchange(t.fd, -1)}, path{std::move(t.path)} {} ~Temp() { - close(fd); - unlink(path.c_str()); + if (fd >= 0) { + llvm::sys::fs::closeFile(fd); + llvm::sys::fs::remove(path.c_str()); + } } - int fd; + llvm::sys::fs::file_t fd; std::string path; }; // Create a temp file in the same directory and with the same suffix as path. // Return an open file descriptor and its path. -static Temp MkTemp(const std::string &path) { +static llvm::ErrorOr MkTemp(const std::string &path) { auto length{path.length()}; auto dot{path.find_last_of("./")}; - std::string suffix{dot < length && path[dot] == '.' ? path.substr(dot) : ""}; + std::string suffix{ + dot < length && path[dot] == '.' ? path.substr(dot + 1) : ""}; CHECK(length > suffix.length() && path.substr(length - suffix.length()) == suffix); - auto tempPath{path.substr(0, length - suffix.length()) + "XXXXXX" + suffix}; - int fd{mkstemps(&tempPath[0], suffix.length())}; - auto mask{umask(0777)}; - umask(mask); - chmod(tempPath.c_str(), 0666 & ~mask); // temp is created with mode 0600 - return Temp{fd, tempPath}; + auto prefix{path.substr(0, length - suffix.length())}; + llvm::sys::fs::file_t fd; + llvm::SmallString<16> tempPath; + if (std::error_code err{llvm::sys::fs::createUniqueFile( + prefix + "%%%%%%" + suffix, fd, tempPath)}) { + return err; + } + return Temp{fd, tempPath.c_str()}; } // Write the module file at path, prepending header. If an error occurs, // return errno, otherwise 0. -static int WriteFile(const std::string &path, const std::string &contents) { +static std::error_code WriteFile( + const std::string &path, const std::string &contents, bool debug) { auto header{std::string{ModHeader::bom} + ModHeader::magic + CheckSum(contents) + ModHeader::terminator}; + if (debug) { + llvm::dbgs() << "Processing module " << path << ": "; + } if (FileContentsMatch(path, header, contents)) { - return 0; + if (debug) { + llvm::dbgs() << "module unchanged, not writing\n"; + } + return {}; } - Temp temp{MkTemp(path)}; - if (temp.fd < 0) { - return errno; + llvm::ErrorOr temp{MkTemp(path)}; + if (!temp) { + return temp.getError(); } - if (write(temp.fd, header.c_str(), header.size()) != - static_cast(header.size()) || - write(temp.fd, contents.c_str(), contents.size()) != - static_cast(contents.size())) { - return errno; + llvm::raw_fd_ostream writer(temp->fd, /*shouldClose=*/false); + writer << header; + writer << contents; + writer.flush(); + if (writer.has_error()) { + return writer.error(); } - if (std::rename(temp.path.c_str(), path.c_str()) == -1) { - return errno; + if (debug) { + llvm::dbgs() << "module written\n"; } - return 0; + return llvm::sys::fs::rename(temp->path, path); } // Return true if the stream matches what we would write for the mod file. @@ -672,34 +685,21 @@ static bool FileContentsMatch(const std::string &path, const std::string &header, const std::string &contents) { std::size_t hsize{header.size()}; std::size_t csize{contents.size()}; - if (GetFileSize(path) != hsize + csize) { + auto buf_or{llvm::MemoryBuffer::getFile(path)}; + if (!buf_or) { return false; } - int fd{open(path.c_str(), O_RDONLY)}; - if (fd < 0) { + auto buf = std::move(buf_or.get()); + if (buf->getBufferSize() != hsize + csize) { return false; } - constexpr std::size_t bufSize{4096}; - std::string buffer(bufSize, '\0'); - if (read(fd, &buffer[0], hsize) != static_cast(hsize) || - std::memcmp(&buffer[0], &header[0], hsize) != 0) { - close(fd); - return false; // header doesn't match - } - for (auto remaining{csize};;) { - auto bytes{std::min(bufSize, remaining)}; - auto got{read(fd, &buffer[0], bytes)}; - if (got != static_cast(bytes) || - std::memcmp(&buffer[0], &contents[csize - remaining], bytes) != 0) { - close(fd); - return false; - } - if (bytes == 0 && remaining == 0) { - close(fd); - return true; - } - remaining -= bytes; + if (!std::equal(header.begin(), header.end(), buf->getBufferStart(), + buf->getBufferStart() + hsize)) { + return false; } + + return std::equal(contents.begin(), contents.end(), + buf->getBufferStart() + hsize, buf->getBufferEnd()); } // Compute a simple hash of the contents of a module file and @@ -729,15 +729,6 @@ static bool VerifyHeader(const char *content, std::size_t len) { return expectSum == actualSum; } -static std::size_t GetFileSize(const std::string &path) { - struct stat statbuf; - if (stat(path.c_str(), &statbuf) == 0) { - return static_cast(statbuf.st_size); - } else { - return 0; - } -} - Scope *ModFileReader::Read(const SourceName &name, Scope *ancestor) { std::string ancestorName; // empty for module if (ancestor) { diff --git a/flang/test-lit/Semantics/Inputs/mod-file-changed.f90 b/flang/test-lit/Semantics/Inputs/mod-file-changed.f90 new file mode 100644 index 0000000..0282038 --- /dev/null +++ b/flang/test-lit/Semantics/Inputs/mod-file-changed.f90 @@ -0,0 +1,5 @@ +module m + dimension :: x(10) + private :: x + real :: x +end module m diff --git a/flang/test-lit/Semantics/Inputs/mod-file-unchanged.f90 b/flang/test-lit/Semantics/Inputs/mod-file-unchanged.f90 new file mode 100644 index 0000000..46c208a --- /dev/null +++ b/flang/test-lit/Semantics/Inputs/mod-file-unchanged.f90 @@ -0,0 +1,5 @@ +module m + dimension :: x(10) + public :: x + real :: x +end module m diff --git a/flang/test-lit/Semantics/mod-file-rewriter.f90 b/flang/test-lit/Semantics/mod-file-rewriter.f90 new file mode 100644 index 0000000..8125291 --- /dev/null +++ b/flang/test-lit/Semantics/mod-file-rewriter.f90 @@ -0,0 +1,12 @@ +! RUN: rm -fr %t && mkdir %t && cd %t +! RUN: %f18 -fparse-only -fdebug-module-writer %s 2>&1 | FileCheck %s --check-prefix CHECK_CHANGED +! RUN: %f18 -fparse-only -fdebug-module-writer %s 2>&1 | FileCheck %s --check-prefix CHECK_UNCHANGED +! RUN: %f18 -fparse-only -fdebug-module-writer %p/Inputs/mod-file-unchanged.f90 2>&1 | FileCheck %s --check-prefix CHECK_UNCHANGED +! RUN: %f18 -fparse-only -fdebug-module-writer %p/Inputs/mod-file-changed.f90 2>&1 | FileCheck %s --check-prefix CHECK_CHANGED + +module m + real :: x(10) +end module m + +! CHECK_CHANGED: Processing module {{.*}}.mod: module written +! CHECK_UNCHANGED: Processing module {{.*}}.mod: module unchanged, not writing diff --git a/flang/tools/f18/f18.cpp b/flang/tools/f18/f18.cpp index 608075a..fe7bd8a 100644 --- a/flang/tools/f18/f18.cpp +++ b/flang/tools/f18/f18.cpp @@ -98,6 +98,7 @@ struct DriverOptions { bool dumpSymbols{false}; bool debugResolveNames{false}; bool debugNoSemantics{false}; + bool debugModuleWriter{false}; bool measureTree{false}; bool unparseTypedExprsToPGF90{false}; std::vector pgf90Args; @@ -251,8 +252,8 @@ std::string CompileFortran(std::string path, Fortran::parser::Options options, if (!driver.debugNoSemantics || driver.debugResolveNames || driver.dumpSymbols || driver.dumpUnparseWithSymbols || driver.getDefinition || driver.getSymbolsSources) { - Fortran::semantics::Semantics semantics{ - semanticsContext, parseTree, parsing.cooked()}; + Fortran::semantics::Semantics semantics{semanticsContext, parseTree, + parsing.cooked(), driver.debugModuleWriter}; semantics.Perform(); semantics.EmitMessages(std::cerr); if (driver.dumpSymbols) { @@ -493,6 +494,8 @@ int main(int argc, char *const argv[]) { driver.dumpSymbols = true; } else if (arg == "-fdebug-resolve-names") { driver.debugResolveNames = true; + } else if (arg == "-fdebug-module-writer") { + driver.debugModuleWriter = true; } else if (arg == "-fdebug-measure-parse-tree") { driver.measureTree = true; } else if (arg == "-fdebug-instrumented-parse") { -- 2.7.4