[find-all-symbols] Fix racy yaml file writing.

[bkramer]

Summary

If multiple find-all-symbols processes access the temporary directory
simultaneously with two files with the same name they would collide and
create a broken yaml file. Fix this by using the safe createUniqueFile
API from LLVM instead.

[hokein]

Thanks! LGTM with one nit.

include-fixer/find-all-symbols/SymbolInfo.h

92 ↗

(On Diff #55559)

It would be better to change the function name to WriteSymbolInfosToStream.

clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h

 //===-- SymbolInfo.h - find all symbols--------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_FIND_ALL_SYMBOLS_SYMBOL_INFO_H
 #define LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_FIND_ALL_SYMBOLS_SYMBOL_INFO_H
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
 #include <set>
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace find_all_symbols {
 
 /// \brief Contains all information for a Symbol.
struct SymbolInfo {
   /// \brief The variable information.
   llvm::Optional<VariableInfo> VariableInfos;
 
   bool operator==(const SymbolInfo &Symbol) const;
 
   bool operator<(const SymbolInfo &Symbol) const;
 };
 
-/// \brief Write SymbolInfos to a single file (YAML format).
-bool WriteSymboInfosToFile(llvm::StringRef FilePath,
+/// \brief Write SymbolInfos to a stream (YAML format).
+bool WriteSymbolInfosToStream(llvm::raw_ostream &OS,
                               const std::set<SymbolInfo> &Symbols);
 
 /// \brief Read SymbolInfos from a YAML document.
 std::vector<SymbolInfo> ReadSymbolInfosFromYAML(llvm::StringRef Yaml);
 
 } // namespace find_all_symbols
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_FIND_ALL_SYMBOLS_SYMBOL_INFO_H

clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.cpp

return Name == Symbol.Name && FilePath == Symbol.FilePath &&
          LineNumber == Symbol.LineNumber && Contexts == Symbol.Contexts;
 }
 
 bool SymbolInfo::operator<(const SymbolInfo &Symbol) const {
   return std::tie(Name, FilePath, LineNumber) <
          std::tie(Symbol.Name, Symbol.FilePath, Symbol.LineNumber);
 }
 
-bool WriteSymboInfosToFile(llvm::StringRef FilePath,
+bool WriteSymbolInfosToStream(llvm::raw_ostream &OS,
                               const std::set<SymbolInfo> &Symbols) {
-  int FD = 0;
-  if (llvm::sys::fs::openFileForWrite(FilePath, FD, llvm::sys::fs::F_None))
-    return false;
-  llvm::raw_fd_ostream OS(FD, true);
   llvm::yaml::Output yout(OS);
   for (auto Symbol : Symbols)
     yout << Symbol;
-  OS.close();
   return true;
 }
 
 std::vector<SymbolInfo> ReadSymbolInfosFromYAML(llvm::StringRef Yaml) {
   std::vector<SymbolInfo> Symbols;
   llvm::yaml::Input yin(Yaml);
   yin >> Symbols;
   return Symbols;
 }
 
 } // namespace find_all_symbols
 } // namespace clang

clang-tools-extra/trunk/include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp

public:
   ~YamlReporter() {}
 
   void reportResult(StringRef FileName, const SymbolInfo &Symbol) override {
     Symbols[FileName].insert(Symbol);
   }
 
   void Write(const std::string &Dir) {
     for (const auto &Symbol : Symbols) {
-      SmallString<256> FilePath(Dir);
-      llvm::sys::path::append(
-          FilePath, llvm::sys::path::filename(Symbol.first) + ".yaml");
-      WriteSymboInfosToFile(FilePath, Symbol.second);
+      int FD;
+      SmallString<128> ResultPath;
+      llvm::sys::fs::createUniqueFile(
+          Dir + "/" + llvm::sys::path::filename(Symbol.first) + "-%%%%%%.yaml",
+          FD, ResultPath);
+      llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true);
+      WriteSymbolInfosToStream(OS, Symbol.second);
     }
   }
 
 private:
   std::map<std::string, std::set<SymbolInfo>> Symbols;
 };
 
 bool Merge(llvm::StringRef MergeDir, llvm::StringRef OutputFile) {
for (llvm::sys::fs::directory_iterator Dir(MergeDir, EC), DirEnd;
     if (!Buffer)
       continue;
     std::vector<SymbolInfo> Symbols =
         ReadSymbolInfosFromYAML(Buffer.get()->getBuffer());
     for (const auto &Symbol : Symbols)
       UniqueSymbols.insert(Symbol);
   }
 
-  WriteSymboInfosToFile(OutputFile, UniqueSymbols);
+  llvm::raw_fd_ostream OS(OutputFile, EC, llvm::sys::fs::F_None);
+  if (EC) {
+    llvm::errs() << "Cann't open '" << OutputFile << "': " << EC.message()
+                 << '\n';
+    return false;
+  }
+  WriteSymbolInfosToStream(OS, UniqueSymbols);
   return true;
 }
 
 } // namespace clang
 } // namespace find_all_symbols
 
 int main(int argc, const char **argv) {
   CommonOptionsParser OptionsParser(argc, argv, FindAllSymbolsCategory);