date:20230330

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

2023-03-30 Thread Pavel Kosov via Phabricator via lldb-commits

kpdev42 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

___
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-30 Thread Mariya Podchishchaeva via Phabricator via lldb-commits

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

Rebase.


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();
+OutputStream.reset(new l

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

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

Fznamznon added a comment.

The test fail is unrelated, seem to be broken by 
https://reviews.llvm.org/D146811 .


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-30 Thread Mariya Podchishchaeva via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG42ae055b4c92: [NFC] Fix potential for use-after-free in 
DumpModuleInfoAction (authored by Fznamznon).

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(O

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

2023-03-30 Thread 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 OutputStream;
 
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
@@ -188,6 +187,9 @@ class DumpModuleInfoAction : public ASTFrontendAction {
   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; }

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 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 Out(
+&result.GetOutputStream().AsRawOstream(), [](llvm::raw_ostream *) {});
+clang::DumpModuleInfoAction dump_module_info(Out);
 // DumpModuleInfoAction requires ObjectFilePCHContainerReader.
 compiler.getPCHContainerOperations()->registerReader(
 std::make_unique());



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

[Lldb-commits] [lldb] e64cc75 - [lldb-server/linux] Use waitpid(-1) to collect inferior events

2023-03-30 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2023-03-30T12:48:36+02:00
New Revision: e64cc756819d567f453467bf7cc16599ad296fdd

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

LOG: [lldb-server/linux] Use waitpid(-1) to collect inferior events

This is a follow-up to D116372, which had a rather unfortunate side
effect of making the processing of a single SIGCHLD quadratic in the
number of threads -- which does not matter for simple applications, but
can get really bad for applications with thousands of threads.

This patch fixes the problem by implementing the other possibility
mentioned in the first patch -- doing waitpid(-1) centrally and then
routing the events to the correct process instance. The "uncollected"
threads are held in the process factory class -- which I've renamed to
Manager for this purpose, as it now does more than creating processes.

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

Added: 


Modified: 
lldb/include/lldb/Host/common/NativeProcessProtocol.h
lldb/source/Host/common/NativeProcessProtocol.cpp
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/tools/lldb-server/lldb-gdbserver.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h 
b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
index 057c07b1536f..744699210d4b 100644
--- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -256,7 +256,7 @@ class NativeProcessProtocol {
   virtual Status GetFileLoadAddress(const llvm::StringRef &file_name,
 lldb::addr_t &load_addr) = 0;
 
-  /// Extension flag constants, returned by Factory::GetSupportedExtensions()
+  /// Extension flag constants, returned by Manager::GetSupportedExtensions()
   /// and passed to SetEnabledExtension()
   enum class Extension {
 multiprocess = (1u << 0),
@@ -272,9 +272,14 @@ class NativeProcessProtocol {
 LLVM_MARK_AS_BITMASK_ENUM(siginfo_read)
   };
 
-  class Factory {
+  class Manager {
   public:
-virtual ~Factory();
+Manager(MainLoop &mainloop) : m_mainloop(mainloop) {}
+Manager(const Manager &) = delete;
+Manager &operator=(const Manager &) = delete;
+
+virtual ~Manager();
+
 /// Launch a process for debugging.
 ///
 /// \param[in] launch_info
@@ -294,8 +299,8 @@ class NativeProcessProtocol {
 /// A NativeProcessProtocol shared pointer if the operation succeeded 
or
 /// an error object if it failed.
 virtual llvm::Expected>
-Launch(ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
-   MainLoop &mainloop) const = 0;
+Launch(ProcessLaunchInfo &launch_info,
+   NativeDelegate &native_delegate) = 0;
 
 /// Attach to an existing process.
 ///
@@ -316,14 +321,16 @@ class NativeProcessProtocol {
 /// A NativeProcessProtocol shared pointer if the operation succeeded 
or
 /// an error object if it failed.
 virtual llvm::Expected>
-Attach(lldb::pid_t pid, NativeDelegate &native_delegate,
-   MainLoop &mainloop) const = 0;
+Attach(lldb::pid_t pid, NativeDelegate &native_delegate) = 0;
 
 /// Get the bitmask of extensions supported by this process plugin.
 ///
 /// \return
 /// A NativeProcessProtocol::Extension bitmask.
 virtual Extension GetSupportedExtensions() const { return {}; }
+
+  protected:
+MainLoop &m_mainloop;
   };
 
   /// Notify tracers that the target process will resume

diff  --git a/lldb/source/Host/common/NativeProcessProtocol.cpp 
b/lldb/source/Host/common/NativeProcessProtocol.cpp
index 975b3d0f7d53..95258e1ddc5e 100644
--- a/lldb/source/Host/common/NativeProcessProtocol.cpp
+++ b/lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -759,4 +759,4 @@ void NativeProcessProtocol::DoStopIDBumped(uint32_t /* 
newBumpId */) {
   // Default implementation does nothing.
 }
 
-NativeProcessProtocol::Factory::~Factory() = default;
+NativeProcessProtocol::Manager::~Manager() = default;

diff  --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSD/Native

[Lldb-commits] [PATCH] D146977: [lldb-server/linux] Use waitpid(-1) to collect inferior events

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe64cc756819d: [lldb-server/linux] Use waitpid(-1) to collect 
inferior events (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146977

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/tools/lldb-server/lldb-gdbserver.cpp

Index: lldb/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -62,16 +62,16 @@
 
 namespace {
 #if defined(__linux__)
-typedef process_linux::NativeProcessLinux::Factory NativeProcessFactory;
+typedef process_linux::NativeProcessLinux::Manager NativeProcessManager;
 #elif defined(__FreeBSD__)
-typedef process_freebsd::NativeProcessFreeBSD::Factory NativeProcessFactory;
+typedef process_freebsd::NativeProcessFreeBSD::Manager NativeProcessManager;
 #elif defined(__NetBSD__)
-typedef process_netbsd::NativeProcessNetBSD::Factory NativeProcessFactory;
+typedef process_netbsd::NativeProcessNetBSD::Manager NativeProcessManager;
 #elif defined(_WIN32)
-typedef NativeProcessWindows::Factory NativeProcessFactory;
+typedef NativeProcessWindows::Manager NativeProcessManager;
 #else
 // Dummy implementation to make sure the code compiles
-class NativeProcessFactory : public NativeProcessProtocol::Factory {
+class NativeProcessManager : public NativeProcessProtocol::Manager {
 public:
   llvm::Expected>
   Launch(ProcessLaunchInfo &launch_info,
@@ -431,8 +431,8 @@
 return 1;
   }
 
-  NativeProcessFactory factory;
-  GDBRemoteCommunicationServerLLGS gdb_server(mainloop, factory);
+  NativeProcessManager manager(mainloop);
+  GDBRemoteCommunicationServerLLGS gdb_server(mainloop, manager);
 
   llvm::StringRef host_and_port;
   if (!Inputs.empty()) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -35,7 +35,7 @@
   // Constructors and Destructors
   GDBRemoteCommunicationServerLLGS(
   MainLoop &mainloop,
-  const NativeProcessProtocol::Factory &process_factory);
+  NativeProcessProtocol::Manager &process_manager);
 
   void SetLaunchInfo(const ProcessLaunchInfo &info);
 
@@ -99,7 +99,7 @@
 protected:
   MainLoop &m_mainloop;
   MainLoop::ReadHandleUP m_network_handle_up;
-  const NativeProcessProtocol::Factory &m_process_factory;
+  NativeProcessProtocol::Manager &m_process_manager;
   lldb::tid_t m_current_tid = LLDB_INVALID_THREAD_ID;
   lldb::tid_t m_continue_tid = LLDB_INVALID_THREAD_ID;
   NativeProcessProtocol *m_current_process;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -69,9 +69,9 @@
 
 // GDBRemoteCommunicationServerLLGS constructor
 GDBRemoteCommunicationServerLLGS::GDBRemoteCommunicationServerLLGS(
-MainLoop &mainloop, const NativeProcessProtocol::Factory &process_factory)
+MainLoop &mainloop, NativeProcessProtocol::Manager &process_manager)
 : GDBRemoteCommunicationServerCommon(), m_mainloop(mainloop),
-  m_process_factory(process_factory), m_current_process(nullptr),
+  m_process_manager(process_manager), m_current_process(nullptr),
   m_continue_process(nullptr), m_stdio_communication() {
   RegisterPacketHandlers();
 }
@@ -286,8 +286,7 @@
 std::lock_guard guard(m_debugged_process_mutex);
 assert(m_debugged_processes.empty() && "lldb-server creating debugged "
"process but one already exists");
-auto process_or =
-m_process_factory.Launch(m_process_launch_info, *this, m_mainloop);
+auto process_or = m_process_manager.Launch(m_process_launch_info, *this);
 if (!process_or)
   return Status(process_or.t

[Lldb-commits] [PATCH] D146977: [lldb-server/linux] Use waitpid(-1) to collect inferior events

2023-03-30 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve added a comment.

@labath I believe this broke the lldb bots, because the declaration of some 
virtual functions had the const qualifier removed, but not the definition:

  
/Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/lldb/tools/lldb-server/lldb-gdbserver.cpp:79:36:
 error: non-virtual member function marked 'override' hides virtual member 
function
   MainLoop &mainloop) const override {
 ^
  
/Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/lldb/include/lldb/Host/common/NativeProcessProtocol.h:302:5:
 note: hidden overloaded virtual function 
'lldb_private::NativeProcessProtocol::Manager::Launch' declared here: different 
number of parameters (2 vs 3)
  Launch(ProcessLaunchInfo &launch_info,
  ^
  
/Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/lldb/tools/lldb-server/lldb-gdbserver.cpp:84:36:
 error: non-virtual member function marked 'override' hides virtual member 
function
   MainLoop &mainloop) const override {
 ^
  
/Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/lldb/include/lldb/Host/common/NativeProcessProtocol.h:324:5:
 note: hidden overloaded virtual function 
'lldb_private::NativeProcessProtocol::Manager::Attach' declared here: different 
number of parameters (2 vs 3)
  Attach(lldb::pid_t pid, NativeDelegate &native_delegate) = 0;
  ^
  
/Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/lldb/tools/lldb-server/lldb-gdbserver.cpp:434:24:
 error: variable type '(anonymous namespace)::NativeProcessManager' is an 
abstract class
NativeProcessManager manager(mainloop);
 ^
  
/Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/lldb/include/lldb/Host/common/NativeProcessProtocol.h:302:5:
 note: unimplemented pure virtual method 'Launch' in 'NativeProcessManager'
  Launch(ProcessLaunchInfo &launch_info,
  ^
  
/Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/lldb/include/lldb/Host/common/NativeProcessProtocol.h:324:5:
 note: unimplemented pure virtual method 'Attach' in 'NativeProcessManager'
  Attach(lldb::pid_t pid, NativeDelegate &native_delegate) = 0;

Could you have a look? 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/53015/console


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146977

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

[Lldb-commits] [PATCH] D146977: [lldb-server/linux] Use waitpid(-1) to collect inferior events

2023-03-30 Thread Nico Weber via Phabricator via lldb-commits

thakis added a comment.

Looks like this breaks building on Mac: 
http://45.33.8.238/macm1/57676/step_4.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146977

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

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

2023-03-30 Thread Michael Buch via Phabricator via lldb-commits

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1482
+if (prev_base) {
+  clang::CXXRecordDecl *prev_base_decl =
+  prev_base->getType()->getAsCXXRecordDecl();

Should we add a comment describing why this block is necessary?

Perhaps even putting it into its own helper function



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1487
+assert(it != layout_info.base_offsets.end());
+if (it->second.getQuantity() == member_byte_offset) {
+  prev_base_decl->markEmpty();

The idea seem reasonable in general.

Though this wouldn't work for overlapping member offsets. E.g.,:

```
struct C
{
 long c,d;
};

struct D
{
};

struct B
{
  [[no_unique_address]] D x;
};

struct E
{
  [[no_unique_address]] D x;
};

struct Foo : B, E, C {

};
```

Here `B` and `C` have offset `0x0`, but `E` can have offset `0x1`. So we 
wouldn't attach the attribute for `E` and would still crash. Maybe we should 
check for overlapping offsets, not just equal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D147246: [lldb] Replace sprintf with snprintf (NFC)

2023-03-30 Thread Dave Lee via Phabricator via lldb-commits

kastiglione created this revision.
kastiglione added a reviewer: JDevlieghere.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

On macOS, `sprintf` is deprecated, using `snprintf` is recommended instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147246

Files:
  lldb/source/Core/SourceManager.cpp
  lldb/source/DataFormatters/StringPrinter.cpp
  lldb/source/Host/macosx/objcxx/Host.mm


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -719,12 +719,12 @@
 const Args &args) {
   size_t count = args.GetArgumentCount();
   char buf[50]; // long enough for 'argXXX'
-  memset(buf, 0, 50);
-  sprintf(buf, "%sCount", prefix);
+  memset(buf, 0, sizeof(buf));
+  snprintf(buf, sizeof(buf), "%sCount", prefix);
   xpc_dictionary_set_int64(message, buf, count);
   for (size_t i = 0; i < count; i++) {
-memset(buf, 0, 50);
-sprintf(buf, "%s%zi", prefix, i);
+memset(buf, 0, sizeof(buf));
+snprintf(buf, sizeof(buf), "%s%zi", prefix, i);
 xpc_dictionary_set_string(message, buf, args.GetArgumentAtIndex(i));
   }
 }
Index: lldb/source/DataFormatters/StringPrinter.cpp
===
--- lldb/source/DataFormatters/StringPrinter.cpp
+++ lldb/source/DataFormatters/StringPrinter.cpp
@@ -154,11 +154,11 @@
   switch (escape_style) {
   case StringPrinter::EscapeStyle::CXX:
 // Prints 4 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\x%02x", *buffer);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\x%02x", *buffer);
 break;
   case StringPrinter::EscapeStyle::Swift:
 // Prints up to 6 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\u{%x}", *buffer);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\u{%x}", *buffer);
 break;
   }
   lldbassert(escaped_len > 0 && "unknown string escape style");
@@ -201,11 +201,11 @@
   switch (escape_style) {
   case StringPrinter::EscapeStyle::CXX:
 // Prints 10 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\U%08x", codepoint);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\U%08x", 
codepoint);
 break;
   case StringPrinter::EscapeStyle::Swift:
 // Prints up to 12 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\u{%x}", codepoint);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\u{%x}", 
codepoint);
 break;
   }
   lldbassert(escaped_len > 0 && "unknown string escape style");
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -205,7 +205,8 @@
   }
 
   char buffer[3];
-  sprintf(buffer, "%2.2s", (line == curr_line) ? current_line_cstr : "");
+  snprintf(buffer, sizeof(buffer), "%2.2s",
+   (line == curr_line) ? current_line_cstr : "");
   std::string current_line_highlight(buffer);
 
   auto debugger_sp = m_debugger_wp.lock();


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -719,12 +719,12 @@
 const Args &args) {
   size_t count = args.GetArgumentCount();
   char buf[50]; // long enough for 'argXXX'
-  memset(buf, 0, 50);
-  sprintf(buf, "%sCount", prefix);
+  memset(buf, 0, sizeof(buf));
+  snprintf(buf, sizeof(buf), "%sCount", prefix);
   xpc_dictionary_set_int64(message, buf, count);
   for (size_t i = 0; i < count; i++) {
-memset(buf, 0, 50);
-sprintf(buf, "%s%zi", prefix, i);
+memset(buf, 0, sizeof(buf));
+snprintf(buf, sizeof(buf), "%s%zi", prefix, i);
 xpc_dictionary_set_string(message, buf, args.GetArgumentAtIndex(i));
   }
 }
Index: lldb/source/DataFormatters/StringPrinter.cpp
===
--- lldb/source/DataFormatters/StringPrinter.cpp
+++ lldb/source/DataFormatters/StringPrinter.cpp
@@ -154,11 +154,11 @@
   switch (escape_style) {
   case StringPrinter::EscapeStyle::CXX:
 // Prints 4 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\x%02x", *buffer);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\x%02x", *buffer);
 break;
   case StringPrinter::EscapeStyle::Swift:
 // Prints up to 6 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\u{%x}", *buffer);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\u{%x}", *buffer);
 break;
   }
   lldbassert(escaped_len > 0 && "unknown string escape s

[Lldb-commits] [PATCH] D147246: [lldb] Replace sprintf with snprintf (NFC)

2023-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

This looks fine, but I wonder if using `llvm::formatv` wouldn't make this a 
whole lot easier?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147246

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

[Lldb-commits] [PATCH] D147246: [lldb] Replace sprintf with snprintf (NFC)

2023-03-30 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added a comment.

That may be better, I don't really know. I would prefer to merge this as is 
(and leave a printf->formatv migration for another time).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147246

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

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

2023-03-30 Thread Dave Lee via Phabricator via lldb-commits

kastiglione created this revision.
kastiglione added a reviewer: JDevlieghere.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Following the work done by @jdevlieghere in D143690 
, this changes how Clang module build
events are emitted.

Instead of one Progress event per module being built, a single Progress event 
is used to
encompass all modules, and each module build is sent as an `Increment` update.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147248

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py


Index: 
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
===
--- 
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
+++ 
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
@@ -43,4 +43,4 @@
 event = lldbutil.fetch_next_event(self, listener, broadcaster)
 payload = lldb.SBDebugger.GetProgressFromEvent(event)
 message = payload[0]
-self.assertEqual(message, "Currently building module MyModule")
+self.assertEqual(message, "Building Clang modules")
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -68,7 +68,7 @@
 
 private:
   bool HandleModuleRemark(const clang::Diagnostic &info);
-  void SetCurrentModuleProgress(llvm::StringRef module_name);
+  void SetCurrentModuleProgress(const std::string &module_name);
 
   typedef std::pair
   IDAndDiagnostic;
@@ -208,8 +208,9 @@
 if (m_module_build_stack.empty()) {
   m_current_progress_up = nullptr;
 } else {
-  // Update the progress to re-show the module that was currently being
-  // built from the time the now completed module was originally began.
+  // When the just completed module began building, a module that depends 
on
+  // it ("module A") was effectively paused. Update the progress to re-show
+  // "module A" as continuing to be built.
   const auto &resumed_module_name = m_module_build_stack.back();
   SetCurrentModuleProgress(resumed_module_name);
 }
@@ -224,13 +225,12 @@
 }
 
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-llvm::StringRef module_name) {
-  // Ensure the ordering of:
-  //   1. Completing the existing progress event.
-  //   2. Beginining a new progress event.
-  m_current_progress_up = nullptr;
-  m_current_progress_up = std::make_unique(
-  llvm::formatv("Currently building module {0}", module_name));
+const std::string &module_name) {
+  if (!m_current_progress_up)
+m_current_progress_up =
+std::make_unique("Building Clang modules");
+
+  m_current_progress_up->Increment(1, module_name);
 }
 
 ClangModulesDeclVendor::ClangModulesDeclVendor()


Index: lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
===
--- lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
+++ lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
@@ -43,4 +43,4 @@
 event = lldbutil.fetch_next_event(self, listener, broadcaster)
 payload = lldb.SBDebugger.GetProgressFromEvent(event)
 message = payload[0]
-self.assertEqual(message, "Currently building module MyModule")
+self.assertEqual(message, "Building Clang modules")
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -68,7 +68,7 @@
 
 private:
   bool HandleModuleRemark(const clang::Diagnostic &info);
-  void SetCurrentModuleProgress(llvm::StringRef module_name);
+  void SetCurrentModuleProgress(const std::string &module_name);
 
   typedef std::pair
   IDAndDiagnostic;
@@ -208,8 +208,9 @@
 if (m_module_build_stack.empty()) {
   m_current_progress_up = nullptr;
 } else {
-  // Update the progress to re-show the module that was currently being
-  // built from the time the now completed module was originally began.
+  // When the just completed module began building, a module that depends on
+  // it ("module A") was effectively paused. Update the progress to re-show
+  //

[Lldb-commits] [PATCH] D147249: [lldb] Replace deprecated CFPropertyListWriteToStream (NFC)

2023-03-30 Thread Dave Lee via Phabricator via lldb-commits

kastiglione created this revision.
kastiglione added a reviewer: JDevlieghere.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Replace `CFPropertyListWriteToStream` with its recommended replacement, 
`CFPropertyListWrite`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147249

Files:
  lldb/source/Host/macosx/cfcpp/CFCData.cpp


Index: lldb/source/Host/macosx/cfcpp/CFCData.cpp
===
--- lldb/source/Host/macosx/cfcpp/CFCData.cpp
+++ lldb/source/Host/macosx/cfcpp/CFCData.cpp
@@ -47,8 +47,7 @@
   CFCReleaser stream(
   ::CFWriteStreamCreateWithAllocatedBuffers(alloc, alloc));
   ::CFWriteStreamOpen(stream.get());
-  CFIndex len =
-  ::CFPropertyListWriteToStream(plist, stream.get(), format, NULL);
+  CFIndex len = ::CFPropertyListWrite(plist, stream.get(), format, 0, nullptr);
   if (len > 0)
 reset((CFDataRef)::CFWriteStreamCopyProperty(stream.get(),
  
kCFStreamPropertyDataWritten));


Index: lldb/source/Host/macosx/cfcpp/CFCData.cpp
===
--- lldb/source/Host/macosx/cfcpp/CFCData.cpp
+++ lldb/source/Host/macosx/cfcpp/CFCData.cpp
@@ -47,8 +47,7 @@
   CFCReleaser stream(
   ::CFWriteStreamCreateWithAllocatedBuffers(alloc, alloc));
   ::CFWriteStreamOpen(stream.get());
-  CFIndex len =
-  ::CFPropertyListWriteToStream(plist, stream.get(), format, NULL);
+  CFIndex len = ::CFPropertyListWrite(plist, stream.get(), format, 0, nullptr);
   if (len > 0)
 reset((CFDataRef)::CFWriteStreamCopyProperty(stream.get(),
  kCFStreamPropertyDataWritten));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147249: [lldb] Replace deprecated CFPropertyListWriteToStream (NFC)

2023-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147249

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

[Lldb-commits] [lldb] 80b476a - [lldb] Replace deprecated CFPropertyListWriteToStream (NFC)

2023-03-30 Thread Dave Lee via lldb-commits


Author: Dave Lee
Date: 2023-03-30T11:21:09-07:00
New Revision: 80b476a902a8e2ab662b2e2a38e2afc55ce85c37

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

LOG: [lldb] Replace deprecated CFPropertyListWriteToStream (NFC)

Replace `CFPropertyListWriteToStream` with its recommended replacement, 
`CFPropertyListWrite`.

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

Added: 


Modified: 
lldb/source/Host/macosx/cfcpp/CFCData.cpp

Removed: 




diff  --git a/lldb/source/Host/macosx/cfcpp/CFCData.cpp 
b/lldb/source/Host/macosx/cfcpp/CFCData.cpp
index ed79e36c8dfb8..6261a3a8b07ed 100644
--- a/lldb/source/Host/macosx/cfcpp/CFCData.cpp
+++ b/lldb/source/Host/macosx/cfcpp/CFCData.cpp
@@ -47,8 +47,7 @@ CFDataRef CFCData::Serialize(CFPropertyListRef plist,
   CFCReleaser stream(
   ::CFWriteStreamCreateWithAllocatedBuffers(alloc, alloc));
   ::CFWriteStreamOpen(stream.get());
-  CFIndex len =
-  ::CFPropertyListWriteToStream(plist, stream.get(), format, NULL);
+  CFIndex len = ::CFPropertyListWrite(plist, stream.get(), format, 0, nullptr);
   if (len > 0)
 reset((CFDataRef)::CFWriteStreamCopyProperty(stream.get(),
  
kCFStreamPropertyDataWritten));



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

[Lldb-commits] [PATCH] D147249: [lldb] Replace deprecated CFPropertyListWriteToStream (NFC)

2023-03-30 Thread Dave Lee via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG80b476a902a8: [lldb] Replace deprecated 
CFPropertyListWriteToStream (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147249

Files:
  lldb/source/Host/macosx/cfcpp/CFCData.cpp


Index: lldb/source/Host/macosx/cfcpp/CFCData.cpp
===
--- lldb/source/Host/macosx/cfcpp/CFCData.cpp
+++ lldb/source/Host/macosx/cfcpp/CFCData.cpp
@@ -47,8 +47,7 @@
   CFCReleaser stream(
   ::CFWriteStreamCreateWithAllocatedBuffers(alloc, alloc));
   ::CFWriteStreamOpen(stream.get());
-  CFIndex len =
-  ::CFPropertyListWriteToStream(plist, stream.get(), format, NULL);
+  CFIndex len = ::CFPropertyListWrite(plist, stream.get(), format, 0, nullptr);
   if (len > 0)
 reset((CFDataRef)::CFWriteStreamCopyProperty(stream.get(),
  
kCFStreamPropertyDataWritten));


Index: lldb/source/Host/macosx/cfcpp/CFCData.cpp
===
--- lldb/source/Host/macosx/cfcpp/CFCData.cpp
+++ lldb/source/Host/macosx/cfcpp/CFCData.cpp
@@ -47,8 +47,7 @@
   CFCReleaser stream(
   ::CFWriteStreamCreateWithAllocatedBuffers(alloc, alloc));
   ::CFWriteStreamOpen(stream.get());
-  CFIndex len =
-  ::CFPropertyListWriteToStream(plist, stream.get(), format, NULL);
+  CFIndex len = ::CFPropertyListWrite(plist, stream.get(), format, 0, nullptr);
   if (len > 0)
 reset((CFDataRef)::CFWriteStreamCopyProperty(stream.get(),
  kCFStreamPropertyDataWritten));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

2023-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:228
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-llvm::StringRef module_name) {
-  // Ensure the ordering of:
-  //   1. Completing the existing progress event.
-  //   2. Beginining a new progress event.
-  m_current_progress_up = nullptr;
-  m_current_progress_up = std::make_unique(
-  llvm::formatv("Currently building module {0}", module_name));
+const std::string &module_name) {
+  if (!m_current_progress_up)

`Progress::Increment` is taking a `std::string` by value, so if you think this 
will ever get called with an r-value reference, you should do the same and 
`std::move` it. Otherwise the StringRef or the `const std::string&` both 
require a copy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147248

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

[Lldb-commits] [PATCH] D147252: [lldb][NFC] Move various constructor definitions from .h to .cpp

2023-03-30 Thread River Riddle via Phabricator via lldb-commits

rriddle created this revision.
Herald added a subscriber: bollu.
Herald added a project: All.
rriddle requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

I ran into issues with linking downstream language plugin to liblldb in
debug builds, hitting link time errors of form:

  undefined reference to `vtable for  lldb_private::'

Anchoring the vtable to the .cpp files resolved those issues.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147252

Files:
  lldb/include/lldb/Expression/ExpressionVariable.h
  lldb/include/lldb/Expression/Materializer.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Expression/ExpressionVariable.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Symbol/TypeSystem.cpp


Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -36,6 +36,7 @@
 bool LanguageSet::Empty() const { return bitvector.none(); }
 bool LanguageSet::operator[](unsigned i) const { return bitvector[i]; }
 
+TypeSystem::TypeSystem() = default;
 TypeSystem::~TypeSystem() = default;
 
 static TypeSystemSP CreateInstanceHelper(lldb::LanguageType language,
Index: lldb/source/Expression/Materializer.cpp
===
--- lldb/source/Expression/Materializer.cpp
+++ lldb/source/Expression/Materializer.cpp
@@ -1598,5 +1598,7 @@
   m_process_address = LLDB_INVALID_ADDRESS;
 }
 
+Materializer::PersistentVariableDelegate::PersistentVariableDelegate() =
+default;
 Materializer::PersistentVariableDelegate::~PersistentVariableDelegate() =
 default;
Index: lldb/source/Expression/ExpressionVariable.cpp
===
--- lldb/source/Expression/ExpressionVariable.cpp
+++ lldb/source/Expression/ExpressionVariable.cpp
@@ -15,6 +15,8 @@
 
 using namespace lldb_private;
 
+ExpressionVariable::ExpressionVariable(LLVMCastKind kind)
+: m_flags(0), m_kind(kind) {}
 ExpressionVariable::~ExpressionVariable() = default;
 
 uint8_t *ExpressionVariable::GetValueBytes() {
@@ -30,6 +32,8 @@
   return nullptr;
 }
 
+PersistentExpressionState::PersistentExpressionState(LLVMCastKind kind)
+: m_kind(kind) {}
 PersistentExpressionState::~PersistentExpressionState() = default;
 
 lldb::addr_t PersistentExpressionState::LookupSymbol(ConstString name) {
Index: lldb/include/lldb/Symbol/TypeSystem.h
===
--- lldb/include/lldb/Symbol/TypeSystem.h
+++ lldb/include/lldb/Symbol/TypeSystem.h
@@ -77,6 +77,7 @@
public std::enable_shared_from_this {
 public:
   // Constructors and Destructors
+  TypeSystem();
   ~TypeSystem() override;
 
   // LLVM RTTI support
Index: lldb/include/lldb/Expression/Materializer.h
===
--- lldb/include/lldb/Expression/Materializer.h
+++ lldb/include/lldb/Expression/Materializer.h
@@ -69,6 +69,7 @@
 
   class PersistentVariableDelegate {
   public:
+PersistentVariableDelegate();
 virtual ~PersistentVariableDelegate();
 virtual ConstString GetName() = 0;
 virtual void DidDematerialize(lldb::ExpressionVariableSP &variable) = 0;
Index: lldb/include/lldb/Expression/ExpressionVariable.h
===
--- lldb/include/lldb/Expression/ExpressionVariable.h
+++ lldb/include/lldb/Expression/ExpressionVariable.h
@@ -29,8 +29,7 @@
 
   LLVMCastKind getKind() const { return m_kind; }
 
-  ExpressionVariable(LLVMCastKind kind) : m_flags(0), m_kind(kind) {}
-
+  ExpressionVariable(LLVMCastKind kind);
   virtual ~ExpressionVariable();
 
   std::optional GetByteSize() { return m_frozen_sp->GetByteSize(); }
@@ -208,8 +207,7 @@
 
   LLVMCastKind getKind() const { return m_kind; }
 
-  PersistentExpressionState(LLVMCastKind kind) : m_kind(kind) {}
-
+  PersistentExpressionState(LLVMCastKind kind);
   virtual ~PersistentExpressionState();
 
   virtual lldb::ExpressionVariableSP


Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -36,6 +36,7 @@
 bool LanguageSet::Empty() const { return bitvector.none(); }
 bool LanguageSet::operator[](unsigned i) const { return bitvector[i]; }
 
+TypeSystem::TypeSystem() = default;
 TypeSystem::~TypeSystem() = default;
 
 static TypeSystemSP CreateInstanceHelper(lldb::LanguageType language,
Index: lldb/source/Expression/Materializer.cpp
===
--- lldb/source/Expression/Materializer.cpp
+++ lldb/source/Expression/Materializer.cpp
@@ -1598,5 +1598,7 @@
   m_process_address = LLDB_INVALID_ADDRESS;
 }
 
+Materializer::PersistentVariableDelegate::PersistentVariableDelegate() =
+

[Lldb-commits] [PATCH] D147252: [lldb][NFC] Move various constructor definitions from .h to .cpp

2023-03-30 Thread walter erquinigo via Phabricator via lldb-commits

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147252

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

[Lldb-commits] [PATCH] D147246: [lldb] Replace sprintf with snprintf (NFC)

2023-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Ship it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147246

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

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

2023-03-30 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:228
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-llvm::StringRef module_name) {
-  // Ensure the ordering of:
-  //   1. Completing the existing progress event.
-  //   2. Beginining a new progress event.
-  m_current_progress_up = nullptr;
-  m_current_progress_up = std::make_unique(
-  llvm::formatv("Currently building module {0}", module_name));
+const std::string &module_name) {
+  if (!m_current_progress_up)

JDevlieghere wrote:
> `Progress::Increment` is taking a `std::string` by value, so if you think 
> this will ever get called with an r-value reference, you should do the same 
> and `std::move` it. Otherwise the StringRef or the `const std::string&` both 
> require a copy.
Understood. The module names are references, I could change this function to 
take a `string &&`, but that would move the copy to the callee rather than 
here. Either way, I don't see a way to avoid a copy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147248

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

[Lldb-commits] [lldb] d03d98b - [lldb] Replace sprintf with snprintf (NFC)

2023-03-30 Thread Dave Lee via lldb-commits


Author: Dave Lee
Date: 2023-03-30T12:29:05-07:00
New Revision: d03d98b71d03eaf2a84e62e461b4fc87940866f1

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

LOG: [lldb] Replace sprintf with snprintf (NFC)

On macOS, `sprintf` is deprecated, using `snprintf` is recommended instead.

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

Added: 


Modified: 
lldb/source/Core/SourceManager.cpp
lldb/source/DataFormatters/StringPrinter.cpp
lldb/source/Host/macosx/objcxx/Host.mm

Removed: 




diff  --git a/lldb/source/Core/SourceManager.cpp 
b/lldb/source/Core/SourceManager.cpp
index 72fabb42507c7..e0eb327223b57 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -205,7 +205,8 @@ size_t 
SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile(
   }
 
   char buffer[3];
-  sprintf(buffer, "%2.2s", (line == curr_line) ? current_line_cstr : "");
+  snprintf(buffer, sizeof(buffer), "%2.2s",
+   (line == curr_line) ? current_line_cstr : "");
   std::string current_line_highlight(buffer);
 
   auto debugger_sp = m_debugger_wp.lock();

diff  --git a/lldb/source/DataFormatters/StringPrinter.cpp 
b/lldb/source/DataFormatters/StringPrinter.cpp
index 0dd5d518f60b8..4b57e87b4ccdc 100644
--- a/lldb/source/DataFormatters/StringPrinter.cpp
+++ b/lldb/source/DataFormatters/StringPrinter.cpp
@@ -154,11 +154,11 @@ DecodedCharBuffer 
GetPrintableImpl(
   switch (escape_style) {
   case StringPrinter::EscapeStyle::CXX:
 // Prints 4 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\x%02x", *buffer);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\x%02x", *buffer);
 break;
   case StringPrinter::EscapeStyle::Swift:
 // Prints up to 6 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\u{%x}", *buffer);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\u{%x}", *buffer);
 break;
   }
   lldbassert(escaped_len > 0 && "unknown string escape style");
@@ -201,11 +201,11 @@ DecodedCharBuffer 
GetPrintableImpl(
   switch (escape_style) {
   case StringPrinter::EscapeStyle::CXX:
 // Prints 10 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\U%08x", codepoint);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\U%08x", 
codepoint);
 break;
   case StringPrinter::EscapeStyle::Swift:
 // Prints up to 12 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\u{%x}", codepoint);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\u{%x}", 
codepoint);
 break;
   }
   lldbassert(escaped_len > 0 && "unknown string escape style");

diff  --git a/lldb/source/Host/macosx/objcxx/Host.mm 
b/lldb/source/Host/macosx/objcxx/Host.mm
index 94644d905e53f..303a138559e93 100644
--- a/lldb/source/Host/macosx/objcxx/Host.mm
+++ b/lldb/source/Host/macosx/objcxx/Host.mm
@@ -719,12 +719,12 @@ static void PackageXPCArguments(xpc_object_t message, 
const char *prefix,
 const Args &args) {
   size_t count = args.GetArgumentCount();
   char buf[50]; // long enough for 'argXXX'
-  memset(buf, 0, 50);
-  sprintf(buf, "%sCount", prefix);
+  memset(buf, 0, sizeof(buf));
+  snprintf(buf, sizeof(buf), "%sCount", prefix);
   xpc_dictionary_set_int64(message, buf, count);
   for (size_t i = 0; i < count; i++) {
-memset(buf, 0, 50);
-sprintf(buf, "%s%zi", prefix, i);
+memset(buf, 0, sizeof(buf));
+snprintf(buf, sizeof(buf), "%s%zi", prefix, i);
 xpc_dictionary_set_string(message, buf, args.GetArgumentAtIndex(i));
   }
 }



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

[Lldb-commits] [PATCH] D147246: [lldb] Replace sprintf with snprintf (NFC)

2023-03-30 Thread Dave Lee via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd03d98b71d03: [lldb] Replace sprintf with snprintf (NFC) 
(authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147246

Files:
  lldb/source/Core/SourceManager.cpp
  lldb/source/DataFormatters/StringPrinter.cpp
  lldb/source/Host/macosx/objcxx/Host.mm


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -719,12 +719,12 @@
 const Args &args) {
   size_t count = args.GetArgumentCount();
   char buf[50]; // long enough for 'argXXX'
-  memset(buf, 0, 50);
-  sprintf(buf, "%sCount", prefix);
+  memset(buf, 0, sizeof(buf));
+  snprintf(buf, sizeof(buf), "%sCount", prefix);
   xpc_dictionary_set_int64(message, buf, count);
   for (size_t i = 0; i < count; i++) {
-memset(buf, 0, 50);
-sprintf(buf, "%s%zi", prefix, i);
+memset(buf, 0, sizeof(buf));
+snprintf(buf, sizeof(buf), "%s%zi", prefix, i);
 xpc_dictionary_set_string(message, buf, args.GetArgumentAtIndex(i));
   }
 }
Index: lldb/source/DataFormatters/StringPrinter.cpp
===
--- lldb/source/DataFormatters/StringPrinter.cpp
+++ lldb/source/DataFormatters/StringPrinter.cpp
@@ -154,11 +154,11 @@
   switch (escape_style) {
   case StringPrinter::EscapeStyle::CXX:
 // Prints 4 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\x%02x", *buffer);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\x%02x", *buffer);
 break;
   case StringPrinter::EscapeStyle::Swift:
 // Prints up to 6 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\u{%x}", *buffer);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\u{%x}", *buffer);
 break;
   }
   lldbassert(escaped_len > 0 && "unknown string escape style");
@@ -201,11 +201,11 @@
   switch (escape_style) {
   case StringPrinter::EscapeStyle::CXX:
 // Prints 10 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\U%08x", codepoint);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\U%08x", 
codepoint);
 break;
   case StringPrinter::EscapeStyle::Swift:
 // Prints up to 12 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\u{%x}", codepoint);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\u{%x}", 
codepoint);
 break;
   }
   lldbassert(escaped_len > 0 && "unknown string escape style");
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -205,7 +205,8 @@
   }
 
   char buffer[3];
-  sprintf(buffer, "%2.2s", (line == curr_line) ? current_line_cstr : "");
+  snprintf(buffer, sizeof(buffer), "%2.2s",
+   (line == curr_line) ? current_line_cstr : "");
   std::string current_line_highlight(buffer);
 
   auto debugger_sp = m_debugger_wp.lock();


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -719,12 +719,12 @@
 const Args &args) {
   size_t count = args.GetArgumentCount();
   char buf[50]; // long enough for 'argXXX'
-  memset(buf, 0, 50);
-  sprintf(buf, "%sCount", prefix);
+  memset(buf, 0, sizeof(buf));
+  snprintf(buf, sizeof(buf), "%sCount", prefix);
   xpc_dictionary_set_int64(message, buf, count);
   for (size_t i = 0; i < count; i++) {
-memset(buf, 0, 50);
-sprintf(buf, "%s%zi", prefix, i);
+memset(buf, 0, sizeof(buf));
+snprintf(buf, sizeof(buf), "%s%zi", prefix, i);
 xpc_dictionary_set_string(message, buf, args.GetArgumentAtIndex(i));
   }
 }
Index: lldb/source/DataFormatters/StringPrinter.cpp
===
--- lldb/source/DataFormatters/StringPrinter.cpp
+++ lldb/source/DataFormatters/StringPrinter.cpp
@@ -154,11 +154,11 @@
   switch (escape_style) {
   case StringPrinter::EscapeStyle::CXX:
 // Prints 4 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\x%02x", *buffer);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\x%02x", *buffer);
 break;
   case StringPrinter::EscapeStyle::Swift:
 // Prints up to 6 characters, then a \0 terminator.
-escaped_len = sprintf((char *)data, "\\u{%x}", *buffer);
+escaped_len = snprintf((char *)data, max_buffer_size, "\\u{%x}", *buffer);
 break;
   }
   lldbassert(escaped_len > 0 && "unknown string escape style");
@@ -201,11 +201,11 @@
   switch (escape_style) {

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

2023-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:228
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-llvm::StringRef module_name) {
-  // Ensure the ordering of:
-  //   1. Completing the existing progress event.
-  //   2. Beginining a new progress event.
-  m_current_progress_up = nullptr;
-  m_current_progress_up = std::make_unique(
-  llvm::formatv("Currently building module {0}", module_name));
+const std::string &module_name) {
+  if (!m_current_progress_up)

kastiglione wrote:
> JDevlieghere wrote:
> > `Progress::Increment` is taking a `std::string` by value, so if you think 
> > this will ever get called with an r-value reference, you should do the same 
> > and `std::move` it. Otherwise the StringRef or the `const std::string&` 
> > both require a copy.
> Understood. The module names are references, I could change this function to 
> take a `string &&`, but that would move the copy to the callee rather than 
> here. Either way, I don't see a way to avoid a copy.
Yeah if you don't have an r-value reference it cannot be avoided, but I think 
it's still best to make this take it by value so in case there ever is one in 
the caller to this function, it is avoided. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147248

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

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

2023-03-30 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:228
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-llvm::StringRef module_name) {
-  // Ensure the ordering of:
-  //   1. Completing the existing progress event.
-  //   2. Beginining a new progress event.
-  m_current_progress_up = nullptr;
-  m_current_progress_up = std::make_unique(
-  llvm::formatv("Currently building module {0}", module_name));
+const std::string &module_name) {
+  if (!m_current_progress_up)

JDevlieghere wrote:
> kastiglione wrote:
> > JDevlieghere wrote:
> > > `Progress::Increment` is taking a `std::string` by value, so if you think 
> > > this will ever get called with an r-value reference, you should do the 
> > > same and `std::move` it. Otherwise the StringRef or the `const 
> > > std::string&` both require a copy.
> > Understood. The module names are references, I could change this function 
> > to take a `string &&`, but that would move the copy to the callee rather 
> > than here. Either way, I don't see a way to avoid a copy.
> Yeah if you don't have an r-value reference it cannot be avoided, but I think 
> it's still best to make this take it by value so in case there ever is one in 
> the caller to this function, it is avoided. 
Take it by value, or by r-value?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147248

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

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

2023-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:227-234
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-llvm::StringRef module_name) {
-  // Ensure the ordering of:
-  //   1. Completing the existing progress event.
-  //   2. Beginining a new progress event.
-  m_current_progress_up = nullptr;
-  m_current_progress_up = std::make_unique(
-  llvm::formatv("Currently building module {0}", module_name));
+const std::string &module_name) {
+  if (!m_current_progress_up)
+m_current_progress_up =
+std::make_unique("Building Clang modules");
+
+  m_current_progress_up->Increment(1, module_name);

By value, like this ^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147248

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

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

2023-03-30 Thread Dave Lee via Phabricator via lldb-commits

kastiglione updated this revision to Diff 509790.
kastiglione added a comment.

Use std::string argument


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147248

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py


Index: 
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
===
--- 
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
+++ 
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
@@ -43,4 +43,4 @@
 event = lldbutil.fetch_next_event(self, listener, broadcaster)
 payload = lldb.SBDebugger.GetProgressFromEvent(event)
 message = payload[0]
-self.assertEqual(message, "Currently building module MyModule")
+self.assertEqual(message, "Building Clang modules")
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -68,7 +68,7 @@
 
 private:
   bool HandleModuleRemark(const clang::Diagnostic &info);
-  void SetCurrentModuleProgress(llvm::StringRef module_name);
+  void SetCurrentModuleProgress(std::string module_name);
 
   typedef std::pair
   IDAndDiagnostic;
@@ -208,8 +208,9 @@
 if (m_module_build_stack.empty()) {
   m_current_progress_up = nullptr;
 } else {
-  // Update the progress to re-show the module that was currently being
-  // built from the time the now completed module was originally began.
+  // When the just completed module began building, a module that depends 
on
+  // it ("module A") was effectively paused. Update the progress to re-show
+  // "module A" as continuing to be built.
   const auto &resumed_module_name = m_module_build_stack.back();
   SetCurrentModuleProgress(resumed_module_name);
 }
@@ -224,13 +225,12 @@
 }
 
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-llvm::StringRef module_name) {
-  // Ensure the ordering of:
-  //   1. Completing the existing progress event.
-  //   2. Beginining a new progress event.
-  m_current_progress_up = nullptr;
-  m_current_progress_up = std::make_unique(
-  llvm::formatv("Currently building module {0}", module_name));
+std::string module_name) {
+  if (!m_current_progress_up)
+m_current_progress_up =
+std::make_unique("Building Clang modules");
+
+  m_current_progress_up->Increment(1, std::move(module_name));
 }
 
 ClangModulesDeclVendor::ClangModulesDeclVendor()


Index: lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
===
--- lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
+++ lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
@@ -43,4 +43,4 @@
 event = lldbutil.fetch_next_event(self, listener, broadcaster)
 payload = lldb.SBDebugger.GetProgressFromEvent(event)
 message = payload[0]
-self.assertEqual(message, "Currently building module MyModule")
+self.assertEqual(message, "Building Clang modules")
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -68,7 +68,7 @@
 
 private:
   bool HandleModuleRemark(const clang::Diagnostic &info);
-  void SetCurrentModuleProgress(llvm::StringRef module_name);
+  void SetCurrentModuleProgress(std::string module_name);
 
   typedef std::pair
   IDAndDiagnostic;
@@ -208,8 +208,9 @@
 if (m_module_build_stack.empty()) {
   m_current_progress_up = nullptr;
 } else {
-  // Update the progress to re-show the module that was currently being
-  // built from the time the now completed module was originally began.
+  // When the just completed module began building, a module that depends on
+  // it ("module A") was effectively paused. Update the progress to re-show
+  // "module A" as continuing to be built.
   const auto &resumed_module_name = m_module_build_stack.back();
   SetCurrentModuleProgress(resumed_module_name);
 }
@@ -224,13 +225,12 @@
 }
 
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-llvm::StringRef module_name) {
-  // Ensure the ordering of:
-  //   1. Completing the existing progress event.
-  //

[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

2023-03-30 Thread Zequan Wu via Phabricator via lldb-commits

zequanwu added inline comments.



Comment at: lld/COFF/LTO.cpp:183
+  [&](size_t task, const Twine &moduleName) {
+buf[task].first = moduleName.str();
 return std::make_unique(

int3 wrote:
> Any reason why this doesn't instead store the module name in the 
> `file_names[task]` vector that the cache callback uses? Are the task IDs not 
> unique across each run of `ltoObj`, regardless of whether we end up using the 
> cache or not?
Users have to explicitly uses the flag `/opt:lldltocache=path` to let lld to 
search for cache files at the given folder. If it's not given, the callback of 
localCache won't be invoked. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217

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

[Lldb-commits] [lldb] a3252d1 - [lldb][NFC] Move various constructor definitions from .h to .cpp

2023-03-30 Thread River Riddle via lldb-commits


Author: River Riddle
Date: 2023-03-30T13:25:30-07:00
New Revision: a3252d1a2c568792974a4bc7413b0c61a43053e9

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

LOG: [lldb][NFC] Move various constructor definitions from .h to .cpp

I ran into issues with linking downstream language plugin to liblldb in
debug builds, hitting link time errors of form:

```
undefined reference to `vtable for  lldb_private::'
```

Anchoring the vtable to the .cpp files resolved those issues.

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

Added: 


Modified: 
lldb/include/lldb/Expression/ExpressionVariable.h
lldb/include/lldb/Expression/Materializer.h
lldb/include/lldb/Symbol/TypeSystem.h
lldb/source/Expression/ExpressionVariable.cpp
lldb/source/Expression/Materializer.cpp
lldb/source/Symbol/TypeSystem.cpp

Removed: 




diff  --git a/lldb/include/lldb/Expression/ExpressionVariable.h 
b/lldb/include/lldb/Expression/ExpressionVariable.h
index 27343530780a1..ec18acb94417c 100644
--- a/lldb/include/lldb/Expression/ExpressionVariable.h
+++ b/lldb/include/lldb/Expression/ExpressionVariable.h
@@ -29,8 +29,7 @@ class ExpressionVariable
 
   LLVMCastKind getKind() const { return m_kind; }
 
-  ExpressionVariable(LLVMCastKind kind) : m_flags(0), m_kind(kind) {}
-
+  ExpressionVariable(LLVMCastKind kind);
   virtual ~ExpressionVariable();
 
   std::optional GetByteSize() { return m_frozen_sp->GetByteSize(); }
@@ -208,8 +207,7 @@ class PersistentExpressionState : public 
ExpressionVariableList {
 
   LLVMCastKind getKind() const { return m_kind; }
 
-  PersistentExpressionState(LLVMCastKind kind) : m_kind(kind) {}
-
+  PersistentExpressionState(LLVMCastKind kind);
   virtual ~PersistentExpressionState();
 
   virtual lldb::ExpressionVariableSP

diff  --git a/lldb/include/lldb/Expression/Materializer.h 
b/lldb/include/lldb/Expression/Materializer.h
index aae94f86a71e6..8f850c3f46439 100644
--- a/lldb/include/lldb/Expression/Materializer.h
+++ b/lldb/include/lldb/Expression/Materializer.h
@@ -69,6 +69,7 @@ class Materializer {
 
   class PersistentVariableDelegate {
   public:
+PersistentVariableDelegate();
 virtual ~PersistentVariableDelegate();
 virtual ConstString GetName() = 0;
 virtual void DidDematerialize(lldb::ExpressionVariableSP &variable) = 0;

diff  --git a/lldb/include/lldb/Symbol/TypeSystem.h 
b/lldb/include/lldb/Symbol/TypeSystem.h
index a16f4af2be6d6..dfef87232628b 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -77,6 +77,7 @@ class TypeSystem : public PluginInterface,
public std::enable_shared_from_this {
 public:
   // Constructors and Destructors
+  TypeSystem();
   ~TypeSystem() override;
 
   // LLVM RTTI support

diff  --git a/lldb/source/Expression/ExpressionVariable.cpp 
b/lldb/source/Expression/ExpressionVariable.cpp
index a397a34601d0c..325dd5bc8a2ad 100644
--- a/lldb/source/Expression/ExpressionVariable.cpp
+++ b/lldb/source/Expression/ExpressionVariable.cpp
@@ -15,6 +15,8 @@
 
 using namespace lldb_private;
 
+ExpressionVariable::ExpressionVariable(LLVMCastKind kind)
+: m_flags(0), m_kind(kind) {}
 ExpressionVariable::~ExpressionVariable() = default;
 
 uint8_t *ExpressionVariable::GetValueBytes() {
@@ -30,6 +32,8 @@ uint8_t *ExpressionVariable::GetValueBytes() {
   return nullptr;
 }
 
+PersistentExpressionState::PersistentExpressionState(LLVMCastKind kind)
+: m_kind(kind) {}
 PersistentExpressionState::~PersistentExpressionState() = default;
 
 lldb::addr_t PersistentExpressionState::LookupSymbol(ConstString name) {

diff  --git a/lldb/source/Expression/Materializer.cpp 
b/lldb/source/Expression/Materializer.cpp
index 0932dc6f95b1f..6e344dfd57c4b 100644
--- a/lldb/source/Expression/Materializer.cpp
+++ b/lldb/source/Expression/Materializer.cpp
@@ -1598,5 +1598,7 @@ void Materializer::Dematerializer::Wipe() {
   m_process_address = LLDB_INVALID_ADDRESS;
 }
 
+Materializer::PersistentVariableDelegate::PersistentVariableDelegate() =
+default;
 Materializer::PersistentVariableDelegate::~PersistentVariableDelegate() =
 default;

diff  --git a/lldb/source/Symbol/TypeSystem.cpp 
b/lldb/source/Symbol/TypeSystem.cpp
index 4eae2c98b12ec..9647102e96c61 100644
--- a/lldb/source/Symbol/TypeSystem.cpp
+++ b/lldb/source/Symbol/TypeSystem.cpp
@@ -36,6 +36,7 @@ size_t LanguageSet::Size() const { return bitvector.count(); }
 bool LanguageSet::Empty() const { return bitvector.none(); }
 bool LanguageSet::operator[](unsigned i) const { return bitvector[i]; }
 
+TypeSystem::TypeSystem() = default;
 TypeSystem::~TypeSystem() = default;
 
 static TypeSystemSP CreateInstanceHelper(lldb::LanguageType language,



___
lldb-commit

[Lldb-commits] [PATCH] D147252: [lldb][NFC] Move various constructor definitions from .h to .cpp

2023-03-30 Thread River Riddle via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3252d1a2c56: [lldb][NFC] Move various constructor 
definitions from .h to .cpp (authored by rriddle).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147252

Files:
  lldb/include/lldb/Expression/ExpressionVariable.h
  lldb/include/lldb/Expression/Materializer.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Expression/ExpressionVariable.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Symbol/TypeSystem.cpp


Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -36,6 +36,7 @@
 bool LanguageSet::Empty() const { return bitvector.none(); }
 bool LanguageSet::operator[](unsigned i) const { return bitvector[i]; }
 
+TypeSystem::TypeSystem() = default;
 TypeSystem::~TypeSystem() = default;
 
 static TypeSystemSP CreateInstanceHelper(lldb::LanguageType language,
Index: lldb/source/Expression/Materializer.cpp
===
--- lldb/source/Expression/Materializer.cpp
+++ lldb/source/Expression/Materializer.cpp
@@ -1598,5 +1598,7 @@
   m_process_address = LLDB_INVALID_ADDRESS;
 }
 
+Materializer::PersistentVariableDelegate::PersistentVariableDelegate() =
+default;
 Materializer::PersistentVariableDelegate::~PersistentVariableDelegate() =
 default;
Index: lldb/source/Expression/ExpressionVariable.cpp
===
--- lldb/source/Expression/ExpressionVariable.cpp
+++ lldb/source/Expression/ExpressionVariable.cpp
@@ -15,6 +15,8 @@
 
 using namespace lldb_private;
 
+ExpressionVariable::ExpressionVariable(LLVMCastKind kind)
+: m_flags(0), m_kind(kind) {}
 ExpressionVariable::~ExpressionVariable() = default;
 
 uint8_t *ExpressionVariable::GetValueBytes() {
@@ -30,6 +32,8 @@
   return nullptr;
 }
 
+PersistentExpressionState::PersistentExpressionState(LLVMCastKind kind)
+: m_kind(kind) {}
 PersistentExpressionState::~PersistentExpressionState() = default;
 
 lldb::addr_t PersistentExpressionState::LookupSymbol(ConstString name) {
Index: lldb/include/lldb/Symbol/TypeSystem.h
===
--- lldb/include/lldb/Symbol/TypeSystem.h
+++ lldb/include/lldb/Symbol/TypeSystem.h
@@ -77,6 +77,7 @@
public std::enable_shared_from_this {
 public:
   // Constructors and Destructors
+  TypeSystem();
   ~TypeSystem() override;
 
   // LLVM RTTI support
Index: lldb/include/lldb/Expression/Materializer.h
===
--- lldb/include/lldb/Expression/Materializer.h
+++ lldb/include/lldb/Expression/Materializer.h
@@ -69,6 +69,7 @@
 
   class PersistentVariableDelegate {
   public:
+PersistentVariableDelegate();
 virtual ~PersistentVariableDelegate();
 virtual ConstString GetName() = 0;
 virtual void DidDematerialize(lldb::ExpressionVariableSP &variable) = 0;
Index: lldb/include/lldb/Expression/ExpressionVariable.h
===
--- lldb/include/lldb/Expression/ExpressionVariable.h
+++ lldb/include/lldb/Expression/ExpressionVariable.h
@@ -29,8 +29,7 @@
 
   LLVMCastKind getKind() const { return m_kind; }
 
-  ExpressionVariable(LLVMCastKind kind) : m_flags(0), m_kind(kind) {}
-
+  ExpressionVariable(LLVMCastKind kind);
   virtual ~ExpressionVariable();
 
   std::optional GetByteSize() { return m_frozen_sp->GetByteSize(); }
@@ -208,8 +207,7 @@
 
   LLVMCastKind getKind() const { return m_kind; }
 
-  PersistentExpressionState(LLVMCastKind kind) : m_kind(kind) {}
-
+  PersistentExpressionState(LLVMCastKind kind);
   virtual ~PersistentExpressionState();
 
   virtual lldb::ExpressionVariableSP


Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -36,6 +36,7 @@
 bool LanguageSet::Empty() const { return bitvector.none(); }
 bool LanguageSet::operator[](unsigned i) const { return bitvector[i]; }
 
+TypeSystem::TypeSystem() = default;
 TypeSystem::~TypeSystem() = default;
 
 static TypeSystemSP CreateInstanceHelper(lldb::LanguageType language,
Index: lldb/source/Expression/Materializer.cpp
===
--- lldb/source/Expression/Materializer.cpp
+++ lldb/source/Expression/Materializer.cpp
@@ -1598,5 +1598,7 @@
   m_process_address = LLDB_INVALID_ADDRESS;
 }
 
+Materializer::PersistentVariableDelegate::PersistentVariableDelegate() =
+default;
 Materializer::PersistentVariableDelegate::~PersistentVariableDelegate() =
 default;
Index: lldb/source/Expression/ExpressionVariable.cpp

[Lldb-commits] [lldb] c8522ca - [lldb] Fix macos build after e64cc756819d

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


Author: Jonas Devlieghere
Date: 2023-03-30T13:54:24-07:00
New Revision: c8522cadf7331bedd548ad5e2c6ef100c9166259

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

LOG: [lldb] Fix macos build after e64cc756819d

Added: 


Modified: 
lldb/tools/lldb-server/lldb-gdbserver.cpp

Removed: 




diff  --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp 
b/lldb/tools/lldb-server/lldb-gdbserver.cpp
index f6e52aac4f65c..5d32ea193fb54 100644
--- a/lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ b/lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -34,6 +34,7 @@
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Errno.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/WithColor.h"
 
 #if defined(__linux__)
@@ -75,13 +76,11 @@ class NativeProcessManager : public 
NativeProcessProtocol::Manager {
 public:
   llvm::Expected>
   Launch(ProcessLaunchInfo &launch_info,
- NativeProcessProtocol::NativeDelegate &delegate,
- MainLoop &mainloop) const override {
+ NativeDelegate &native_delegate) override {
 llvm_unreachable("Not implemented");
   }
   llvm::Expected>
-  Attach(lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &delegate,
- MainLoop &mainloop) const override {
+  Attach(lldb::pid_t pid, NativeDelegate &native_delegate) override {
 llvm_unreachable("Not implemented");
   }
 };



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

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

2023-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147248

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

[Lldb-commits] [lldb] 7edff3c - [lldb] Use one Progress event per root module build

2023-03-30 Thread Dave Lee via lldb-commits


Author: Dave Lee
Date: 2023-03-30T13:59:02-07:00
New Revision: 7edff3c1b298f696c632625fa863acbc7d68d446

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

LOG: [lldb] Use one Progress event per root module build

Following the work done by @jdevlieghere in D143690, this changes how Clang 
module build
events are emitted.

Instead of one Progress event per module being built, a single Progress event 
is used to
encompass all modules, and each module build is sent as an `Increment` update.

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

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
index 2b98fc83097a..98c1b1a73b78 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -68,7 +68,7 @@ class StoringDiagnosticConsumer : public 
clang::DiagnosticConsumer {
 
 private:
   bool HandleModuleRemark(const clang::Diagnostic &info);
-  void SetCurrentModuleProgress(llvm::StringRef module_name);
+  void SetCurrentModuleProgress(std::string module_name);
 
   typedef std::pair
   IDAndDiagnostic;
@@ -208,8 +208,9 @@ bool StoringDiagnosticConsumer::HandleModuleRemark(
 if (m_module_build_stack.empty()) {
   m_current_progress_up = nullptr;
 } else {
-  // Update the progress to re-show the module that was currently being
-  // built from the time the now completed module was originally began.
+  // When the just completed module began building, a module that depends 
on
+  // it ("module A") was effectively paused. Update the progress to re-show
+  // "module A" as continuing to be built.
   const auto &resumed_module_name = m_module_build_stack.back();
   SetCurrentModuleProgress(resumed_module_name);
 }
@@ -224,13 +225,12 @@ bool StoringDiagnosticConsumer::HandleModuleRemark(
 }
 
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-llvm::StringRef module_name) {
-  // Ensure the ordering of:
-  //   1. Completing the existing progress event.
-  //   2. Beginining a new progress event.
-  m_current_progress_up = nullptr;
-  m_current_progress_up = std::make_unique(
-  llvm::formatv("Currently building module {0}", module_name));
+std::string module_name) {
+  if (!m_current_progress_up)
+m_current_progress_up =
+std::make_unique("Building Clang modules");
+
+  m_current_progress_up->Increment(1, std::move(module_name));
 }
 
 ClangModulesDeclVendor::ClangModulesDeclVendor()

diff  --git 
a/lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
 
b/lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
index af0b59cd555f..228f676aedf6 100644
--- 
a/lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
+++ 
b/lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
@@ -43,4 +43,4 @@ def test_clang_module_build_progress_report(self):
 event = lldbutil.fetch_next_event(self, listener, broadcaster)
 payload = lldb.SBDebugger.GetProgressFromEvent(event)
 message = payload[0]
-self.assertEqual(message, "Currently building module MyModule")
+self.assertEqual(message, "Building Clang modules")



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

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

2023-03-30 Thread Dave Lee via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7edff3c1b298: [lldb] Use one Progress event per root module 
build (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147248

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py


Index: 
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
===
--- 
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
+++ 
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
@@ -43,4 +43,4 @@
 event = lldbutil.fetch_next_event(self, listener, broadcaster)
 payload = lldb.SBDebugger.GetProgressFromEvent(event)
 message = payload[0]
-self.assertEqual(message, "Currently building module MyModule")
+self.assertEqual(message, "Building Clang modules")
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -68,7 +68,7 @@
 
 private:
   bool HandleModuleRemark(const clang::Diagnostic &info);
-  void SetCurrentModuleProgress(llvm::StringRef module_name);
+  void SetCurrentModuleProgress(std::string module_name);
 
   typedef std::pair
   IDAndDiagnostic;
@@ -208,8 +208,9 @@
 if (m_module_build_stack.empty()) {
   m_current_progress_up = nullptr;
 } else {
-  // Update the progress to re-show the module that was currently being
-  // built from the time the now completed module was originally began.
+  // When the just completed module began building, a module that depends 
on
+  // it ("module A") was effectively paused. Update the progress to re-show
+  // "module A" as continuing to be built.
   const auto &resumed_module_name = m_module_build_stack.back();
   SetCurrentModuleProgress(resumed_module_name);
 }
@@ -224,13 +225,12 @@
 }
 
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-llvm::StringRef module_name) {
-  // Ensure the ordering of:
-  //   1. Completing the existing progress event.
-  //   2. Beginining a new progress event.
-  m_current_progress_up = nullptr;
-  m_current_progress_up = std::make_unique(
-  llvm::formatv("Currently building module {0}", module_name));
+std::string module_name) {
+  if (!m_current_progress_up)
+m_current_progress_up =
+std::make_unique("Building Clang modules");
+
+  m_current_progress_up->Increment(1, std::move(module_name));
 }
 
 ClangModulesDeclVendor::ClangModulesDeclVendor()


Index: lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
===
--- lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
+++ lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
@@ -43,4 +43,4 @@
 event = lldbutil.fetch_next_event(self, listener, broadcaster)
 payload = lldb.SBDebugger.GetProgressFromEvent(event)
 message = payload[0]
-self.assertEqual(message, "Currently building module MyModule")
+self.assertEqual(message, "Building Clang modules")
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -68,7 +68,7 @@
 
 private:
   bool HandleModuleRemark(const clang::Diagnostic &info);
-  void SetCurrentModuleProgress(llvm::StringRef module_name);
+  void SetCurrentModuleProgress(std::string module_name);
 
   typedef std::pair
   IDAndDiagnostic;
@@ -208,8 +208,9 @@
 if (m_module_build_stack.empty()) {
   m_current_progress_up = nullptr;
 } else {
-  // Update the progress to re-show the module that was currently being
-  // built from the time the now completed module was originally began.
+  // When the just completed module began building, a module that depends on
+  // it ("module A") was effectively paused. Update the progress to re-show
+  // "module A" as continuing to be built.
   const auto &resumed_module_name = m_module_build_stack.back();
   SetCurrentModuleProgress(resumed_module_name);
 }
@@ -224,13 +225,12 @@
 }
 
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-llvm::StringRef module_name) {
-  // E

[Lldb-commits] [lldb] f03f811 - [lldb] Only run TestUniversal64 on macOS 11 and later

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


Author: Jonas Devlieghere
Date: 2023-03-30T14:12:32-07:00
New Revision: f03f8111d2af41561c2be918f56ac348bf228dc7

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

LOG: [lldb] Only run TestUniversal64 on macOS 11 and later

GreenDragon is running on a host OS and toolchain that doesn't support
building for Apple Silicon.

Added: 


Modified: 
lldb/test/API/macosx/universal64/TestUniversal64.py

Removed: 




diff  --git a/lldb/test/API/macosx/universal64/TestUniversal64.py 
b/lldb/test/API/macosx/universal64/TestUniversal64.py
index 0c4226fac538..d97ebc05176d 100644
--- a/lldb/test/API/macosx/universal64/TestUniversal64.py
+++ b/lldb/test/API/macosx/universal64/TestUniversal64.py
@@ -25,6 +25,7 @@ def do_test(self):
 
 @skipUnlessDarwin
 @skipIfDarwinEmbedded
+@skipIf(macos_version=["<", "11.0"])
 def test_universal64_executable(self):
 """Test fat64 universal executable"""
 self.build(debug_info="dsym")
@@ -32,7 +33,7 @@ def test_universal64_executable(self):
 
 @skipUnlessDarwin
 @skipIfDarwinEmbedded
-@skipIf(compiler="clang", compiler_version=['<', '7.0'])
+@skipIf(macos_version=["<", "11.0"])
 def test_universal64_dsym(self):
 """Test fat64 universal dSYM"""
 self.build(debug_info="dsym", dictionary={'FAT64_DSYM': '1'})



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

[Lldb-commits] [PATCH] D146977: [lldb-server/linux] Use waitpid(-1) to collect inferior events

2023-03-30 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve added a comment.

Jonas fixed the build failures in c8522cadf7331bedd548ad5e2c6ef100c9166259 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146977

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

[Lldb-commits] [lldb] b24e290 - [lldb] Fix macos build after e64cc756819d (2/2)

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


Author: Jonas Devlieghere
Date: 2023-03-30T14:50:45-07:00
New Revision: b24e2900fa743e0abab7f4fa9747e5cbbfc2567a

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

LOG: [lldb] Fix macos build after e64cc756819d (2/2)

My previous commit was still missing the ctor and the NativeDelegate
parent class.

Added: 


Modified: 
lldb/tools/lldb-server/lldb-gdbserver.cpp

Removed: 




diff  --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp 
b/lldb/tools/lldb-server/lldb-gdbserver.cpp
index 5d32ea193fb5..8d0346c1c0c4 100644
--- a/lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ b/lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -74,13 +74,17 @@ typedef NativeProcessWindows::Manager NativeProcessManager;
 // Dummy implementation to make sure the code compiles
 class NativeProcessManager : public NativeProcessProtocol::Manager {
 public:
+  NativeProcessManager(MainLoop &mainloop)
+  : NativeProcessProtocol::Manager(mainloop) {}
+
   llvm::Expected>
   Launch(ProcessLaunchInfo &launch_info,
- NativeDelegate &native_delegate) override {
+ NativeProcessProtocol::NativeDelegate &native_delegate) override {
 llvm_unreachable("Not implemented");
   }
   llvm::Expected>
-  Attach(lldb::pid_t pid, NativeDelegate &native_delegate) override {
+  Attach(lldb::pid_t pid,
+ NativeProcessProtocol::NativeDelegate &native_delegate) override {
 llvm_unreachable("Not implemented");
   }
 };



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

[Lldb-commits] [PATCH] D147292: [lldb] Add support for the DW_AT_trampoline attribute with a boolean

2023-03-30 Thread Augusto Noronha via Phabricator via lldb-commits

augusto2112 created this revision.
augusto2112 added reviewers: aprantl, jingham, dblaikie.
Herald added a reviewer: shafik.
Herald added a project: All.
augusto2112 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch adds support for the DW_AT_trampoline attribute whose value
is a boolean. Which is a "generic trampoline". Stepping into a generic
trampoline by default will step through the function, checking
at every branch, until we stop in a function which makes sense to stop
at (a function with debug info, which isn't a trampoline, for example).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147292

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanStepOverRange.h
  lldb/include/lldb/Target/ThreadPlanStepRange.h
  lldb/include/lldb/Target/ThreadPlanStepThroughGenericTrampoline.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Symbol/Function.cpp
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadPlanShouldStopHere.cpp
  lldb/source/Target/ThreadPlanStepInRange.cpp
  lldb/source/Target/ThreadPlanStepOverRange.cpp
  lldb/source/Target/ThreadPlanStepRange.cpp
  lldb/source/Target/ThreadPlanStepThrough.cpp
  lldb/source/Target/ThreadPlanStepThroughGenericTrampoline.cpp
  lldb/test/API/lang/c/trampoline_stepping/Makefile
  lldb/test/API/lang/c/trampoline_stepping/TestTrampolineStepping.py
  lldb/test/API/lang/c/trampoline_stepping/main.c

Index: lldb/test/API/lang/c/trampoline_stepping/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/trampoline_stepping/main.c
@@ -0,0 +1,52 @@
+void foo(void) {}
+
+__attribute__((transparent_stepping))
+void bar(void) {
+  foo();
+}
+
+__attribute__((transparent_stepping))
+void baz(void) {
+  bar();
+}
+
+__attribute__((nodebug))
+void nodebug(void) {}
+
+__attribute__((transparent_stepping))
+void nodebug_then_trampoline(void) {
+  nodebug();
+  baz();
+}
+
+__attribute__((transparent_stepping))
+void doesnt_call_trampoline(void) {}
+
+void direct_trampoline_call(void) {
+  bar(); // Break here for direct 
+  bar();
+}
+
+void chained_trampoline_call(void) {
+  baz(); // Break here for chained
+  baz();
+}
+
+void trampoline_after_nodebug(void) {
+  nodebug_then_trampoline(); // Break here for nodebug then trampoline
+  nodebug_then_trampoline();
+}
+
+void unused_target(void) {
+  doesnt_call_trampoline(); // Break here for unused
+}
+
+
+int main(void) {
+  direct_trampoline_call();
+  chained_trampoline_call();
+  trampoline_after_nodebug();
+  unused_target();
+  return 0;
+}
+
Index: lldb/test/API/lang/c/trampoline_stepping/TestTrampolineStepping.py
===
--- /dev/null
+++ lldb/test/API/lang/c/trampoline_stepping/TestTrampolineStepping.py
@@ -0,0 +1,104 @@
+"""Test that stepping in/out of trampolines works as expected.
+"""
+
+
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestTrampoline(TestBase):
+def setup(self, bkpt_str):
+self.build()
+
+_, _, thread, _ = lldbutil.run_to_source_breakpoint(
+self, bkpt_str, lldb.SBFileSpec('main.c'))
+return thread
+
+def test_direct_call(self):
+thread = self.setup('Break here for direct')
+
+# Sanity check that we start out in the correct function.
+name = thread.frames[0].GetFunctionName()
+self.assertIn('direct_trampoline_call', name)
+
+# Check that stepping in will take us directly to the trampoline target.
+thread.StepInto()
+name = thread.frames[0].GetFunctionName()
+self.assertIn('foo', name)
+
+# Check that stepping out takes us back to the trampoline caller.
+thread.StepOut()
+name = thread.frames[0].GetFunctionName()
+self.assertIn('direct_trampoline_call', name)
+
+# Check that stepping over the end of the trampoline target 
+# takes us back to the trampoline caller.
+thread.StepInto()
+thread.StepOver()
+name = thread.frames[0].GetFunctionName()
+self.assertIn('direct_trampoline_call', name)
+
+
+def test_chained_call(self):
+thread = self.setup('Break here for chained')
+
+# Sanity check that we start out in the correct function.
+name = thread.frames[0].GetFunctionName

[Lldb-commits] [PATCH] D147300: [lldb] Fix build on older FreeBSD

2023-03-30 Thread Brooks Davis via Phabricator via lldb-commits

brooks created this revision.
brooks added reviewers: DavidSpickett, emaste, arichardson, dim.
Herald added a subscriber: krytarowski.
Herald added a project: All.
brooks requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Commit 392d9eb03af5a1adac66a86939351b22b3e73495 
 added a 
dependency on
FPE_FLTIDO which was only defined in FreeBSD main on May 19, 2022 and it
not in all releases.  Just define it if it's missing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147300

Files:
  lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp


Index: lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp
===
--- lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp
+++ lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp
@@ -11,6 +11,10 @@
 #ifdef __FreeBSD__
 #include 
 
+#ifndef FPE_FLTIDO
+#define FPE_FLTIDO 9
+#endif
+
 #define ADD_SIGCODE(signal_name, signal_value, code_name, code_value, ...) 
\
   static_assert(signal_name == signal_value,   
\
 "Value mismatch for signal number " #signal_name); 
\


Index: lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp
===
--- lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp
+++ lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp
@@ -11,6 +11,10 @@
 #ifdef __FreeBSD__
 #include 
 
+#ifndef FPE_FLTIDO
+#define FPE_FLTIDO 9
+#endif
+
 #define ADD_SIGCODE(signal_name, signal_value, code_name, code_value, ...) \
   static_assert(signal_name == signal_value,   \
 "Value mismatch for signal number " #signal_name); \
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

41 matches

Mail list logo