[Lldb-commits] [PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

[Kadir Cetinkaya via Phabricator via lldb-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470
 const char *const *Prefixes[DriverID::LastOption] = {nullptr};
-#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE;
+#define PREFIX(NAME, VALUE) static constexpr llvm::StringLiteral NAME[] = 
VALUE;
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  
\

this is actually an incorrect change (even builds shouldn't be succeeding). as 
the values here can also be `nullptr` (which cannot be stored in a 
StringLiteral) but moreover we later on assign these to `Prefixes` array, which 
is of type `char*`, hence the conversion should also be failing.

but in general i'd actually expect people to be assigning "nullptr"s to these 
`char*`s, hence if this was a purely mechanical migration without some extra 
constraints, it might still blow up at runtime even if it succeeds compiles.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139881

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

[Lldb-commits] [PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

[serge via Phabricator via lldb-commits]

serge-sans-paille added inline comments.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470
 const char *const *Prefixes[DriverID::LastOption] = {nullptr};
-#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE;
+#define PREFIX(NAME, VALUE) static constexpr llvm::StringLiteral NAME[] = 
VALUE;
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  
\

kadircet wrote:
> this is actually an incorrect change (even builds shouldn't be succeeding). 
> as the values here can also be `nullptr` (which cannot be stored in a 
> StringLiteral) but moreover we later on assign these to `Prefixes` array, 
> which is of type `char*`, hence the conversion should also be failing.
> 
> but in general i'd actually expect people to be assigning "nullptr"s to these 
> `char*`s, hence if this was a purely mechanical migration without some extra 
> constraints, it might still blow up at runtime even if it succeeds compiles.
Builds (and test suite) all succeed (at least locally :-)). This patch also 
modifies tablegen to *not* generate `nullptr`, but empty string instead. The 
constructor of `OptTable::Info` has been modified to turn these "Empty string 
terminated array" into regular `ArrayRef`, which makes iteration code much more 
straight forward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139881

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

[Lldb-commits] [PATCH] D139252: [lldb/Plugins] Introduce Scripted Platform Plugin

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 482413.
mib marked 7 inline comments as done.
mib added a comment.

Address @JDevlieghere comments.


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

https://reviews.llvm.org/D139252

Files:
  lldb/source/Plugins/Platform/CMakeLists.txt
  lldb/source/Plugins/Platform/scripted/CMakeLists.txt
  lldb/source/Plugins/Platform/scripted/ScriptedPlatform.cpp
  lldb/source/Plugins/Platform/scripted/ScriptedPlatform.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -23,8 +23,8 @@
 using namespace lldb_private;
 
 void ScriptedThread::CheckInterpreterAndScriptObject() const {
-  lldbassert(m_script_object_sp && "Invalid Script Object.");
-  lldbassert(GetInterface() && "Invalid Scripted Thread Interface.");
+  assert(m_script_object_sp && "Invalid Script Object.");
+  assert(GetInterface() && "Invalid Scripted Thread Interface.");
 }
 
 llvm::Expected>
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -90,14 +90,14 @@
   friend class ScriptedThread;
 
   void CheckInterpreterAndScriptObject() const;
+
   ScriptedProcessInterface &GetInterface() const;
+
   static bool IsScriptLanguageSupported(lldb::ScriptLanguage language);
 
-  // Member variables.
   const ScriptedMetadata m_scripted_metadata;
   lldb_private::ScriptInterpreter *m_interpreter = nullptr;
   lldb_private::StructuredData::ObjectSP m_script_object_sp = nullptr;
-  //@}
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/Platform/scripted/ScriptedPlatform.h
===
--- /dev/null
+++ lldb/source/Plugins/Platform/scripted/ScriptedPlatform.h
@@ -0,0 +1,83 @@
+//===-- ScriptedPlatform.h --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_PLUGINS_SCRIPTED_PLATFORM_H
+#define LLDB_SOURCE_PLUGINS_SCRIPTED_PLATFORM_H
+
+#include "lldb/Interpreter/ScriptedMetadata.h"
+#include "lldb/Target/Platform.h"
+
+namespace lldb_private {
+
+class ScriptedPlatform : public Platform {
+public:
+  ScriptedPlatform(Debugger *debugger,
+   const ScriptedMetadata *scripted_metadata, Status &error);
+
+  ~ScriptedPlatform() override;
+
+  static lldb::PlatformSP CreateInstance(bool force, const ArchSpec *arch,
+ const Debugger *debugger,
+ const ScriptedMetadata *metadata);
+
+  static void Initialize();
+
+  static void Terminate();
+
+  static llvm::StringRef GetPluginNameStatic() { return "scripted-platform"; }
+
+  static llvm::StringRef GetDescriptionStatic() {
+return "Scripted Platform plug-in.";
+  }
+
+  llvm::StringRef GetDescription() override { return GetDescriptionStatic(); }
+
+  llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
+
+  std::vector
+  GetSupportedArchitectures(const ArchSpec &process_host_arch) override;
+
+  bool IsConnected() const override { return true; }
+
+  lldb::ProcessSP Attach(lldb_private::ProcessAttachInfo &attach_info,
+ lldb_private::Debugger &debugger,
+ lldb_private::Target *target,
+ lldb_private::Status &error) override;
+
+  uint32_t FindProcesses(const ProcessInstanceInfoMatch &match_info,
+ ProcessInstanceInfoList &proc_infos) override;
+
+  bool GetProcessInfo(lldb::pid_t pid, ProcessInstanceInfo &proc_info) override;
+
+  Status LaunchProcess(ProcessLaunchInfo &launch_info) override;
+
+  Status KillProcess(const lldb::pid_t pid) override;
+
+  void CalculateTrapHandlerSymbolNames() override {}
+
+private:
+  ScriptedPlatform(const ScriptedPlatform &) = delete;
+  const ScriptedPlatform &operator=(const ScriptedPlatform &) = delete;
+
+  void CheckInterpreterAndScriptObject() const;
+
+  ScriptedPlatformInterface &GetInterface() const;
+
+  llvm::Expected
+  ParseProcessInfo(StructuredData::Dictionary &dict, lldb::pid_t pid) const;
+
+  static bool IsScriptLanguageSupported(lldb::ScriptLanguage language);
+
+  const ScriptedMetadata *m_scripted_metadata = nullptr;
+  lldb_private::ScriptInterpreter *m_interpreter = nullptr;
+  lldb_private::StructuredData::ObjectS

[Lldb-commits] [PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

[Kadir Cetinkaya via Phabricator via lldb-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470
 const char *const *Prefixes[DriverID::LastOption] = {nullptr};
-#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE;
+#define PREFIX(NAME, VALUE) static constexpr llvm::StringLiteral NAME[] = 
VALUE;
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  
\

serge-sans-paille wrote:
> kadircet wrote:
> > this is actually an incorrect change (even builds shouldn't be succeeding). 
> > as the values here can also be `nullptr` (which cannot be stored in a 
> > StringLiteral) but moreover we later on assign these to `Prefixes` array, 
> > which is of type `char*`, hence the conversion should also be failing.
> > 
> > but in general i'd actually expect people to be assigning "nullptr"s to 
> > these `char*`s, hence if this was a purely mechanical migration without 
> > some extra constraints, it might still blow up at runtime even if it 
> > succeeds compiles.
> Builds (and test suite) all succeed (at least locally :-)). This patch also 
> modifies tablegen to *not* generate `nullptr`, but empty string instead. The 
> constructor of `OptTable::Info` has been modified to turn these "Empty string 
> terminated array" into regular `ArrayRef`, which makes iteration code much 
> more straight forward.
> This patch also modifies tablegen to *not* generate nullptr

Oh i see, i definitely missed that one. It might be better to somehow separate 
that piece out (or at least mention somewhere in the patch description).

But nevertheless, as mentioned these values get assigned into `Prefixes` array 
mentioned above, which is a `char *`. hence the conversion **must** fail. are 
you sure you have `clang-tools-extra` in your enabled projects?

this also checks for a `nullptr` below (which I marked in a separate comment).



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:502
   for (unsigned A = ID; A != DriverID::OPT_INVALID; A = NextAlias[A]) {
 if (Prefixes[A] == nullptr) // option groups.
   continue;

this place for example is still checking for `nullptr`s as termination.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:511
 // Iterate over each spelling of the alias, e.g. -foo vs --foo.
 for (auto *Prefix = Prefixes[A]; *Prefix != nullptr; ++Prefix) {
   llvm::SmallString<64> Buf(*Prefix);

also this place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139881

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

[Lldb-commits] [PATCH] D139484: [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 482433.
mib added a comment.

Update test program and test


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

https://reviews.llvm.org/D139484

Files:
  lldb/test/API/functionalities/scripted_process/Makefile
  lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/baz.c
  lldb/test/API/functionalities/scripted_process/baz.cpp
  lldb/test/API/functionalities/scripted_process/baz.h
  lldb/test/API/functionalities/scripted_process/main.cpp

Index: lldb/test/API/functionalities/scripted_process/main.cpp
===
--- lldb/test/API/functionalities/scripted_process/main.cpp
+++ lldb/test/API/functionalities/scripted_process/main.cpp
@@ -1,10 +1,9 @@
-#include 
-#include 
 #include 
 
-extern "C" {
-int baz(int);
-}
+#include "baz.h"
+
+std::condition_variable cv;
+std::mutex mutex;
 
 int bar(int i) {
   int j = i * i;
@@ -13,23 +12,25 @@
 
 int foo(int i) { return bar(i); }
 
+void compute_pow(int &n) {
+  std::unique_lock lock(mutex);
+  n = foo(n);
+  lock.unlock();
+  cv.notify_one(); // waiting thread is notified with n == 42 * 42, cv.wait
+   // returns
+  lock.lock();
+}
+
 void call_and_wait(int &n) {
-  std::cout << "waiting for computation!" << std::endl;
-  while (baz(n) != 42)
+  while (baz(n, mutex, cv) != 42) {
 ;
-  std::cout << "finished computation!" << std::endl;
+  }
 }
 
-void compute_pow(int &n) { n = foo(n); }
-
 int main() {
   int n = 42;
-  std::mutex mutex;
-  std::unique_lock lock(mutex);
-
   std::thread thread_1(call_and_wait, std::ref(n));
   std::thread thread_2(compute_pow, std::ref(n));
-  lock.unlock();
 
   thread_1.join();
   thread_2.join();
Index: lldb/test/API/functionalities/scripted_process/baz.h
===
--- lldb/test/API/functionalities/scripted_process/baz.h
+++ lldb/test/API/functionalities/scripted_process/baz.h
@@ -1,3 +1,6 @@
 #pragma once
 
-int baz(int j);
+#include 
+#include 
+
+int baz(int &j, std::mutex &mutex, std::condition_variable &cv);
Index: lldb/test/API/functionalities/scripted_process/baz.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_process/baz.cpp
@@ -0,0 +1,10 @@
+#include "baz.h"
+
+#include 
+
+int baz(int &j, std::mutex &mutex, std::condition_variable &cv) {
+  std::unique_lock lock(mutex);
+  cv.wait(lock, [&j] { return j == 42 * 42; });
+  int k = sqrt(j);
+  return k; // break here
+}
Index: lldb/test/API/functionalities/scripted_process/baz.c
===
--- lldb/test/API/functionalities/scripted_process/baz.c
+++ /dev/null
@@ -1,8 +0,0 @@
-#include "baz.h"
-
-#include 
-
-int baz(int j) {
-  int k = sqrt(j);
-  return k; // break here
-}
Index: lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
@@ -17,7 +17,7 @@
 def create_stack_skinny_corefile(self, file):
 self.build()
 target, process, thread, _ = lldbutil.run_to_source_breakpoint(self, "// break here",
-   lldb.SBFileSpec("baz.c"))
+   lldb.SBFileSpec("baz.cpp"))
 self.assertTrue(process.IsValid(), "Process is invalid.")
 # FIXME: Use SBAPI to save the process corefile.
 self.runCmd("process save-core -s stack  " + file)
@@ -109,9 +109,9 @@
 self.assertTrue(func, "Invalid function.")
 
 self.assertIn("baz", frame.GetFunctionName())
-self.assertEqual(frame.vars.GetSize(), 2)
-self.assertEqual(int(frame.vars.GetFirstValueByName('j').GetValue()), 42 * 42)
+self.assertGreater(frame.vars.GetSize(), 0)
 self.assertEqual(int(frame.vars.GetFirstValueByName('k').GetValue()), 42)
+self.assertEqual(int(frame.vars.GetFirstValueByName('j').Dereference().GetValue()), 42 * 42)
 
 corefile_dylib = self.get_module_with_name(corefile_target, 'libbaz.dylib')
 self.assertTrue(corefile_dylib, "Dynamic library libbaz.dylib not found.")
Index: lldb/test/API/functionalities/scripted_process/Makefile
===
--- lldb/test/API/functionalities/scripted_process/Makefile
+++ lldb/test/API/functionalities/scripted_process/Makefile
@@ -6,8 +6,8 @@
 
 all: libbaz.dylib a.out
 
-libbaz.dylib: baz.c
+libbaz.dylib: baz.cpp
 	$(MAKE) -f $(MAKEFILE_RULES) ARCH=$(ARCH) \
-		DYLIB_ONLY=YES DYLIB_NAME=baz DYLIB_C_SOURCES=baz.c
+		DYLIB_ONLY=YES DYLIB_NAME=baz DYLIB_C

[Lldb-commits] [PATCH] D139853: [lldb/Process] Populate queues in Scripted Process

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp:757
 
+  bool is_scripted_process = m_process->GetPluginName() == "ScriptedProcess";
   for (ThreadSP thread_sp : m_process->Threads()) {

mib wrote:
> JDevlieghere wrote:
> > Comparing the plugin name defeats the abstraction a plugin is meant to 
> > provide. While we have other instances of LLDB breaking these abstractions, 
> > I don't recall other places where we compare the plugin name. The way we 
> > normally deal with this is extend the plugins capability (by adding a 
> > method) and implementing it accordingly for all the plugins (or have a sane 
> > default).
> > 
> > Based on the description of the patch it's not clear to me why this is 
> > special for scripted processes. If we need to special case this I'd like to 
> > see a comment explaining why.
> I agree, but in this case, `Process::UpdateQueueListIfNeeded` calls the 
> System Runtime plugin `PopulateQueueList` method and since queues are only 
> supported on macOS, it does make sense in my opinion.
> 
> I think we definitely shouldn't add another system runtime for scripted 
> processes just to support that case, so adding a special case for scripted 
> processes seems to be the less intrusive way to implement it.
I'm with Jonas here. Even though these Queue concepts are a big mystery do me, 
I would like to understand why the scripted processes do can not go through the 
same code path as "regular" ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139853

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

[Lldb-commits] [PATCH] D139853: [lldb/Process] Populate queues in Scripted Process

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp:757
 
+  bool is_scripted_process = m_process->GetPluginName() == "ScriptedProcess";
   for (ThreadSP thread_sp : m_process->Threads()) {

labath wrote:
> mib wrote:
> > JDevlieghere wrote:
> > > Comparing the plugin name defeats the abstraction a plugin is meant to 
> > > provide. While we have other instances of LLDB breaking these 
> > > abstractions, I don't recall other places where we compare the plugin 
> > > name. The way we normally deal with this is extend the plugins capability 
> > > (by adding a method) and implementing it accordingly for all the plugins 
> > > (or have a sane default).
> > > 
> > > Based on the description of the patch it's not clear to me why this is 
> > > special for scripted processes. If we need to special case this I'd like 
> > > to see a comment explaining why.
> > I agree, but in this case, `Process::UpdateQueueListIfNeeded` calls the 
> > System Runtime plugin `PopulateQueueList` method and since queues are only 
> > supported on macOS, it does make sense in my opinion.
> > 
> > I think we definitely shouldn't add another system runtime for scripted 
> > processes just to support that case, so adding a special case for scripted 
> > processes seems to be the less intrusive way to implement it.
> I'm with Jonas here. Even though these Queue concepts are a big mystery do 
> me, I would like to understand why the scripted processes do can not go 
> through the same code path as "regular" ones.
The reason here why this is not implemented at the Scripted Process plugin 
level, is because `UpdateQueueListIfNeeded` is a base method of the `Process` 
base class that's not expected to be redefined in the process plugin.

I guess we could either make `UpdateQueueListIfNeeded` virtual and keep the 
base class implementation the default and override it in the Scripted Process 
plugin class or we can have a `virtual void Process::DoUpdateQueueList() {}` 
method that would be called in `Process::UpdateQueueListIfNeeded`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139853

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

[Lldb-commits] [PATCH] D139853: [lldb/Process] Populate queues in Scripted Process

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 482486.
mib marked an inline comment as done.
mib added a comment.

Make use of the process plugin model to avoid adding a special case on the 
system runtime plugin.


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

https://reviews.llvm.org/D139853

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -72,6 +72,8 @@
 
   lldb_private::StructuredData::DictionarySP GetMetadata() override;
 
+  void UpdateQueueListIfNeeded() override;
+
 protected:
   ScriptedProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
   const ScriptedMetadata &scripted_metadata, Status &error);
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -497,6 +497,17 @@
   return metadata_sp;
 }
 
+void ScriptedProcess::UpdateQueueListIfNeeded() {
+  CheckInterpreterAndScriptObject();
+  for (ThreadSP thread_sp : Threads()) {
+if (const char *queue_name = thread_sp->GetQueueName()) {
+  QueueSP queue_sp = std::make_shared(
+  shared_from_this(), LLDB_INVALID_QUEUE_ID, queue_name);
+  m_queue_list.AddQueue(queue_sp);
+}
+  }
+}
+
 ScriptedProcessInterface &ScriptedProcess::GetInterface() const {
   return m_interpreter->GetScriptedProcessInterface();
 }
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -2107,7 +2107,7 @@
 
   // Queue Queries
 
-  void UpdateQueueListIfNeeded();
+  virtual void UpdateQueueListIfNeeded();
 
   QueueList &GetQueueList() {
 UpdateQueueListIfNeeded();


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -72,6 +72,8 @@
 
   lldb_private::StructuredData::DictionarySP GetMetadata() override;
 
+  void UpdateQueueListIfNeeded() override;
+
 protected:
   ScriptedProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
   const ScriptedMetadata &scripted_metadata, Status &error);
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -497,6 +497,17 @@
   return metadata_sp;
 }
 
+void ScriptedProcess::UpdateQueueListIfNeeded() {
+  CheckInterpreterAndScriptObject();
+  for (ThreadSP thread_sp : Threads()) {
+if (const char *queue_name = thread_sp->GetQueueName()) {
+  QueueSP queue_sp = std::make_shared(
+  shared_from_this(), LLDB_INVALID_QUEUE_ID, queue_name);
+  m_queue_list.AddQueue(queue_sp);
+}
+  }
+}
+
 ScriptedProcessInterface &ScriptedProcess::GetInterface() const {
   return m_interpreter->GetScriptedProcessInterface();
 }
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -2107,7 +2107,7 @@
 
   // Queue Queries
 
-  void UpdateQueueListIfNeeded();
+  virtual void UpdateQueueListIfNeeded();
 
   QueueList &GetQueueList() {
 UpdateQueueListIfNeeded();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139853: [lldb/Process] Populate queues in Scripted Process

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

That definitely looks much better. As I understand it, the scripted thread 
interface is more high-level that our internal thread interface, and that means 
it is computing its queue by itself, instead of delegating the work to a 
separate plugin. That makes sense to me.


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

https://reviews.llvm.org/D139853

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

[Lldb-commits] [PATCH] D139853: [lldb/Process] Populate queues in Scripted Process

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D139853#3992058 , @labath wrote:

> That definitely looks much better. As I understand it, the scripted thread 
> interface is more high-level that our internal thread interface, and that 
> means it is computing its queue by itself, instead of delegating the work to 
> a separate plugin. That makes sense to me.

Correct, Scripted Thread will report "pre-computed" queues to lldb, so it won't 
need to rely on the system runtime to resolve them in lldb.


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

https://reviews.llvm.org/D139853

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

[Lldb-commits] [PATCH] D139853: [lldb/Process] Populate queues in Scripted Process

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 482495.

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

https://reviews.llvm.org/D139853

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -72,6 +72,8 @@
 
   lldb_private::StructuredData::DictionarySP GetMetadata() override;
 
+  void UpdateQueueListIfNeeded() override;
+
 protected:
   ScriptedProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
   const ScriptedMetadata &scripted_metadata, Status &error);
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Interpreter/ScriptedMetadata.h"
 #include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Target/Queue.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/State.h"
@@ -497,6 +498,17 @@
   return metadata_sp;
 }
 
+void ScriptedProcess::UpdateQueueListIfNeeded() {
+  CheckInterpreterAndScriptObject();
+  for (ThreadSP thread_sp : Threads()) {
+if (const char *queue_name = thread_sp->GetQueueName()) {
+  QueueSP queue_sp = std::make_shared(
+  m_process->shared_from_this(), thread_sp->GetQueueID(), queue_name);
+  m_queue_list.AddQueue(queue_sp);
+}
+  }
+}
+
 ScriptedProcessInterface &ScriptedProcess::GetInterface() const {
   return m_interpreter->GetScriptedProcessInterface();
 }
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -2107,7 +2107,7 @@
 
   // Queue Queries
 
-  void UpdateQueueListIfNeeded();
+  virtual void UpdateQueueListIfNeeded();
 
   QueueList &GetQueueList() {
 UpdateQueueListIfNeeded();


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -72,6 +72,8 @@
 
   lldb_private::StructuredData::DictionarySP GetMetadata() override;
 
+  void UpdateQueueListIfNeeded() override;
+
 protected:
   ScriptedProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
   const ScriptedMetadata &scripted_metadata, Status &error);
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Interpreter/ScriptedMetadata.h"
 #include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Target/Queue.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/State.h"
@@ -497,6 +498,17 @@
   return metadata_sp;
 }
 
+void ScriptedProcess::UpdateQueueListIfNeeded() {
+  CheckInterpreterAndScriptObject();
+  for (ThreadSP thread_sp : Threads()) {
+if (const char *queue_name = thread_sp->GetQueueName()) {
+  QueueSP queue_sp = std::make_shared(
+  m_process->shared_from_this(), thread_sp->GetQueueID(), queue_name);
+  m_queue_list.AddQueue(queue_sp);
+}
+  }
+}
+
 ScriptedProcessInterface &ScriptedProcess::GetInterface() const {
   return m_interpreter->GetScriptedProcessInterface();
 }
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -2107,7 +2107,7 @@
 
   // Queue Queries
 
-  void UpdateQueueListIfNeeded();
+  virtual void UpdateQueueListIfNeeded();
 
   QueueList &GetQueueList() {
 UpdateQueueListIfNeeded();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

[Balázs Benics via Phabricator via lldb-commits]

steakhal added a comment.

The `StaticAnalyzer` portion looks good to me AFAICT.




Comment at: clang/lib/StaticAnalyzer/Core/CallDescription.cpp:39
 ento::CallDescription::CallDescription(CallDescriptionFlags Flags,
-   ArrayRef QualifiedName,
+   ArrayRef QualifiedName,
MaybeCount RequiredArgs /*= None*/,

Maybe we could restrict it even more to `StringLiteral`. The same applies to 
the other ctor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139881

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

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D138724#3988715 , @Michael137 
wrote:

> In D138724#3988318 , @labath wrote:
>
>> I just found out that this test (the non-shared-library version) fails on 
>> linux if the executable is built with `-no-pie` (which is the default if the 
>> `CLANG_DEFAULT_PIE_ON_LINUX` option is not set, which happened to be the 
>> case for my build). I think the important fact here is that a PIE ELF 
>> executable gets its `e_type` field set to `ET_DYN`, which causes lldb to 
>> identify it as a shared library. However, it is not clear to me why would 
>> that matter in this case...
>
> From what I can tell the main difference with `no-PIE` executable is that 
> `Target::ModulesDidUnload` doesn't get called when relaunching the process. 
> Whereas in the `PIE` case we do:
>
>   PlatformPOSIX::DebugProcess
>   -> Process::Launch
> -> DynamicLoaderPOSIXDYLD::DidLaunch
>   -> DynamicLoader::GetTargetExecutable
> -> Target::SetExecutableModule
>   -> Target::ClearModules
> -> Target::ModulesDidUnload

Ah I may have spotted the issue (but yet to confirm).

Currently, when relaunching an executable, the `DynamicLoader` plugin does this:

  ModuleSP DynamicLoader::GetTargetExecutable() {
  ...
  if (executable.get() != target.GetExecutableModulePointer()) {
// Don't load dependent images since we are in dyld where we will   
// know and find out about all images that are loaded
target.SetExecutableModule(executable, eLoadDependentsNo);  

In `target.GetExecutableModulePointer` we return the pointer the first module 
of type `eTypeObjectFile`, which with `no-pie` will not be `a.out`, so then we 
just return the first module in `m_images`. In the `no-pie` case we don't take 
this branch and maybe this is why. Will check shortly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [PATCH] D139945: [lldb] Add scripted process launch/attach option to platform commands

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: jingham, labath, JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch does several things:

First, it refactors the `CommandObject{,Platform}ProcessObject` command
option class into a separate `CommandOptionsProcessAttach` option group.

This will make sure both the `platform process attach` and `process attach`
command options will always stay in sync without having with duplicate
them each time. But more importantly, making this class an `OptionGroup`
allows us to combine with a `OptionGroupPythonClassWithDict` to add
support for the scripted process managing class name and user-provided
dictionary options.

This patch also improves feature parity between `ProcessLaunchInfo` and
`ProcessAttachInfo` with regard to ScriptedProcesses, by exposing the
various getters and setters necessary to use them through the SBAPI.

This is foundation work for adding support to "attach" to a process from
the scripted platform.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139945

Files:
  lldb/bindings/interface/SBAttachInfo.i
  lldb/include/lldb/API/SBAttachInfo.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Target/Process.h
  lldb/source/API/SBAttachInfo.cpp
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandOptionsProcessAttach.cpp
  lldb/source/Commands/CommandOptionsProcessAttach.h

Index: lldb/source/Commands/CommandOptionsProcessAttach.h
===
--- /dev/null
+++ lldb/source/Commands/CommandOptionsProcessAttach.h
@@ -0,0 +1,47 @@
+//===-- CommandOptionsProcessAttach.h ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_COMMANDS_COMMANDOPTIONSPROCESSATTACH_H
+#define LLDB_SOURCE_COMMANDS_COMMANDOPTIONSPROCESSATTACH_H
+
+#include "lldb/Interpreter/Options.h"
+#include "lldb/Target/Process.h"
+
+namespace lldb_private {
+
+// CommandOptionsProcessAttach
+
+class CommandOptionsProcessAttach : public lldb_private::OptionGroup {
+public:
+  CommandOptionsProcessAttach() {
+// Keep default values of all options in one place: OptionParsingStarting
+// ()
+OptionParsingStarting(nullptr);
+  }
+
+  ~CommandOptionsProcessAttach() override = default;
+
+  lldb_private::Status
+  SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+ lldb_private::ExecutionContext *execution_context) override;
+
+  void OptionParsingStarting(
+  lldb_private::ExecutionContext *execution_context) override {
+attach_info.Clear();
+  }
+
+  llvm::ArrayRef GetDefinitions() override;
+
+  // Instance variables to hold the values for command options.
+
+  lldb_private::ProcessAttachInfo attach_info;
+}; // CommandOptionsProcessAttach
+
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_COMMANDS_COMMANDOPTIONSPROCESSATTACH_H
Index: lldb/source/Commands/CommandOptionsProcessAttach.cpp
===
--- /dev/null
+++ lldb/source/Commands/CommandOptionsProcessAttach.cpp
@@ -0,0 +1,76 @@
+//===-- CommandOptionsProcessAttach.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "CommandOptionsProcessAttach.h"
+
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Host/OptionParser.h"
+#include "lldb/Interpreter/CommandCompletions.h"
+#include "lldb/Interpreter/CommandObject.h"
+#include "lldb/Interpreter/CommandOptionArgumentTable.h"
+#include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Target/Target.h"
+
+#include "llvm/ADT/ArrayRef.h"
+
+using namespace llvm;
+using namespace lldb;
+using namespace lldb_private;
+
+#define LLDB_OPTIONS_process_attach
+#include "CommandOptions.inc"
+
+Status CommandOptionsProcessAttach::SetOptionValue(
+uint32_t option_idx, llvm::StringRef option_arg,
+ExecutionContext *execution_context) {
+  Status error;
+  const int short_option = g_process_attach_options[option_idx].short_option;
+  switch (short_option) {
+  case 'c':
+attach_info.SetContinueOnceAttached(true);
+break;
+
+  case 'p': {
+   

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/examples/python/scripted_process/scripted_platform.py:31
+def list_processes(self):
+""" Get a list of processes that can be ran on the platform.
+

mib wrote:
> mib wrote:
> > labath wrote:
> > > I am surprised that you want to go down the "run" path for this 
> > > functionality. I think most of the launch functionality does not make 
> > > sense for this use case (e.g., you can't provide arguments to these 
> > > processes, when you "run" them, can you?), and it is not consistent with 
> > > what the "process listing" functionality does for regular platforms.
> > > 
> > > OTOH, the "attach" flow makes perfect sense here -- you take the pid of 
> > > an existing process, attach to it, and stop it at a random point in its 
> > > execution. You can't customize anything about how that process is run 
> > > (because it's already running) -- all you can do is choose how you want 
> > > to select the target process.
> > For now, there is no support for attaching to a scripted process, because 
> > we didn't have any use for it quite yet: cripted processes were mostly used 
> > for doing post-mortem debugging, so we "ran" them artificially in lldb by 
> > providing some launch options (the name of the class managing the process 
> > and an optional user-provided dictionary) through the command line or using 
> > an `SBLaunchInfo` object.
> > 
> > I guess I'll need to extend the `platform process launch/attach` commands 
> > and `SBAttachInfo` object to also support these options since they're 
> > required for the scripted process instantiation.
> > 
> > Note that we aren't really attaching to the real running process, we're 
> > creating a scripted process that knows how to read memory to mock the real 
> > process.
> @labath, I'll do that work on a follow-up patch
@labath here D139945 :) 


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

https://reviews.llvm.org/D139250

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

[Lldb-commits] [PATCH] D139951: [lldb/crashlog] Refactor CrashLogParser into a Factory patern

[Med Ismail Bennani via Phabricator via lldb-commits]

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

This patch should fix an undefined behaviour that's happening when
parsing a crash report from an IDE. In the previous implementation, the
CrashLogParser base class would use the `__new__` static class method to
create the right parser instance depending on the crash report type.

For some reasons, the derived parser initializer wouldn't be called when
running the command from an IDE, so this patch refactors the
CrashLogParser code to replace the use of the `__new__` method with a
factory method.

rdar://100527640

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139951

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py


Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -6,11 +6,11 @@
 from lldb.plugins.scripted_process import ScriptedProcess
 from lldb.plugins.scripted_process import ScriptedThread
 
-from lldb.macosx.crashlog import CrashLog,CrashLogParser
+from lldb.macosx.crashlog import CrashLog,CrashLogParserFactory
 
 class CrashLogScriptedProcess(ScriptedProcess):
 def parse_crashlog(self):
-crashlog_parser = CrashLogParser(self.dbg, self.crashlog_path, False)
+crashlog_parser = CrashLogParserFactory(self.dbg, self.crashlog_path, 
False)
 crash_log = crashlog_parser.parse()
 
 self.pid = crash_log.process_id
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -416,17 +416,16 @@
 class InteractiveCrashLogException(Exception):
 pass
 
-class CrashLogParser:
-"CrashLog parser base class and factory."
-def __new__(cls, debugger, path, verbose):
-data = JSONCrashLogParser.is_valid_json(path)
-if data:
-self = object.__new__(JSONCrashLogParser)
-self.data = data
-return self
-else:
-return object.__new__(TextCrashLogParser)
+def CrashLogParserFactory(debugger, path, verbose):
+data = JSONCrashLogParser.is_valid_json(path)
+if data:
+parser = JSONCrashLogParser(debugger, path, verbose)
+parser.data = data
+return parser
+else:
+return TextCrashLogParser(debugger, path, verbose)
 
+class CrashLogParser:
 def __init__(self, debugger, path, verbose):
 self.path = os.path.expanduser(path)
 self.verbose = verbose
@@ -1076,7 +1075,7 @@
 if not os.path.exists(crashlog_path):
 raise InteractiveCrashLogException("crashlog file %s does not exist" % 
crashlog_path)
 
-crashlog = CrashLogParser(debugger, crashlog_path, False).parse()
+crashlog = CrashLogParserFactory(debugger, crashlog_path, False).parse()
 
 target = lldb.SBTarget()
 # 1. Try to use the user-provided target
@@ -1332,7 +1331,7 @@
 except InteractiveCrashLogException as e:
 result.SetError(str(e))
 else:
-crash_log = CrashLogParser(debugger, crash_log_file, 
options.verbose).parse()
+crash_log = CrashLogParserFactory(debugger, crash_log_file, 
options.verbose).parse()
 SymbolicateCrashLog(crash_log, options)
 
 if __name__ == '__main__':


Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -6,11 +6,11 @@
 from lldb.plugins.scripted_process import ScriptedProcess
 from lldb.plugins.scripted_process import ScriptedThread
 
-from lldb.macosx.crashlog import CrashLog,CrashLogParser
+from lldb.macosx.crashlog import CrashLog,CrashLogParserFactory
 
 class CrashLogScriptedProcess(ScriptedProcess):
 def parse_crashlog(self):
-crashlog_parser = CrashLogParser(self.dbg, self.crashlog_path, False)
+crashlog_parser = CrashLogParserFactory(self.dbg, self.crashlog_path, False)
 crash_log = crashlog_parser.parse()
 
 self.pid = crash_log.process_id
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -416,17 +416,16 @@
 class InteractiveCrashLogException(Exception):
 pass
 
-class CrashLogParser:
-"CrashLog parser base class and factory."
-def __new__(cls, debugger, path,

[Lldb-commits] [PATCH] D139955: [LLDB] Change formatting to use llvm::formatv

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo created this revision.
Herald added subscribers: hoy, modimo, wenlei, arphaman.
Herald added a reviewer: shafik.
Herald added a project: All.
ayermolo requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.
Herald added a project: LLDB.

In preparation for eanbling 64bit support in LLDB switching to use llvm::formatv
instead of format MACROs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139955

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Utility/Status.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/DWARFCallFrameInfo.cpp
  lldb/source/Utility/Status.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s
@@ -2,7 +2,7 @@
 # RUN: %lldb %t -o "image lookup -v -s lookup_ranges" -o exit 2>%t.error | FileCheck %s
 # RUN: cat %t.error | FileCheck %s --check-prefix ERROR
 
-# ERROR: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x47) attribute, but range extraction failed (No debug_ranges section),
+# ERROR: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x0047) attribute, but range extraction failed (No debug_ranges section),
 # CHECK:  Function: id = {0x001c}, name = "ranges", range = [0x-0x0004)
 # CHECK:Blocks: id = {0x001c}, range = [0x-0x0004)
 
Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
@@ -22,7 +22,7 @@
 # RUN: cat %t.error | FileCheck --check-prefix=ERROR %s
 
 # RNGLISTX-LABEL: image lookup -v -s lookup_rnglists
-# ERROR: error: {{.*}} {0x003f}: DIE has DW_AT_ranges(DW_FORM_rnglistx 0x0) attribute, but range extraction failed (DW_FORM_rnglistx cannot be used without DW_AT_rnglists_base for CU at 0x), please file a bug and attach the file at the start of this error message
+# ERROR: error: {{.*}} {0x003f}: DIE has DW_AT_ranges(DW_FORM_rnglistx 0x) attribute, but range extraction failed (DW_FORM_rnglistx cannot be used without DW_AT_rnglists_base for CU at 0x), please file a bug and attach the file at the start of this error message
 
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj \
 # RUN:   --defsym RNGLISTX=0 --defsym RNGLISTBASE=0 %s > %t-rnglistbase
@@ -31,7 +31,7 @@
 # RUN: cat %t.error | FileCheck --check-prefix=ERRORBASE %s
 
 # RNGLISTBASE-LABEL: image lookup -v -s lookup_rnglists
-# ERRORBASE: error: {{.*}}-rnglistbase {0x0043}: DIE has DW_AT_ranges(DW_FORM_rnglistx 0x0) attribute, but range extraction failed (invalid range list table index 0; OffsetEntryCount is 0, DW_AT_rnglists_base is 24), please file a bug and attach the file at the start of this error message
+# ERRORBASE: error: {{.*}}-rnglistbase {0x0043}: DIE has DW_AT_ranges(DW_FORM_rnglistx 0x) attribute, but range extraction failed (invalid range list table index 0; OffsetEntryCount is 0, DW_AT_rnglists_base is 24), please file a bug and attach the file at the start of this error message
 
 .text
 rnglists:
Index: lldb/source/Utility/Status.cpp
===
--- lldb/source/Utility/Status.cpp
+++ lldb/source/Utility/Status.cpp
@@ -59,6 +59,11 @@
   va_end(args);
 }
 
+Status::Status(const std::string &errMsg) : m_string() {
+  SetErrorToGenericError();
+  m_string = errMsg;
+}
+
 const Status &Status::operator=(llvm::Error error) {
   if (!error) {
 Clear();
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -772,12 +772,16 @@
   // useful for compilers that move epilogue code into the body of a
   // function.)
   if (stack.empty()) {
-LLDB_LOGF(log,
-  "DWARFCallFrameInfo::%s(dwarf_offset: %" PRIx32
-  ", startaddr:

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo updated this revision to Diff 482550.
ayermolo added a comment.
Herald added a subscriber: Michael137.

Separated format patch, and oso patch. Although without OSO changes a lot of 
tests fail on mac.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

Files:
  lldb/include/lldb/Core/dwarf.h
  lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp

Index: lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
===
--- lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
+++ lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
@@ -45,6 +45,26 @@
   EncodeDecode(DIERef(200, DIERef::Section::DebugTypes, 0x11223344));
 }
 
+TEST(DWARFIndexCachingTest, DIERefEncodeDecodeMax) {
+  // Tests DIERef::Encode(...) and DIERef::Decode(...)
+  EncodeDecode(DIERef(llvm::None, DIERef::Section::DebugInfo,
+  DIERef::k_die_offset_mask));
+  EncodeDecode(DIERef(llvm::None, DIERef::Section::DebugTypes,
+  DIERef::k_die_offset_mask));
+  EncodeDecode(
+  DIERef(100, DIERef::Section::DebugInfo, DIERef::k_die_offset_mask));
+  EncodeDecode(
+  DIERef(200, DIERef::Section::DebugTypes, DIERef::k_die_offset_mask));
+  EncodeDecode(DIERef(DIERef::k_dwo_num_mask, DIERef::Section::DebugInfo,
+  DIERef::k_dwo_num_mask));
+  EncodeDecode(DIERef(DIERef::k_dwo_num_mask, DIERef::Section::DebugTypes,
+  DIERef::k_dwo_num_mask));
+  EncodeDecode(
+  DIERef(DIERef::k_dwo_num_mask, DIERef::Section::DebugInfo, 0x11223344));
+  EncodeDecode(
+  DIERef(DIERef::k_dwo_num_mask, DIERef::Section::DebugTypes, 0x11223344));
+}
+
 static void EncodeDecode(const NameToDIE &object, ByteOrder byte_order) {
   const uint8_t addr_size = 8;
   DataEncoder encoder(byte_order, addr_size);
Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -713,7 +713,7 @@
   //   Entries:
   // - AbbrCode:0x1
   //   Values:
-  // - Value:   0x01020304
+  // - Value:   0x0120304
   // - AbbrCode:0x0
   const char *dwo_yamldata = R"(
 --- !ELF
@@ -750,7 +750,7 @@
   auto dwo_module_sp = std::make_shared(dwo_file->moduleSpec());
   SymbolFileDWARFDwo dwo_symfile(
   skeleton_symfile, dwo_module_sp->GetObjectFile()->shared_from_this(),
-  0x01020304);
+  0x0120304);
   auto *dwo_dwarf_unit = dwo_symfile.DebugInfo().GetUnitAtIndex(0);
 
   testExpressionVendorExtensions(dwo_module_sp, *dwo_dwarf_unit);
Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
@@ -8,7 +8,7 @@
 # RUN:   -o exit | FileCheck %s
 
 # Failure was the block range 1..2 was not printed plus:
-# error: DW_AT_range-DW_FORM_sec_offset.s.tmp {0x003f}: DIE has DW_AT_ranges(0xc) attribute, but range extraction failed (missing or invalid range list table), please file a bug and attach the file at the start of this error message
+# error: DW_AT_range-DW_FORM_sec_offset.s.tmp {0x003f}: DIE has DW_AT_ranges(0xc) attribute, but range extraction failed (missing or invalid range list table), please file a bug and attach the file at the start of this error message
 
 # CHECK-LABEL: image lookup -v -s lookup_rnglists
 # CHECK:  Function: id = {0x0029}, name = "rnglists", range = [0x-0x0003)
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_SYMBOLFILEDWARFDWO_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_SYMBOLFILEDWARFDWO_H
 
+#include "DIERef.h"
 #include "SymbolFileDWARF.h"
 
 class SymbolFileDWARFDwo : public SymbolFileDWARF {
@@

[Lldb-commits] [PATCH] D139957: [LLDB] Change OSO to use DieRef

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo created this revision.
Herald added subscribers: hoy, modimo, wenlei.
Herald added a project: All.
ayermolo requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Following example of DWO changing it to use DIERef.

Depends on D138618 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139957

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
@@ -4,9 +4,9 @@
 # RUN:   -o exit | FileCheck %s
 
 # CHECK-LABEL: image lookup -v -s lookup_rnglists
-# CHECK:  Function: id = {0x4028}, name = "rnglists", range = [0x-0x0003)
-# CHECK:Blocks: id = {0x4028}, range = [0x-0x0003)
-# CHECK-NEXT:   id = {0x4037}, range = [0x0001-0x0002)
+# CHECK:  Function: id = {0x2028}, name = "rnglists", range = [0x-0x0003)
+# CHECK:Blocks: id = {0x2028}, range = [0x-0x0003)
+# CHECK-NEXT:   id = {0x2037}, range = [0x0001-0x0002)
 
 .text
 rnglists:
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_SYMBOLFILEDWARFDEBUGMAP_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_SYMBOLFILEDWARFDEBUGMAP_H
 
+#include "DIERef.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Utility/RangeMap.h"
 #include "llvm/Support/Chrono.h"
@@ -208,7 +209,9 @@
   lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
 
   static uint32_t GetOSOIndexFromUserID(lldb::user_id_t uid) {
-return (uint32_t)((uid >> 32ull) - 1ull);
+llvm::Optional OsoNum = DIERef(uid).oso_num();
+lldbassert(OsoNum && "Invalid OSO Index");
+return *OsoNum;
   }
 
   static SymbolFileDWARF *GetSymbolFileAsSymbolFileDWARF(SymbolFile *sym_file);
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "SymbolFileDWARFDebugMap.h"
+#include "DIERef.h"
 #include "DWARFCompileUnit.h"
 #include "DWARFDebugAranges.h"
 #include "DWARFDebugInfo.h"
@@ -210,7 +211,9 @@
 // Set the ID of the symbol file DWARF to the index of the OSO
 // shifted left by 32 bits to provide a unique prefix for any
 // UserID's that get created in the symbol file.
-oso_symfile->SetID(((uint64_t)m_cu_idx + 1ull) << 32ull);
+oso_symfile->SetID(DIERef(DIERef::IndexType::OSONum, m_cu_idx,
+  DIERef::Section(0), 0)
+   .get_id());
   }
   return symfile;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1395,8 +1395,11 @@
 }
 
 user_id_t SymbolFileDWARF::GetUID(DIERef ref) {
-  if (GetDebugMapSymfile())
-return GetID() | ref.die_offset();
+  if (GetDebugMapSymfile()) {
+DIERef die_ref(GetID());
+die_ref.set_die_offset(ref.die_offset());
+return die_ref.get_id();
+  }
 
   return DIERef(GetDwoNum(), ref.section(), ref.die_offset()).get_id();
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
@@ -27,18 +27,33 @@
 class DIERef {
 public:
   enum Section : uint8_t { DebugInfo, DebugTypes };
+  enum IndexType : uint8_t { DWONum, OSONum };
 
-  DIERef(llvm::Optional dwo_num, Section section,
+  DIERef(llvm::Optional dwo_oso_num, Section section,
  dw_offset_t die_offset)
-  : m_die_offset(die_offset), m_dwo_num(dwo_num.value_or(0)),
-m_dwo_num_valid(dwo_num ? true : false), m_section(section) {
-assert(this->dwo_num() == dwo_nu

[Lldb-commits] [lldb] f54497f - [lldb] Remove basestring references in examples (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2022-12-13T11:10:11-08:00
New Revision: f54497f114b9b5acaa8ebff296594b2cd7e437dd

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

LOG: [lldb] Remove basestring references in examples (NFC)

`basestring` is Python 2 only.

Added: 


Modified: 
lldb/examples/summaries/cocoa/CFArray.py
lldb/examples/summaries/cocoa/CFBag.py
lldb/examples/summaries/cocoa/CFBinaryHeap.py
lldb/examples/summaries/cocoa/CFDictionary.py
lldb/examples/summaries/cocoa/NSData.py
lldb/examples/summaries/cocoa/NSIndexSet.py
lldb/examples/summaries/cocoa/NSMachPort.py
lldb/examples/summaries/cocoa/NSSet.py

Removed: 




diff  --git a/lldb/examples/summaries/cocoa/CFArray.py 
b/lldb/examples/summaries/cocoa/CFArray.py
index baf1ca30ce0e6..e3916011bd8ac 100644
--- a/lldb/examples/summaries/cocoa/CFArray.py
+++ b/lldb/examples/summaries/cocoa/CFArray.py
@@ -13,11 +13,6 @@
 import lldb.formatters.metrics
 import lldb.formatters.Logger
 
-try:
-basestring
-except NameError:
-basestring = str
-
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
@@ -222,7 +217,7 @@ def CFArray_SummaryProvider(valobj, dict):
 logger >> "provider gave me " + str(summary)
 if summary is None:
 summary = ''
-elif isinstance(summary, basestring):
+elif isinstance(summary, str):
 pass
 else:
 # we format it like it were a CFString to make it look the same as

diff  --git a/lldb/examples/summaries/cocoa/CFBag.py 
b/lldb/examples/summaries/cocoa/CFBag.py
index 55c3fe6b68f52..6a75b0222abb1 100644
--- a/lldb/examples/summaries/cocoa/CFBag.py
+++ b/lldb/examples/summaries/cocoa/CFBag.py
@@ -13,11 +13,6 @@
 import lldb.formatters.metrics
 import lldb.formatters.Logger
 
-try:
-basestring
-except NameError:
-basestring = str
-
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
@@ -150,7 +145,7 @@ def CFBag_SummaryProvider(valobj, dict):
 #  the mask needs to be changed)
 if summary is None:
 summary = ''
-elif isinstance(summary, basestring):
+elif isinstance(summary, str):
 pass
 else:
 if provider.sys_params.is_64_bit:

diff  --git a/lldb/examples/summaries/cocoa/CFBinaryHeap.py 
b/lldb/examples/summaries/cocoa/CFBinaryHeap.py
index 061ab61ae4c55..f22a044b6811e 100644
--- a/lldb/examples/summaries/cocoa/CFBinaryHeap.py
+++ b/lldb/examples/summaries/cocoa/CFBinaryHeap.py
@@ -13,11 +13,6 @@
 import lldb.formatters.metrics
 import lldb.formatters.Logger
 
-try:
-basestring
-except NameError:
-basestring = str
-
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
@@ -148,7 +143,7 @@ def CFBinaryHeap_SummaryProvider(valobj, dict):
 #  the mask needs to be changed)
 if summary is None:
 summary = ''
-elif isinstance(summary, basestring):
+elif isinstance(summary, str):
 pass
 else:
 if provider.sys_params.is_64_bit:

diff  --git a/lldb/examples/summaries/cocoa/CFDictionary.py 
b/lldb/examples/summaries/cocoa/CFDictionary.py
index 6fcc0a6fe7712..fa132605cb185 100644
--- a/lldb/examples/summaries/cocoa/CFDictionary.py
+++ b/lldb/examples/summaries/cocoa/CFDictionary.py
@@ -13,11 +13,6 @@
 import lldb.formatters.metrics
 import lldb.formatters.Logger
 
-try:
-basestring
-except NameError
-basestring = str
-
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
@@ -227,7 +222,7 @@ def CFDictionary_SummaryProvider(valobj, dict):
 logger >> "got summary " + str(summary)
 if summary is None:
 return ''
-if isinstance(summary, basestring):
+if isinstance(summary, str):
 return summary
 return str(summary) + (" key/value pairs" if summary !=
1 else " key/value pair")
@@ -249,7 +244,7 @@ def CFDictionary_SummaryProvider2(valobj, dict):
 logger >> "got summary " + str(summary)
 if summary is None:
 summary = ''
-if isinstance(summary, basestring):
+if isinstance(summary, str):
 return summary
 else:
 # needed on OSX Mountain Lion

diff  --git a/lldb/examples/summaries/cocoa/NSData.py 
b/lldb/examples/summaries/cocoa/NSData.py
index 005a1cac50ced..d6edabbc23164 100644
--- a/lldb/examples/summaries/cocoa/NSData.py
+++ b/lldb/examples/summaries/cocoa/NSData.py
@@ -13,11 +13,6 @@
 import ll

[Lldb-commits] [PATCH] D139955: [LLDB] Change formatting to use llvm::formatv

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added a comment.
This revision now requires changes to proceed.

I don't really understand the motivation. Can you elaborate on why "enabling 64 
bit support" requires this change? I definitely prefer the `formatv` approach, 
but wouldn't the PRIx64 should do what you want here?

Also, at a higher level, can we do the same thing as the `Log::Format` function 
that takes varargs and uses `formavt` under the hood?

  template 
  void Format(llvm::StringRef file, llvm::StringRef function,
  const char *format, Args &&... args) {
Format(file, function, llvm::formatv(format, std::forward(args)...));
  }




Comment at: lldb/include/lldb/Core/Module.h:827-837
   void LogMessage(Log *log, const char *format, ...)
   __attribute__((format(printf, 3, 4)));
 
   void LogMessageVerboseBacktrace(Log *log, const char *format, ...)
   __attribute__((format(printf, 3, 4)));
 
   void ReportWarning(const char *format, ...)

Let's not keep both. 



Comment at: lldb/include/lldb/Core/Module.h:839
 
+  // The llvm::formatv version of above APIs
+  void LogMessage(Log *log, const std::string &msg);

This should be (1) a doxygen comment and (2) using a group. But that's moot if 
we don't keep the printf-style variants around. 



Comment at: lldb/source/Core/Module.cpp:1248-1253
+void Module::LogMessage(Log *log, const std::string &msg) {
+  StreamString log_message;
+  GetDescription(log_message.AsRawOstream(), lldb::eDescriptionLevelFull);
+  log_message.PutCString(": ");
+  log->PutCString(log_message.GetData());
+}

This is not using `msg`. Missing `strm.PutCString(msg);`? 



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:11-58
+#include "AppleDWARFIndex.h"
+#include "DWARFASTParser.h"
+#include "DWARFASTParserClang.h"
+#include "DWARFCompileUnit.h"
+#include "DWARFDebugAbbrev.h"
+#include "DWARFDebugAranges.h"
+#include "DWARFDebugInfo.h"

Unrelated change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139955

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

[Lldb-commits] [lldb] ee11ef6 - Launch state discoverable in Darwin, use for SafeToCallFunctions

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2022-12-13T11:42:56-08:00
New Revision: ee11ef6dc0b293161dd916b64581eb37a231709b

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

LOG: Launch state discoverable in Darwin, use for SafeToCallFunctions

The dynamic linker on Darwin, dyld, can provide status of
the process state for a few significant points early on,
most importantly, when libSystem has been initialized and it
is safe to call functions behind the scenes.  Pipe this
information up from debugserver to DynamicLoaderMacOS, for
the DynamicLoader::IsFullyInitialized() method, then have
Thread::SafeToCallFunctions use this information.  Finally,
for the two utility functions in the AppleObjCRuntimeV2
LanguageRuntime plugin that I was fixing, call this method
before running our utility functions to collect the list of
objc classes registered in the runtime.

User expressions will still be allowed to run any time -
we assume the user knows what they are doing - but these
two additional utility functions that they are unaware of
will be limited by this state.

Differential Revision: https://reviews.llvm.org/D139054
rdar://102436092
can probably make function calls.

Added: 
lldb/test/API/macosx/early-process-launch/Makefile
lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
lldb/test/API/macosx/early-process-launch/main.c

Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Target/Process.h
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Target/Thread.cpp
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/DNB.h
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
lldb/tools/debugserver/source/RNBRemote.cpp
lldb/tools/debugserver/source/RNBRemote.h

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 551e94d8b71ad..4930eb1ccb712 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -2153,3 +2153,21 @@ Example packet:
 The data in this packet is a single a character, which should be '0' if the
 inferior process should be killed, or '1' if the server should remove all
 breakpoints and detach from the inferior.
+
+//--
+// "jGetDyldProcessState"
+//
+// BRIEF
+//  This packet fetches the process launch state, as reported by libdyld on
+//  Darwin systems, most importantly to indicate when the system libraries 
+//  have initialized sufficiently to safely call utility functions.
+//
+//
+//  LLDB SENDS: jGetDyldProcessState
+//  STUB REPLIES: {"process_state_value":48,"process_state 
string":"dyld_process_state_libSystem_initialized"}
+//
+// PRIORITY TO IMPLEMENT
+//  Low. This packet is needed to prevent lldb's utility functions for
+//  scanning the Objective-C class list from running very early in 
+//  process startup.
+//--

diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index b9995c2a44326..47040137078f6 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1321,6 +1321,15 @@ class Process : public 
std::enable_shared_from_this,
 return StructuredData::ObjectSP();
   }
 
+  // Get information about the launch state of the process, if possible.
+  //
+  // On Darwin systems, libdyld can report on process state, most importantly
+  // the startup stages where the system library is not yet initialized.
+  virtual lldb_private::StructuredData::ObjectSP
+  GetDynamicLoaderProcessState() {
+return {};
+  }
+
   /// Print a user-visible warning about a module being built with
   /// optimization
   ///

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
index 63b0b96e8ea3f..74db38e4f4efe 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -79,7 +79,8 @@ DynamicLoaderMacOS::DynamicLoaderMacOS(Process *process)
 : DynamicLoaderD

[Lldb-commits] [PATCH] D139054: Delay calling ObjC class list read utility functions very early in process startup

[Jason Molenda via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGee11ef6dc0b2: Launch state discoverable in Darwin, use for 
SafeToCallFunctions (authored by jasonmolenda).

Changed prior to commit:
  https://reviews.llvm.org/D139054?vs=480277&id=482575#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139054

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Thread.cpp
  lldb/test/API/macosx/early-process-launch/Makefile
  lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
  lldb/test/API/macosx/early-process-launch/main.c
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/RNBRemote.cpp
  lldb/tools/debugserver/source/RNBRemote.h

Index: lldb/tools/debugserver/source/RNBRemote.h
===
--- lldb/tools/debugserver/source/RNBRemote.h
+++ lldb/tools/debugserver/source/RNBRemote.h
@@ -136,6 +136,7 @@
 speed_test, // 'qSpeedTest:'
 set_detach_on_error,// 'QSetDetachOnError:'
 query_transfer, // 'qXfer:'
+json_query_dyld_process_state,  // 'jGetDyldProcessState'
 unknown_type
   };
 
@@ -246,6 +247,7 @@
   rnb_err_t HandlePacket_qXfer(const char *p);
   rnb_err_t HandlePacket_stop_process(const char *p);
   rnb_err_t HandlePacket_QSetDetachOnError(const char *p);
+  rnb_err_t HandlePacket_jGetDyldProcessState(const char *p);
   rnb_err_t SendStopReplyPacketForThread(nub_thread_t tid);
   rnb_err_t SendHexEncodedBytePacket(const char *header, const void *buf,
  size_t buf_len, const char *footer);
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -499,6 +499,10 @@
   "Test the maximum speed at which packet can be sent/received."));
   t.push_back(Packet(query_transfer, &RNBRemote::HandlePacket_qXfer, NULL,
  "qXfer:", "Support the qXfer packet."));
+  t.push_back(Packet(json_query_dyld_process_state,
+ &RNBRemote::HandlePacket_jGetDyldProcessState, NULL,
+ "jGetDyldProcessState",
+ "Query the process state from dyld."));
 }
 
 void RNBRemote::FlushSTDIO() {
@@ -5256,6 +5260,22 @@
   return SendPacket(strm.str());
 }
 
+rnb_err_t RNBRemote::HandlePacket_jGetDyldProcessState(const char *p) {
+  const nub_process_t pid = m_ctx.ProcessID();
+  if (pid == INVALID_NUB_PROCESS)
+return SendPacket("E87");
+
+  JSONGenerator::ObjectSP dyld_state_sp = DNBGetDyldProcessState(pid);
+  if (dyld_state_sp) {
+std::ostringstream strm;
+dyld_state_sp->DumpBinaryEscaped(strm);
+dyld_state_sp->Clear();
+if (strm.str().size() > 0)
+  return SendPacket(strm.str());
+  }
+  return SendPacket("E88");
+}
+
 // A helper function that retrieves a single integer value from
 // a one-level-deep JSON dictionary of key-value pairs.  e.g.
 // jThreadExtendedInfo:{"plo_pthread_tsd_base_address_offset":0,"plo_pthread_tsd_base_offset":224,"plo_pthread_tsd_entry_size":8,"thread":144305}]
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -508,7 +508,6 @@
 #define _POSIX_SPAWN_DISABLE_ASLR 0x0100
 #endif
 
-
 MachProcess::MachProcess()
 : m_pid(0), m_cpu_type(0), m_child_stdin(-1), m_child_stdout(-1),
   m_child_stderr(-1), m_path(), m_args(), m_task(this),
@@ -516,8 +515,8 @@
   m_stdio_mutex(PTHREAD_MUTEX_RECURSIVE), m_stdout_data(),
   m_profile_enabled(false), m_profile_interval_usec(0), m_profile_thread(0),
   m_profile_data_mutex(PTHREAD_MUTEX_RECURSIVE), m_profile_data(),
-  m_profile_events(0, eMachProcessProfileCancel),
-  m_thread_actions(), m_exception_messages(),
+  m_profile_events(0, eMachProcessProfileCancel), m_thread_actions(),
+  m_exception_messages(),
  

[Lldb-commits] [PATCH] D139910: [NFC] Cleanup: Remove Function::getBasicBlockList() when not required.

[Vasileios Porpodas 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 rGadfb23c607ce: [NFC] Cleanup: Remove 
Function::getBasicBlockList() when not required. (authored by vporpo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139910

Files:
  clang/lib/CodeGen/CGVTables.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
  llvm/lib/ExecutionEngine/Orc/SpeculateAnalyses.cpp
  llvm/lib/Target/AArch64/SMEABIPass.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/lib/Transforms/IPO/InlineSimple.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/unittests/FuzzMutate/StrategiesTest.cpp
  llvm/unittests/IR/InstructionsTest.cpp
  mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

Index: mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
===
--- mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -321,8 +321,7 @@
   blocks.insert(traversal.begin(), traversal.end());
 }
   }
-  assert(blocks.size() == func->getBasicBlockList().size() &&
- "some blocks are not sorted");
+  assert(blocks.size() == func->size() && "some blocks are not sorted");
 
   return blocks;
 }
Index: llvm/unittests/IR/InstructionsTest.cpp
===
--- llvm/unittests/IR/InstructionsTest.cpp
+++ llvm/unittests/IR/InstructionsTest.cpp
@@ -1516,7 +1516,7 @@
 }
 )");
   Function *Foo = M->getFunction("foo");
-  auto BBs = Foo->getBasicBlockList().begin();
+  auto BBs = Foo->begin();
   CallBrInst &CBI = cast(BBs->front());
   ++BBs;
   ++BBs;
Index: llvm/unittests/FuzzMutate/StrategiesTest.cpp
===
--- llvm/unittests/FuzzMutate/StrategiesTest.cpp
+++ llvm/unittests/FuzzMutate/StrategiesTest.cpp
@@ -505,14 +505,14 @@
   std::unique_ptr M = parseAssembly(Source.data(), Ctx);
   Function *F = &*M->begin();
   DenseMap PreShuffleInstCnt;
-  for (BasicBlock &BB : F->getBasicBlockList()) {
+  for (BasicBlock &BB : *F) {
 PreShuffleInstCnt.insert({&BB, BB.size()});
   }
   std::mt19937 mt(Seed);
   std::uniform_int_distribution RandInt(INT_MIN, INT_MAX);
   for (int i = 0; i < 100; i++) {
 Mutator->mutateModule(*M, RandInt(mt), Source.size(), Source.size() + 1024);
-for (BasicBlock &BB : F->getBasicBlockList()) {
+for (BasicBlock &BB : *F) {
   int PostShuffleIntCnt = BB.size();
   EXPECT_EQ(PostShuffleIntCnt, PreShuffleInstCnt[&BB]);
 }
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -2758,7 +2758,7 @@
 OrigBB->splice(CB.getIterator(), &*FirstNewBlock, FirstNewBlock->begin(),
FirstNewBlock->end());
 // Remove the cloned basic block.
-Caller->getBasicBlockList().pop_back();
+Caller->back().eraseFromParent();
 
 // If the call site was an invoke instruction, add a branch to the normal
 // destination.
@@ -2932,7 +2932,7 @@
   Br->eraseFromParent();
 
   // Now we can remove the CalleeEntry block, which is now empty.
-  Caller->getBasicBlockList().erase(CalleeEntry);
+  CalleeEntry->eraseFromParent();
 
   // If we inserted a phi node, check to see if it has a single value (e.g. all
   // the entries are the same or undef).  If so, remove the PHI so it doesn't
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -381,7 +381,7 @@
   BasicBlock *RBB = RBA->getBasicBlock();
   if (LBB == RBB)
 return 0;
-  for (BasicBlock &BB : F->getBasicBlockList()) {
+  for (BasicBlock &BB : *F) {
 if (&BB == LBB) {
   assert(&BB != RBB);
   return -1;
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1692,7 +1692,7 @@
   IRB.CreateCall(AsanPoisonGlobals, ModuleNameAddr);
 
   // Add calls to unpoison all globals before each return instruction.
-  for (auto &BB : GlobalInit.getBasicBlockList())
+  for (auto &BB : GlobalInit)
 if (ReturnInst *RI = dyn_cast(BB.getTerminator()))
   CallInst::Create(AsanUnpoisonGlobals, "", RI);
 }
Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
==

[Lldb-commits] [lldb] adfb23c - [NFC] Cleanup: Remove Function::getBasicBlockList() when not required.

[Vasileios Porpodas via lldb-commits]

Author: Vasileios Porpodas
Date: 2022-12-13T11:46:29-08:00
New Revision: adfb23c607ce35f090cf3e29fba9d382b22c3a50

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

LOG: [NFC] Cleanup: Remove Function::getBasicBlockList() when not required.

This is part of a series of patches that aim at making 
Function::getBasicBlockList() private.

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

Added: 


Modified: 
clang/lib/CodeGen/CGVTables.cpp

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
llvm/lib/ExecutionEngine/Orc/SpeculateAnalyses.cpp
llvm/lib/Target/AArch64/SMEABIPass.cpp
llvm/lib/Transforms/CFGuard/CFGuard.cpp
llvm/lib/Transforms/IPO/InlineSimple.cpp
llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Utils/FunctionComparator.cpp
llvm/lib/Transforms/Utils/InlineFunction.cpp
llvm/unittests/FuzzMutate/StrategiesTest.cpp
llvm/unittests/IR/InstructionsTest.cpp
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 85acebeeaec9b..354a3f901ff1e 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -128,7 +128,7 @@ static void resolveTopLevelMetadata(llvm::Function *Fn,
 
   // Find all llvm.dbg.declare intrinsics and resolve the DILocalVariable nodes
   // they are referencing.
-  for (auto &BB : Fn->getBasicBlockList()) {
+  for (auto &BB : *Fn) {
 for (auto &I : BB) {
   if (auto *DII = dyn_cast(&I)) {
 auto *DILocal = DII->getVariable();

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
index ec8f8d83c4b37..917242e9b287b 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
@@ -122,7 +122,7 @@ findRSCallSites(llvm::Module &module, 
std::set &rs_callsites,
   bool found = false;
 
   for (auto &func : module.getFunctionList())
-for (auto &block : func.getBasicBlockList())
+for (auto &block : func)
   for (auto &inst : block) {
 llvm::CallInst *call_inst =
 llvm::dyn_cast_or_null(&inst);

diff  --git a/llvm/lib/ExecutionEngine/Orc/SpeculateAnalyses.cpp 
b/llvm/lib/ExecutionEngine/Orc/SpeculateAnalyses.cpp
index 0076267f933e7..42e2a24940127 100644
--- a/llvm/lib/ExecutionEngine/Orc/SpeculateAnalyses.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/SpeculateAnalyses.cpp
@@ -136,7 +136,7 @@ SequenceBBQuery::BlockListTy
 SequenceBBQuery::rearrangeBB(const Function &F, const BlockListTy &BBList) {
   BlockListTy RearrangedBBSet;
 
-  for (auto &Block : F.getBasicBlockList())
+  for (auto &Block : F)
 if (llvm::is_contained(BBList, &Block))
   RearrangedBBSet.push_back(&Block);
 

diff  --git a/llvm/lib/Target/AArch64/SMEABIPass.cpp 
b/llvm/lib/Target/AArch64/SMEABIPass.cpp
index a56f8e052d1c9..83010017c761f 100644
--- a/llvm/lib/Target/AArch64/SMEABIPass.cpp
+++ b/llvm/lib/Target/AArch64/SMEABIPass.cpp
@@ -113,7 +113,7 @@ bool SMEABI::updateNewZAFunctions(Module *M, Function *F,
   Builder.CreateCall(EnableZAIntr->getFunctionType(), EnableZAIntr);
 
   // Before returning, disable pstate.za
-  for (BasicBlock &BB : F->getBasicBlockList()) {
+  for (BasicBlock &BB : *F) {
 Instruction *T = BB.getTerminator();
 if (!T || !isa(T))
   continue;

diff  --git a/llvm/lib/Transforms/CFGuard/CFGuard.cpp 
b/llvm/lib/Transforms/CFGuard/CFGuard.cpp
index 55ef7c214fc84..bebaa6cb59699 100644
--- a/llvm/lib/Transforms/CFGuard/CFGuard.cpp
+++ b/llvm/lib/Transforms/CFGuard/CFGuard.cpp
@@ -272,7 +272,7 @@ bool CFGuard::runOnFunction(Function &F) {
   // instructions. Make a separate list of pointers to indirect
   // call/invoke/callbr instructions because the original instructions will be
   // deleted as the checks are added.
-  for (BasicBlock &BB : F.getBasicBlockList()) {
+  for (BasicBlock &BB : F) {
 for (Instruction &I : BB) {
   auto *CB = dyn_cast(&I);
   if (CB && CB->isIndirectCall() && !CB->hasFnAttr("guard_nocf")) {

diff  --git a/llvm/lib/Transforms/IPO/InlineSimple.cpp 
b/llvm/lib/Transforms/IPO/InlineSimple.cpp
index 2143e39d488dc..eba0d6636d6c0 100644
--- a/llvm/lib/Transforms/IPO/InlineSimple.cpp
+++ b/llvm/lib/Transforms/IPO/InlineSimple.cpp
@@ -50,7 +50,7 @@ class SimpleInliner : public LegacyInlinerBase {
 TargetTransformInfo &TTI = TTIWP->getTTI(*Callee);
 
 bool RemarksEnabled = false;
-

[Lldb-commits] [PATCH] D139951: [lldb/crashlog] Refactor CrashLogParser into a Factory patern

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/examples/python/crashlog.py:419-426
+def CrashLogParserFactory(debugger, path, verbose):
+data = JSONCrashLogParser.is_valid_json(path)
+if data:
+parser = JSONCrashLogParser(debugger, path, verbose)
+parser.data = data
+return parser
+else:

This looks like a class now. The common patter for a factory is to create a 
static method (e.g. `create`) in the class you're instantiating. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139951

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

[Lldb-commits] [PATCH] D139484: [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

[Alex Langford via Phabricator via lldb-commits]

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

LGTM, small nit




Comment at: lldb/test/API/functionalities/scripted_process/main.cpp:21
+   // returns
+  lock.lock();
+}

nit: no need to relock, you're done modifying `n`. The lock will get freed 
right after anyway.


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

https://reviews.llvm.org/D139484

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

[Lldb-commits] [PATCH] D139484: [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/test/API/functionalities/scripted_process/main.cpp:25-26
 void call_and_wait(int &n) {
-  std::cout << "waiting for computation!" << std::endl;
-  while (baz(n) != 42)
+  while (baz(n, mutex, cv) != 42) {
 ;
+  }

Unless I misunderstand the code, I don't think you need this while loop 
anymore. `baz` should block until the condition variable is changed. 


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

https://reviews.llvm.org/D139484

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

[Lldb-commits] [PATCH] D139853: [lldb/Process] Populate queues in Scripted Process

[Jonas Devlieghere via Phabricator via lldb-commits]

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

This looks like what I had in mind. LGTM.


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

https://reviews.llvm.org/D139853

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

[Lldb-commits] [PATCH] D139969: Add a json dumper for call graph reconstructor

[Sujin Park via Phabricator via lldb-commits]

persona0220 created this revision.
persona0220 added reviewers: jj10306, wallace.
Herald added a project: All.
persona0220 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This diff is created on top of D137614 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139969

Files:
  lldb/include/lldb/Target/TraceDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Target/TraceDumper.cpp

Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -340,7 +340,7 @@
   }
 }
 
-TraceDumper::TraceItem TraceDumper::CreatRawTraceItem() {
+TraceDumper::TraceItem TraceDumper::CreateRawTraceItem() {
   TraceItem item = {};
   item.id = m_cursor_sp->GetId();
 
@@ -428,7 +428,7 @@
m_cursor_sp->Next()) {
 
 last_id = m_cursor_sp->GetId();
-TraceItem item = CreatRawTraceItem();
+TraceItem item = CreateRawTraceItem();
 
 if (m_cursor_sp->IsEvent() && m_options.show_events) {
   item.event = m_cursor_sp->GetEventType();
@@ -469,246 +469,390 @@
   return last_id;
 }
 
-TraceCallGraphDumper::TraceCallGraphDumper(
-const TraceCursorSP &cursor_sp, const TraceCallGraphSP &call_graph_sp,
-Stream &s)
-: m_cursor_sp(cursor_sp), m_graph_sp(call_graph_sp), m_s(s),
-  m_exe_ctx(cursor_sp->GetExecutionContextRef()) {}
-
 // Overload pattern-based syntax sugar for easier variant matching.
 template  struct match : Ts... { using Ts::operator()...; };
 template  match(Ts...) -> match;
 
-/// Utility for indenting and writing to a stream in one line.
-static Stream &WithIndent(Stream &s) {
-  s.Indent();
-  return s;
-}
+class GraphOutputWriterCLI : public TraceCallGraphDumper::OutputWriter {
+public:
+  GraphOutputWriterCLI(const TraceCursorSP &cursor_sp,
+  const TraceCallGraphSP &call_graph_sp, Stream &s)
+  : m_cursor_sp(cursor_sp), m_graph_sp(call_graph_sp),
+m_exe_ctx(cursor_sp->GetExecutionContextRef()), m_s(s) {};
+
+  /// Utility for indenting and writing to a stream in one line.
+  Stream &WithIndent() {
+m_s.Indent();
+return m_s;
+  }
 
-/// RAII-based utility for handling indentation and de-indentation of a stream.
-struct AddIndent {
-  AddIndent(Stream &s) : m_s(s) { m_s.IndentMore(); }
-  ~AddIndent() { m_s.IndentLess(); }
+  /// RAII-based utility for handling indentation and de-indentation of a stream.
+  struct AddIndent {
+AddIndent(Stream &s) : m_s(s) { m_s.IndentMore(); }
+~AddIndent() { m_s.IndentLess(); }
 
-private:
-  Stream &m_s;
-};
+  private:
+Stream &m_s;
+  };
 
-void TraceCallGraphDumper::Dump() {
-  Thread &thread = m_exe_ctx.GetThreadRef();
-  m_s.Format("thread #{0}: tid = {1}\n", thread.GetIndexID(), thread.GetID());
+  void Dump() override {
+Thread &thread = m_exe_ctx.GetThreadRef();
+llvm::ArrayRef trees = m_graph_sp->GetTrees();
+m_s.Format("thread #{0}: tid = {1}\n", thread.GetIndexID(), thread.GetID());
 
-  llvm::ArrayRef trees = m_graph_sp->GetTrees();
-  AddIndent _(m_s);
-  for (size_t i = 0; i < trees.size(); i++) {
-m_s << "\n";
-std::visit(
-match{[&](TraceErrorSegment &errors) {
-WithIndent(m_s).Format("\n", i);
-DumpErrorSegment(errors);
-  },
-  [&](TraceInstructionCallTree &tree) {
-WithIndent(m_s).Format("\n", i);
-DumpCall(tree.GetRootCall());
-  }},
-*trees[i]);
+AddIndent _(m_s);
+for (size_t i = 0; i < trees.size(); i++) {
+  m_s << "\n";
+  std::visit(
+  match{[&](TraceErrorSegment &errors) {
+WithIndent().Format("\n", i);
+DumpErrorSegment(errors);
+  },
+  [&](TraceInstructionCallTree &tree) {
+WithIndent().Format("\n", i);
+AddIndent _(m_s);
+DumpCall(tree.GetRootCall());
+  }},
+  *trees[i]);
+}
   }
-}
 
-void TraceCallGraphDumper::DumpErrorSegment(TraceErrorSegment &error_segment) {
-  AddIndent _(m_s);
-
-  auto print_error = [&](lldb::user_id_t error_id) {
-m_cursor_sp->GoToId(error_id);
-WithIndent(m_s).Format("|{0}: {1}\n", error_id, m_cursor_sp->GetError());
-  };
+private:
+  static const char *ToDisplayString(const ModuleSP &module_sp) {
+if (const char *str = module_sp->GetFileSpec().GetFilename().AsCString())
+  return str;
+return "(none)";
+  }
 
-  auto [from, to] = error_segment.GetRange();
-  print_error(from);
-  if (from != to) {
-uint64_t remaining = error_segment.GetErrorCount() - 2;
-if (remaining >= 2) {
-  WithIndent(m_s).Format("| ... {0} error{1}\n", remaining,
- remaining == 1 ? "" : "s");
+  void DumpErrorSegment(TraceErrorSegment &error_segment) override {
+AddIndent _(m_s);
+
+auto print_error = [&](ll

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/bindings/python/python-wrapper.swig:317
 
+PythonObject lldb_private::LLDBSwigPythonCreateScriptedPlatform(
+const char *python_class_name, const char *session_dictionary_name,

This looks pretty similar to `LLDBSwigPythonCreateScriptedThread` and 
`LLDBSwigPythonCreateScriptedProcess`. Can we factor out the common parts?



Comment at: lldb/examples/python/scripted_process/scripted_platform.py:33
+
+processes = {
+420: {

Why is this method implemented and not a `pass` like the others?


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

https://reviews.llvm.org/D139250

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

[Lldb-commits] [PATCH] D139252: [lldb/Plugins] Introduce Scripted Platform Plugin

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:25-28
 void ScriptedThread::CheckInterpreterAndScriptObject() const {
-  lldbassert(m_script_object_sp && "Invalid Script Object.");
-  lldbassert(GetInterface() && "Invalid Scripted Thread Interface.");
+  assert(m_script_object_sp && "Invalid Script Object.");
+  assert(GetInterface() && "Invalid Scripted Thread Interface.");
 }

I guess this can be inlined now? 


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

https://reviews.llvm.org/D139252

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

[Lldb-commits] [PATCH] D139969: Add a json dumper for call graph reconstructor

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

I'll take a look at this later tonight or tomorrow


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139969

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

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/examples/python/scripted_process/scripted_platform.py:33
+
+processes = {
+420: {

JDevlieghere wrote:
> Why is this method implemented and not a `pass` like the others?
This method is not implemented, it's just a docstring explaining how this 
method should function. The `pass` is on line 50.


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

https://reviews.llvm.org/D139250

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

[Lldb-commits] [lldb] ad10b3d - Skip TestEarlyProcessLaunch.py w/ system debugserver

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2022-12-13T14:52:46-08:00
New Revision: ad10b3dc3053fbbdea4b4b3c3e2b913fe2224647

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

LOG: Skip TestEarlyProcessLaunch.py w/ system debugserver

This test depends on having a new packet supported by debugserver;
skip it until we have a system debugserver installed on the CI bots
with this change.

Added: 


Modified: 
lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py

Removed: 




diff  --git 
a/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py 
b/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
index 7ab52cb73e80..dd0cc559ecad 100644
--- a/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
+++ b/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
@@ -13,6 +13,9 @@ class TestEarlyProcessLaunch(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipUnlessDarwin
+@skipIfOutOfTreeDebugserver  # 2022-12-13 FIXME: skipping system 
debugserver 
+ # until this feature is included in the system
+ # debugserver.
 @add_test_categories(['pyapi'])
 def test_early_process_launch(self):
 """Test that we don't read objc class tables early in proc startup"""



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

[Lldb-commits] [PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sebastian Neubauer via Phabricator via lldb-commits]

sebastian-ne created this revision.
sebastian-ne added reviewers: jloser, MaskRay, dblaikie, jsilvanus.
Herald added subscribers: ormris, StephenFan, wenlei, hiraditya.
Herald added a project: All.
sebastian-ne requested review of this revision.
Herald added projects: clang, LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits.

This facilitates replacing llvm::Any with std::any.

- Deprecate any_isa in favor of using any_cast(Any*) and checking for nullptr 
because C++17 has no any_isa.
- Remove the assert from any_cast(Any*), so it returns nullptr if the type is 
not correct. This aligns it with std::any_cast(any*).

Use any_cast(Any*) throughout LLVM instead of checks with any_isa.

This is the first part outlined in
https://discourse.llvm.org/t/rfc-switching-from-llvm-any-to-std-any/67176


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139973

Files:
  clang/unittests/Tooling/RefactoringTest.cpp
  clang/unittests/Tooling/TransformerTest.cpp
  lldb/include/lldb/Core/RichManglingContext.h
  llvm/include/llvm/ADT/Any.h
  llvm/lib/CodeGen/MachinePassManager.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
  llvm/lib/Transforms/Scalar/LoopPassManager.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/unittests/ADT/AnyTest.cpp
  llvm/unittests/IR/PassBuilderCallbacksTest.cpp

Index: llvm/unittests/IR/PassBuilderCallbacksTest.cpp
===
--- llvm/unittests/IR/PassBuilderCallbacksTest.cpp
+++ llvm/unittests/IR/PassBuilderCallbacksTest.cpp
@@ -290,17 +290,17 @@
   return std::string(name);
 }
 
-template <> std::string getName(const llvm::Any &WrappedIR) {
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName();
+template <> std::string getName(const Any &WrappedIR) {
+  if (const auto *const *M = any_cast(&WrappedIR))
+return (*M)->getName().str();
+  if (const auto *const *F = any_cast(&WrappedIR))
+return (*F)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *C = any_cast(&WrappedIR))
+return (*C)->getName();
   return "";
 }
 /// Define a custom matcher for objects which support a 'getName' method.
Index: llvm/unittests/ADT/AnyTest.cpp
===
--- llvm/unittests/ADT/AnyTest.cpp
+++ llvm/unittests/ADT/AnyTest.cpp
@@ -24,55 +24,55 @@
 
   // An empty Any is not anything.
   EXPECT_FALSE(A.has_value());
-  EXPECT_FALSE(any_isa(A));
+  EXPECT_FALSE(any_cast(&A));
 
   // An int is an int but not something else.
   EXPECT_TRUE(B.has_value());
-  EXPECT_TRUE(any_isa(B));
-  EXPECT_FALSE(any_isa(B));
+  EXPECT_TRUE(any_cast(&B));
+  EXPECT_FALSE(any_cast(&B));
 
   EXPECT_TRUE(C.has_value());
-  EXPECT_TRUE(any_isa(C));
+  EXPECT_TRUE(any_cast(&C));
 
   // A const char * is a const char * but not an int.
   EXPECT_TRUE(D.has_value());
-  EXPECT_TRUE(any_isa(D));
-  EXPECT_FALSE(any_isa(D));
+  EXPECT_TRUE(any_cast(&D));
+  EXPECT_FALSE(any_cast(&D));
 
   // A double is a double but not a float.
   EXPECT_TRUE(E.has_value());
-  EXPECT_TRUE(any_isa(E));
-  EXPECT_FALSE(any_isa(E));
+  EXPECT_TRUE(any_cast(&E));
+  EXPECT_FALSE(any_cast(&E));
 
   // After copy constructing from an int, the new item and old item are both
   // ints.
   llvm::Any F(B);
   EXPECT_TRUE(B.has_value());
   EXPECT_TRUE(F.has_value());
-  EXPECT_TRUE(any_isa(F));
-  EXPECT_TRUE(any_isa(B));
+  EXPECT_TRUE(any_cast(&F));
+  EXPECT_TRUE(any_cast(&B));
 
   // After move constructing from an int, the new item is an int and the old one
   // isn't.
   llvm::Any G(std::move(C));
   EXPECT_FALSE(C.has_value());
   EXPECT_TRUE(G.has_value());
-  EXPECT_TRUE(any_isa(G));
-  EXPECT_FALSE(any_isa(C));
+  EXPECT_TRUE(any_cast(&G));
+  EXPECT_FALSE(any_cast(&C));
 
   // After copy-assigning from an int, the new item and old item are both ints.
   A = F;
   EXPECT_TRUE(A.has_value());
   EXPECT_TRUE(F.has_value());
-  EXPECT_TRUE(any_isa(A));
-  EXPECT_TRUE(any_isa(F));
+  EXPECT_TRUE(any_cast(&A));
+  EXPECT_TRUE(any_cast(&F));
 
   // After move-assigning from an int, the new item and old item are both ints.
   B = std::move(G);
   EXPECT_TRUE(B.has_value());
   EXPECT_FALSE(G.has_value());
-  EXPECT_TRUE(any_isa(B));
-  EXPECT_FALSE(any_isa(G));
+  EXPECT_TRUE(any_cast(&B));
+  EXPECT_FALSE(any_cast(&G));
 }
 
 TEST(AnyTest, GoodAnyCast) {
Index: llvm/lib/Transforms/Utils/Debugify.cpp
==