date:20230329

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential use-after-free in DumpModuleInfoAction::ExecuteAction()

2023-03-29 Thread Mariya Podchishchaeva via Phabricator via lldb-commits

Fznamznon updated this revision to Diff 509285.
Fznamznon added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Use private shared_ptr in `DumpModuleInfoAction`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146412/new/

https://reviews.llvm.org/D146412

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


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -2179,8 +2179,9 @@
 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();
+std::shared_ptr Out(
+&result.GetOutputStream().AsRawOstream(), [](llvm::raw_ostream *) {});
+clang::DumpModuleInfoAction dump_module_info(Out);
 // DumpModuleInfoAction requires ObjectFilePCHContainerReader.
 compiler.getPCHContainerOperations()->registerReader(
 std::make_unique());
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -780,14 +780,12 @@
 void DumpModuleInfoAction::ExecuteAction() {
   assert(isCurrentFileAST() && "dumping non-AST?");
   // Set up the output file.
-  std::unique_ptr 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();
 
Index: clang/include/clang/Frontend/FrontendActions.h
===
--- clang/include/clang/Frontend/FrontendActions.h
+++ clang/include/clang/Frontend/FrontendActions.h
@@ -177,9 +177,8 @@
 /// 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 OutputStream;
 
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
@@ -188,6 +187,9 @@
   void ExecuteAction() override;
 
 public:
+  DumpModuleInfoAction() = default;
+  DumpModuleInfoAction(std::shared_ptr Out)
+  : OutputStream(Out) {}
   bool hasPCHSupport() const override { return false; }
   bool hasASTFileSupport() const override { return true; }
   bool hasIRSupport() const override { return false; }


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -2179,8 +2179,9 @@
 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();
+std::shared_ptr Out(
+&result.GetOutputStream().AsRawOstream(), [](llvm::raw_ostream *) {});
+clang::DumpModuleInfoAction dump_module_info(Out);
 // DumpModuleInfoAction requires ObjectFilePCHContainerReader.
 compiler.getPCHContainerOperations()->registerReader(
 std::make_unique());
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -780,14 +780,12 @@
 void DumpModuleInfoAction::ExecuteAction() {
   assert(isCurrentFileAST() && "dumping non-AST?");
   // Set up the output file.
-  std::unique_ptr 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();

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

2023-03-29 Thread Aaron Ballman via Phabricator via lldb-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

This looks much better to me, thank you! LGTM with one small nit for safety.




Comment at: clang/include/clang/Frontend/FrontendActions.h:191
+  DumpModuleInfoAction() = default;
+  DumpModuleInfoAction(std::shared_ptr Out)
+  : OutputStream(Out) {}

A bit of extra safety around implicit conversions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146412/new/

https://reviews.llvm.org/D146412

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

2023-03-29 Thread Mariya Podchishchaeva via Phabricator via lldb-commits

Fznamznon updated this revision to Diff 509372.
Fznamznon added a comment.

Add a little bit of safety.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146412/new/

https://reviews.llvm.org/D146412

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


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -2179,8 +2179,9 @@
 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();
+std::shared_ptr Out(
+&result.GetOutputStream().AsRawOstream(), [](llvm::raw_ostream *) {});
+clang::DumpModuleInfoAction dump_module_info(Out);
 // DumpModuleInfoAction requires ObjectFilePCHContainerReader.
 compiler.getPCHContainerOperations()->registerReader(
 std::make_unique());
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -780,14 +780,12 @@
 void DumpModuleInfoAction::ExecuteAction() {
   assert(isCurrentFileAST() && "dumping non-AST?");
   // Set up the output file.
-  std::unique_ptr 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();
 
Index: clang/include/clang/Frontend/FrontendActions.h
===
--- clang/include/clang/Frontend/FrontendActions.h
+++ clang/include/clang/Frontend/FrontendActions.h
@@ -177,9 +177,8 @@
 /// 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 OutputStream;
 
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
@@ -188,6 +187,9 @@
   void ExecuteAction() override;
 
 public:
+  DumpModuleInfoAction() = default;
+  explicit DumpModuleInfoAction(std::shared_ptr Out)
+  : OutputStream(Out) {}
   bool hasPCHSupport() const override { return false; }
   bool hasASTFileSupport() const override { return true; }
   bool hasIRSupport() const override { return false; }


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -2179,8 +2179,9 @@
 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();
+std::shared_ptr Out(
+&result.GetOutputStream().AsRawOstream(), [](llvm::raw_ostream *) {});
+clang::DumpModuleInfoAction dump_module_info(Out);
 // DumpModuleInfoAction requires ObjectFilePCHContainerReader.
 compiler.getPCHContainerOperations()->registerReader(
 std::make_unique());
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -780,14 +780,12 @@
 void DumpModuleInfoAction::ExecuteAction() {
   assert(isCurrentFileAST() && "dumping non-AST?");
   // Set up the output file.
-  std::unique_ptr 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();
 
Index: clang/include/clang/Frontend/FrontendActions.h

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

2023-03-29 Thread Tom Honermann via Phabricator via lldb-commits

tahonermann added inline comments.



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:2182-2183
 
-clang::DumpModuleInfoAction dump_module_info;
-dump_module_info.OutputStream = &result.GetOutputStream().AsRawOstream();
+std::shared_ptr Out(
+&result.GetOutputStream().AsRawOstream(), [](llvm::raw_ostream *) {});
+clang::DumpModuleInfoAction dump_module_info(Out);

Use of `std::shared_ptr` with a deleter that doesn't do anything is unusual; I 
think this deserves a comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146412/new/

https://reviews.llvm.org/D146412

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147084: [lldb] Move ObjectFileJIT to lldbExpression

2023-03-29 Thread Pavel Labath via Phabricator via lldb-commits

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Makes sense to me. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147084/new/

https://reviews.llvm.org/D147084

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

2023-03-29 Thread Mariya Podchishchaeva via Phabricator via lldb-commits

Fznamznon updated this revision to Diff 509420.
Fznamznon added a comment.

Add comment explaining custom deleter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146412/new/

https://reviews.llvm.org/D146412

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


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -2179,8 +2179,11 @@
 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 Out(
+&result.GetOutputStream().AsRawOstream(), [](llvm::raw_ostream *) {});
+clang::DumpModuleInfoAction dump_module_info(Out);
 // DumpModuleInfoAction requires ObjectFilePCHContainerReader.
 compiler.getPCHContainerOperations()->registerReader(
 std::make_unique());
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -780,14 +780,12 @@
 void DumpModuleInfoAction::ExecuteAction() {
   assert(isCurrentFileAST() && "dumping non-AST?");
   // Set up the output file.
-  std::unique_ptr 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();
 
Index: clang/include/clang/Frontend/FrontendActions.h
===
--- clang/include/clang/Frontend/FrontendActions.h
+++ clang/include/clang/Frontend/FrontendActions.h
@@ -177,9 +177,8 @@
 /// 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 OutputStream;
 
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
@@ -188,6 +187,9 @@
   void ExecuteAction() override;
 
 public:
+  DumpModuleInfoAction() = default;
+  explicit DumpModuleInfoAction(std::shared_ptr Out)
+  : OutputStream(Out) {}
   bool hasPCHSupport() const override { return false; }
   bool hasASTFileSupport() const override { return true; }
   bool hasIRSupport() const override { return false; }


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -2179,8 +2179,11 @@
 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 Out(
+&result.GetOutputStream().AsRawOstream(), [](llvm::raw_ostream *) {});
+clang::DumpModuleInfoAction dump_module_info(Out);
 // DumpModuleInfoAction requires ObjectFilePCHContainerReader.
 compiler.getPCHContainerOperations()->registerReader(
 std::make_unique());
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -780,14 +780,12 @@
 void DumpModuleInfoAction::ExecuteAction() {
   assert(isCurrentFileAST() && "dumping non-AST?");
   // Set up the output file.
-  std::unique_ptr 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()

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

2023-03-29 Thread Mariya Podchishchaeva via Phabricator via lldb-commits

Fznamznon added inline comments.



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:2182-2183
 
-clang::DumpModuleInfoAction dump_module_info;
-dump_module_info.OutputStream = &result.GetOutputStream().AsRawOstream();
+std::shared_ptr Out(
+&result.GetOutputStream().AsRawOstream(), [](llvm::raw_ostream *) {});
+clang::DumpModuleInfoAction dump_module_info(Out);

tahonermann wrote:
> Use of `std::shared_ptr` with a deleter that doesn't do anything is unusual; 
> I think this deserves a comment.
Sure, done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146412/new/

https://reviews.llvm.org/D146412

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

2023-03-29 Thread Tom Honermann via Phabricator via lldb-commits

tahonermann accepted this revision.
tahonermann added a comment.

Thank you, Mariya! Looks good to me!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146412/new/

https://reviews.llvm.org/D146412

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] a7005d7 - [lldb] Unify target triples across compiler and linker invocations

2023-03-29 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2023-03-29T14:58:19-07:00
New Revision: a7005d7813b392f6d1c749239089bbc5cd6e0c54

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

LOG: [lldb] Unify target triples across compiler and linker invocations

rdar://107364766

Added: 


Modified: 
lldb/test/API/macosx/universal64/Makefile

Removed: 




diff  --git a/lldb/test/API/macosx/universal64/Makefile 
b/lldb/test/API/macosx/universal64/Makefile
index f763f3ae2f6c9..ea773863cedc1 100644
--- a/lldb/test/API/macosx/universal64/Makefile
+++ b/lldb/test/API/macosx/universal64/Makefile
@@ -18,7 +18,7 @@ fat.arm64.out: fat.arm64.o
$(CC) -isysroot $(SDKROOT) -target arm64-apple-macosx10.9 -o $@ $<
 
 fat.x86_64.o: main.c
-   $(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64-apple-macosx11 -c -o 
$@ $<
+   $(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64-apple-macosx10.9 -c -o 
$@ $<
 
 fat.arm64.o: main.c
-   $(CC) -isysroot $(SDKROOT) -g -O0 -target arm64-apple-macosx11 -c -o $@ 
$<
+   $(CC) -isysroot $(SDKROOT) -g -O0 -target arm64-apple-macosx10.9 -c -o 
$@ $<



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential use-after-free in DumpModuleInfoAction::ExecuteAction()

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

[Lldb-commits] [PATCH] D147084: [lldb] Move ObjectFileJIT to lldbExpression

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

[Lldb-commits] [PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

[Lldb-commits] [lldb] a7005d7 - [lldb] Unify target triples across compiler and linker invocations

9 matches

Site Navigation

Mail list logo

Footer information