[flang] Replace module writer posix file handling with llvm file handling. (flang...
authorDavid Truby <david.truby@arm.com>
Tue, 25 Feb 2020 15:59:50 +0000 (15:59 +0000)
committerGitHub <noreply@github.com>
Tue, 25 Feb 2020 15:59:50 +0000 (07:59 -0800)
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
flang/include/flang/Semantics/semantics.h
flang/lib/Semantics/CMakeLists.txt
flang/lib/Semantics/mod-file.cpp
flang/test-lit/Semantics/Inputs/mod-file-changed.f90 [new file with mode: 0644]
flang/test-lit/Semantics/Inputs/mod-file-unchanged.f90 [new file with mode: 0644]
flang/test-lit/Semantics/mod-file-rewriter.f90 [new file with mode: 0644]
flang/tools/f18/f18.cpp

index 47dfca7..7d09fda 100644 (file)
@@ -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",
                     ],
                 },
             ],
index dd38458..b88bcc4 100644 (file)
@@ -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()});
   }
 
index 7900eef..0c6b612 100644 (file)
@@ -45,6 +45,7 @@ add_library(FortranSemantics
 target_link_libraries(FortranSemantics
   FortranCommon
   FortranEvaluate
+  LLVMSupport
 )
 
 install (TARGETS FortranSemantics
index 53f72b4..2810c35 100644 (file)
 #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 <algorithm>
-#include <cerrno>
 #include <fstream>
 #include <ostream>
 #include <set>
 #include <string_view>
-#include <sys/file.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <unistd.h>
 #include <vector>
 
 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<Temp> 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> temp{MkTemp(path)};
+  if (!temp) {
+    return temp.getError();
   }
-  if (write(temp.fd, header.c_str(), header.size()) !=
-          static_cast<ssize_t>(header.size()) ||
-      write(temp.fd, contents.c_str(), contents.size()) !=
-          static_cast<ssize_t>(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<ssize_t>(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<ssize_t>(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<std::size_t>(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 (file)
index 0000000..0282038
--- /dev/null
@@ -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 (file)
index 0000000..46c208a
--- /dev/null
@@ -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 (file)
index 0000000..8125291
--- /dev/null
@@ -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
index 608075a..fe7bd8a 100644 (file)
@@ -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<std::string> 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") {