[Lldb-commits] [lldb] 42ae055 - [NFC] Fix potential for use-after-free in DumpModuleInfoAction

[Mariya Podchishchaeva via lldb-commits]

Author: Mariya Podchishchaeva
Date: 2023-03-30T06:44:23-04:00
New Revision: 42ae055b4c9282050636dd11198cad500424adf2

URL: https://github.com/llvm/llvm-project/commit/42ae055b4c9282050636dd11198cad500424adf2
DIFF: https://github.com/llvm/llvm-project/commit/42ae055b4c9282050636dd11198cad500424adf2.diff

LOG: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

Since each `DumpModuleInfoAction` can now contain a pointer to a
`raw_ostream`, saving there a poiter that owned by a local `unique_ptr`
may cause use-after-free. Clarify ownership and save a `shared_ptr`
inside of `DumpModuleInfoAction` instead.
Found by static analyzer.

Reviewed By: tahonermann, aaron.ballman

Differential Revision: https://reviews.llvm.org/D146412

Added: 
    

Modified: 
    clang/include/clang/Frontend/FrontendActions.h
    clang/lib/Frontend/FrontendActions.cpp
    lldb/source/Commands/CommandObjectTarget.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Frontend/FrontendActions.h b/clang/include/clang/Frontend/FrontendActions.h
index 9e6ed1ace190..3940e00eeb8d 100644
--- a/clang/include/clang/Frontend/FrontendActions.h
+++ b/clang/include/clang/Frontend/FrontendActions.h
@@ -177,9 +177,8 @@ class SyntaxOnlyAction : public ASTFrontendAction {
 /// Dump information about the given module file, to be used for
 /// basic debugging and discovery.
 class DumpModuleInfoAction : public ASTFrontendAction {
-public:
   // Allow other tools (ex lldb) to direct output for their use.
-  llvm::raw_ostream *OutputStream = nullptr;
+  std::shared_ptr<llvm::raw_ostream> OutputStream;
 
 protected:
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
@@ -188,6 +187,9 @@ class DumpModuleInfoAction : public ASTFrontendAction {
   void ExecuteAction() override;
 
 public:
+  DumpModuleInfoAction() = default;
+  explicit DumpModuleInfoAction(std::shared_ptr<llvm::raw_ostream> Out)
+      : OutputStream(Out) {}
   bool hasPCHSupport() const override { return false; }
   bool hasASTFileSupport() const override { return true; }
   bool hasIRSupport() const override { return false; }

diff  --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 05d9fc8208b2..0349e769595d 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -780,14 +780,12 @@ static StringRef ModuleKindName(Module::ModuleKind MK) {
 void DumpModuleInfoAction::ExecuteAction() {
   assert(isCurrentFileAST() && "dumping non-AST?");
   // Set up the output file.
-  std::unique_ptr<llvm::raw_fd_ostream> OutFile;
   CompilerInstance &CI = getCompilerInstance();
   StringRef OutputFileName = CI.getFrontendOpts().OutputFile;
   if (!OutputFileName.empty() && OutputFileName != "-") {
     std::error_code EC;
-    OutFile.reset(new llvm::raw_fd_ostream(OutputFileName.str(), EC,
-                                           llvm::sys::fs::OF_TextWithCRLF));
-    OutputStream = OutFile.get();
+    OutputStream.reset(new llvm::raw_fd_ostream(
+        OutputFileName.str(), EC, llvm::sys::fs::OF_TextWithCRLF));
   }
   llvm::raw_ostream &Out = OutputStream ? *OutputStream : llvm::outs();
 

diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index 5874453bca23..aecdf595ee5b 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -2179,8 +2179,11 @@ class CommandObjectTargetModulesDumpClangPCMInfo : public CommandObjectParsed {
     const char *clang_args[] = {"clang", pcm_path};
     compiler.setInvocation(clang::createInvocation(clang_args));
 
-    clang::DumpModuleInfoAction dump_module_info;
-    dump_module_info.OutputStream = &result.GetOutputStream().AsRawOstream();
+    // Pass empty deleter to not attempt to free memory that was allocated
+    // outside of the current scope, possibly statically.
+    std::shared_ptr<llvm::raw_ostream> Out(
+        &result.GetOutputStream().AsRawOstream(), [](llvm::raw_ostream *) {});
+    clang::DumpModuleInfoAction dump_module_info(Out);
     // DumpModuleInfoAction requires ObjectFilePCHContainerReader.
     compiler.getPCHContainerOperations()->registerReader(
         std::make_unique<clang::ObjectFilePCHContainerReader>());