Fix use-after-free bug in Tooling.

[ymandel]

Summary

buildASTFromCodeWithArgs() was creating a memory buffer referencing a
stack-allocated string. This diff changes the implementation to copy the code
string into the memory buffer so that said buffer owns the memory.

[ymandel]

Change buildAST functions to take a StringRef instead of a Twine.

In practice, code is almost never constructed on the fly using a Twine, so StringRef is simpler and avoids needing a copy when constructing the MemBuffer.

[ymandel]

Change header correspondingly.

[ymandel]

Update test that was calling builAST with an actual Twine.

[alexfh]

include/clang/Tooling/Tooling.h

208 ↗

(On Diff #178507)

While we're here, can we change FileName and ToolName to StringRef too?

[ymandel]

Convert other Twine-typed args to StringRef.

[ymandel]

Adjust comments.

[alexfh]

LG

[ymandel]

Alex, would you mind submitting this since I don't yet have submit privileges? I'll request them shortly, but I'd rather this go one in the meantime.

cfe/trunk/include/clang/Tooling/Tooling.h

 ///
 /// \param Code C++ code.
 /// \param FileName The file name which 'Code' will be mapped as.
 /// \param PCHContainerOps The PCHContainerOperations for loading and creating
 /// clang modules.
 ///
 /// \return The resulting AST or null if an error occurred.
 std::unique_ptr<ASTUnit>
-buildASTFromCode(const Twine &Code, const Twine &FileName = "input.cc",
+buildASTFromCode(StringRef Code, StringRef FileName = "input.cc",
                  std::shared_ptr<PCHContainerOperations> PCHContainerOps =
                      std::make_shared<PCHContainerOperations>());
 
 /// Builds an AST for 'Code' with additional flags.
 ///
 /// \param Code C++ code.
 /// \param Args Additional flags to pass on.
 /// \param FileName The file name which 'Code' will be mapped as.
 /// \param ToolName The name of the binary running the tool. Standard library
 ///                 header paths will be resolved relative to this.
 /// \param PCHContainerOps The PCHContainerOperations for loading and creating
 /// clang modules.
 ///
 /// \param Adjuster A function to filter the command line arguments as specified.
 ///
 /// \return The resulting AST or null if an error occurred.
 std::unique_ptr<ASTUnit> buildASTFromCodeWithArgs(
-    const Twine &Code, const std::vector<std::string> &Args,
-    const Twine &FileName = "input.cc", const Twine &ToolName = "clang-tool",
+    StringRef Code, const std::vector<std::string> &Args,
+    StringRef FileName = "input.cc", StringRef ToolName = "clang-tool",
     std::shared_ptr<PCHContainerOperations> PCHContainerOps =
         std::make_shared<PCHContainerOperations>(),
     ArgumentsAdjuster Adjuster = getClangStripDependencyFileAdjuster());
 
 /// Utility to run a FrontendAction in a single clang invocation.
 class ToolInvocation {
 public:
   /// Create a tool invocation.
   ///
   /// \param CommandLine The command line arguments to clang. Note that clang

cfe/trunk/lib/Tooling/Tooling.cpp

 void ClangTool::setRestoreWorkingDir(bool RestoreCWD) {
   this->RestoreCWD = RestoreCWD;
 }
 
 namespace clang {
 namespace tooling {
 
 std::unique_ptr<ASTUnit>
-buildASTFromCode(const Twine &Code, const Twine &FileName,
+buildASTFromCode(StringRef Code, StringRef FileName,
                  std::shared_ptr<PCHContainerOperations> PCHContainerOps) {
   return buildASTFromCodeWithArgs(Code, std::vector<std::string>(), FileName,
                                   "clang-tool", std::move(PCHContainerOps));
 }
 
 std::unique_ptr<ASTUnit> buildASTFromCodeWithArgs(
-    const Twine &Code, const std::vector<std::string> &Args,
-    const Twine &FileName, const Twine &ToolName,
-    std::shared_ptr<PCHContainerOperations> PCHContainerOps,
+    StringRef Code, const std::vector<std::string> &Args, StringRef FileName,
+    StringRef ToolName, std::shared_ptr<PCHContainerOperations> PCHContainerOps,
     ArgumentsAdjuster Adjuster) {
-  SmallString<16> FileNameStorage;
-  StringRef FileNameRef = FileName.toNullTerminatedStringRef(FileNameStorage);
-
   std::vector<std::unique_ptr<ASTUnit>> ASTs;
   ASTBuilderAction Action(ASTs);
   llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFileSystem(
       new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem()));
   llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
       new llvm::vfs::InMemoryFileSystem);
   OverlayFileSystem->pushOverlay(InMemoryFileSystem);
   llvm::IntrusiveRefCntPtr<FileManager> Files(
       new FileManager(FileSystemOptions(), OverlayFileSystem));
 
   ToolInvocation Invocation(
-      getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), FileNameRef),
+      getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileName), FileName),
       &Action, Files.get(), std::move(PCHContainerOps));
 
-  SmallString<1024> CodeStorage;
-  InMemoryFileSystem->addFile(FileNameRef, 0,
-                              llvm::MemoryBuffer::getMemBuffer(
-                                  Code.toNullTerminatedStringRef(CodeStorage)));
+  InMemoryFileSystem->addFile(FileName, 0,
+                              llvm::MemoryBuffer::getMemBufferCopy(Code));
   if (!Invocation.run())
     return nullptr;
 
   assert(ASTs.size() == 1);
   return std::move(ASTs[0]);
 }
 
 } // namespace tooling
 } // namespace clang

cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp

 //===---------- ExprMutationAnalyzerTest.cpp ------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/SmallString.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <cctype>
 
 namespace clang {
 
 using namespace clang::ast_matchers;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
 using ::testing::ResultOf;
 using ::testing::StartsWith;
 using ::testing::Values;
 
 namespace {
 
 using ExprMatcher = internal::Matcher<Expr>;
 using StmtMatcher = internal::Matcher<Stmt>;
 
 std::unique_ptr<ASTUnit>
 buildASTFromCodeWithArgs(const Twine &Code,
                          const std::vector<std::string> &Args) {
-  auto AST = tooling::buildASTFromCodeWithArgs(Code, Args);
+  SmallString<1024> CodeStorage;
+  auto AST =
+      tooling::buildASTFromCodeWithArgs(Code.toStringRef(CodeStorage), Args);
   EXPECT_FALSE(AST->getDiagnostics().hasErrorOccurred());
   return AST;
 }
 
 std::unique_ptr<ASTUnit> buildASTFromCode(const Twine &Code) {
   return buildASTFromCodeWithArgs(Code, {});
 }