[Lldb-commits] [lldb] [lldb] Move DownloadObjectAndSymbolFile to SymbolLocator plugin (PR #71267)

2023-11-04 Thread Alex Langford via lldb-commits

https://github.com/bulbazord edited 
https://github.com/llvm/llvm-project/pull/71267
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Move DownloadObjectAndSymbolFile to SymbolLocator plugin (PR #71267)

2023-11-04 Thread Alex Langford via lldb-commits

https://github.com/bulbazord approved this pull request.


https://github.com/llvm/llvm-project/pull/71267
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Move DownloadObjectAndSymbolFile to SymbolLocator plugin (PR #71267)

2023-11-04 Thread Alex Langford via lldb-commits


@@ -96,6 +96,10 @@ typedef std::optional 
(*SymbolLocatorFindSymbolFileInBundle)(
 const FileSpec &dsym_bundle_fspec, const UUID *uuid, const ArchSpec *arch);
 typedef std::optional (*SymbolLocatorLocateExecutableSymbolFile)(
 const ModuleSpec &module_spec, const FileSpecList &default_search_paths);
+

bulbazord wrote:

Why the long space?

https://github.com/llvm/llvm-project/pull/71267
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [llvm] [compiler-rt] [libcxx] [clang-tools-extra] [openmp] [lldb] [flang] [OpenMP] Add memory diff dump for kernel record-replay (PR #70667)

2023-11-04 Thread Giorgis Georgakoudis via lldb-commits

https://github.com/ggeorgakoudis requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/70667
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [libcxx] [clang] [compiler-rt] [clang-tools-extra] [flang] [lldb] [openmp] [llvm] [OpenMP] Add memory diff dump for kernel record-replay (PR #70667)

2023-11-04 Thread Giorgis Georgakoudis via lldb-commits

https://github.com/ggeorgakoudis edited 
https://github.com/llvm/llvm-project/pull/70667
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [openmp] [compiler-rt] [clang] [libcxx] [clang-tools-extra] [llvm] [flang] [OpenMP] Add memory diff dump for kernel record-replay (PR #70667)

2023-11-04 Thread Giorgis Georgakoudis via lldb-commits


@@ -151,6 +151,74 @@ struct RecordReplayTy {
 OS.close();
   }
 
+  void dumpDeviceMemoryDiff(StringRef Filename) {
+ErrorOr> DeviceMemoryMB =
+WritableMemoryBuffer::getNewUninitMemBuffer(MemorySize);
+if (!DeviceMemoryMB)
+  report_fatal_error("Error creating MemoryBuffer for device memory");
+
+auto Err = Device->dataRetrieve(DeviceMemoryMB.get()->getBufferStart(),
+MemoryStart, MemorySize, nullptr);
+if (Err)
+  report_fatal_error("Error retrieving data for target pointer");
+
+// Get the pre-record memory filename
+SmallString<128> InputFilename = {Filename.split('.').first, ".memory"};
+
+// Read the pre-record memorydump
+auto InputFileBuffer = MemoryBuffer::getFileOrSTDIN(InputFilename);
+if (std::error_code EC = InputFileBuffer.getError())
+  report_fatal_error("Error reading pre-record device memory");
+
+StringRef InputBufferContents = (*InputFileBuffer)->getBuffer();
+if (InputBufferContents.size() != MemorySize)
+  report_fatal_error("Error: Pre-record device memory size mismatch");
+
+std::error_code EC;
+raw_fd_ostream OS(Filename, EC);
+if (EC)
+  report_fatal_error("Error dumping memory to file " + Filename + " :" +
+ EC.message());
+
+// Get current memory contents
+StringRef DeviceMemoryContents(DeviceMemoryMB.get()->getBuffer().data(),
+   DeviceMemoryMB.get()->getBuffer().size());
+
+for (size_t I = 0; I < MemorySize; ++I) {
+  // If buffers are same, continue
+  if (InputBufferContents[I] == DeviceMemoryContents[I])
+continue;
+
+  // If mismatch is found create a new diff line
+  // Current format: location, size, differences
+  OS << I << " "; // Marks the start offset
+
+  SmallVector Modified;
+  Modified.push_back(DeviceMemoryContents[I]);
+
+  size_t J; // Length of current diff line
+  // Loop until next match is found
+  for (J = 1; J < MemorySize - I; ++J) {
+// If no more mismatch, break out of the loop
+if (InputBufferContents[I + J] == DeviceMemoryContents[I + J])
+  break;
+
+// If mismatch continues - push diff to Modified
+Modified.push_back(DeviceMemoryContents[I + J]);
+  }
+
+  OS << J << " "; // Marks the length of the mismatching sequence
+  for (const auto &value : Modified)

ggeorgakoudis wrote:

value -> Value

https://github.com/llvm/llvm-project/pull/70667
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [compiler-rt] [openmp] [clang] [flang] [llvm] [lldb] [libcxx] [clang-tools-extra] [OpenMP] Add memory diff dump for kernel record-replay (PR #70667)

2023-11-04 Thread Giorgis Georgakoudis via lldb-commits


@@ -151,6 +151,74 @@ struct RecordReplayTy {
 OS.close();
   }
 
+  void dumpDeviceMemoryDiff(StringRef Filename) {
+ErrorOr> DeviceMemoryMB =
+WritableMemoryBuffer::getNewUninitMemBuffer(MemorySize);
+if (!DeviceMemoryMB)
+  report_fatal_error("Error creating MemoryBuffer for device memory");
+
+auto Err = Device->dataRetrieve(DeviceMemoryMB.get()->getBufferStart(),
+MemoryStart, MemorySize, nullptr);
+if (Err)
+  report_fatal_error("Error retrieving data for target pointer");
+
+// Get the pre-record memory filename
+SmallString<128> InputFilename = {Filename.split('.').first, ".memory"};
+
+// Read the pre-record memorydump
+auto InputFileBuffer = MemoryBuffer::getFileOrSTDIN(InputFilename);
+if (std::error_code EC = InputFileBuffer.getError())
+  report_fatal_error("Error reading pre-record device memory");
+
+StringRef InputBufferContents = (*InputFileBuffer)->getBuffer();
+if (InputBufferContents.size() != MemorySize)
+  report_fatal_error("Error: Pre-record device memory size mismatch");
+
+std::error_code EC;
+raw_fd_ostream OS(Filename, EC);
+if (EC)
+  report_fatal_error("Error dumping memory to file " + Filename + " :" +
+ EC.message());
+
+// Get current memory contents
+StringRef DeviceMemoryContents(DeviceMemoryMB.get()->getBuffer().data(),
+   DeviceMemoryMB.get()->getBuffer().size());
+
+for (size_t I = 0; I < MemorySize; ++I) {
+  // If buffers are same, continue
+  if (InputBufferContents[I] == DeviceMemoryContents[I])
+continue;
+
+  // If mismatch is found create a new diff line
+  // Current format: location, size, differences
+  OS << I << " "; // Marks the start offset
+
+  SmallVector Modified;
+  Modified.push_back(DeviceMemoryContents[I]);
+
+  size_t J; // Length of current diff line
+  // Loop until next match is found
+  for (J = 1; J < MemorySize - I; ++J) {
+// If no more mismatch, break out of the loop
+if (InputBufferContents[I + J] == DeviceMemoryContents[I + J])
+  break;
+
+// If mismatch continues - push diff to Modified
+Modified.push_back(DeviceMemoryContents[I + J]);
+  }
+
+  OS << J << " "; // Marks the length of the mismatching sequence
+  for (const auto &value : Modified)
+OS << value << " ";
+  OS << "\n";
+
+  // Increment I by J to skip ahead to next
+  // matching sequence in the buffer
+  I += J;

ggeorgakoudis wrote:

Simpler, more readable as:
```
for (I+=1; I < MemorySize; ++I) {
// If no more mismatch, break out of the loop
if (InputBufferContents[I] == DeviceMemoryContents[I])
  break;
// If mismatch continues - push diff to Modified
Modified.push_back(DeviceMemoryContents[I]);
  }
...
OS << Modified.size() << " ";
```
Avoids the skip-ahead increment too

https://github.com/llvm/llvm-project/pull/70667
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [llvm] [clang-tools-extra] [openmp] [clang] [libcxx] [lldb] [compiler-rt] [flang] [OpenMP] Add memory diff dump for kernel record-replay (PR #70667)

2023-11-04 Thread Giorgis Georgakoudis via lldb-commits


@@ -151,6 +151,74 @@ struct RecordReplayTy {
 OS.close();
   }
 
+  void dumpDeviceMemoryDiff(StringRef Filename) {
+ErrorOr> DeviceMemoryMB =
+WritableMemoryBuffer::getNewUninitMemBuffer(MemorySize);
+if (!DeviceMemoryMB)
+  report_fatal_error("Error creating MemoryBuffer for device memory");
+
+auto Err = Device->dataRetrieve(DeviceMemoryMB.get()->getBufferStart(),
+MemoryStart, MemorySize, nullptr);
+if (Err)
+  report_fatal_error("Error retrieving data for target pointer");
+
+// Get the pre-record memory filename
+SmallString<128> InputFilename = {Filename.split('.').first, ".memory"};
+
+// Read the pre-record memorydump
+auto InputFileBuffer = MemoryBuffer::getFileOrSTDIN(InputFilename);
+if (std::error_code EC = InputFileBuffer.getError())
+  report_fatal_error("Error reading pre-record device memory");
+
+StringRef InputBufferContents = (*InputFileBuffer)->getBuffer();
+if (InputBufferContents.size() != MemorySize)
+  report_fatal_error("Error: Pre-record device memory size mismatch");
+
+std::error_code EC;
+raw_fd_ostream OS(Filename, EC);
+if (EC)
+  report_fatal_error("Error dumping memory to file " + Filename + " :" +
+ EC.message());
+
+// Get current memory contents
+StringRef DeviceMemoryContents(DeviceMemoryMB.get()->getBuffer().data(),
+   DeviceMemoryMB.get()->getBuffer().size());
+
+for (size_t I = 0; I < MemorySize; ++I) {
+  // If buffers are same, continue
+  if (InputBufferContents[I] == DeviceMemoryContents[I])
+continue;
+
+  // If mismatch is found create a new diff line
+  // Current format: location, size, differences
+  OS << I << " "; // Marks the start offset

ggeorgakoudis wrote:

Move comment above

https://github.com/llvm/llvm-project/pull/70667
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [llvm] [clang-tools-extra] [flang] [lldb] [clang] [IndVars] Add check of loop invariant for trunc instructions (PR #71072)

2023-11-04 Thread Markos Horro via lldb-commits


@@ -0,0 +1,28 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=indvars -S | FileCheck %s
+
+declare void @foo(i16 noundef)
+
+; Function Attrs: mustprogress noreturn uwtable
+define void @bar(i64 noundef %ptr) {
+; CHECK-LABEL: @bar(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[TMP0:%.*]] = trunc i64 [[PTR:%.*]] to i4
+; CHECK-NEXT:[[TMP1:%.*]] = zext i4 [[TMP0]] to i16
+; CHECK-NEXT:br label [[WHILE_BODY:%.*]]
+; CHECK:   while.body:
+; CHECK-NEXT:tail call void @foo(i16 noundef signext [[TMP1]])
+; CHECK-NEXT:br label [[WHILE_BODY]]
+;
+entry:
+  br label %while.body
+
+while.body:   ; preds = %entry, %while.body
+  %0 = phi i64 [ %ptr, %entry ], [ %add.ptr, %while.body ]
+  %1 = trunc i64 %0 to i16
+  %and = and i16 %1, 15   ; loop invariant
+  tail call void @foo(i16 noundef signext %and)

markoshorro wrote:

Having something like:

`declare void @foo(i2 noundef)

entry:
  %ptr.addr.0 = ptrtoint ptr %ptr to i64
  br label %while.body

while.body:   ; preds = %entry, %while.body
  %0 = phi i64 [ %ptr.addr.0, %entry ], [ %add.ptr, %while.body ]
  %1 = trunc i64 %0 to i2
  %and = and i2 %1, 15   ; loop invariant
  tail call void @foo(i2 noundef signext %and)
  %add.ptr = add nsw i64 %0,  16
  br label %while.body
}`

With this change, the IR generated is the following:

`declare void @foo(i2 noundef)

define void @bar(ptr noundef %ptr) {
entry:
  %ptr.addr.0 = ptrtoint ptr %ptr to i64
  %0 = trunc i64 %ptr.addr.0 to i2
  br label %while.body

while.body:   ; preds = %while.body, %entry
  %and = and i2 %0, -1
  tail call void @foo(i2 noundef signext %and)
  br label %while.body
}`

This is valid to me, when you say that `trunc` can affect the result, do you 
have any example in mind? Thanks!

https://github.com/llvm/llvm-project/pull/71072
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [llvm] [clang-tools-extra] [flang] [lldb] [clang] [IndVars] Add check of loop invariant for trunc instructions (PR #71072)

2023-11-04 Thread Markos Horro via lldb-commits

https://github.com/markoshorro edited 
https://github.com/llvm/llvm-project/pull/71072
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [compiler-rt] [flang] [libcxx] [clang] [openmp] [llvm] [clang-tools-extra] [lldb] [OpenMP] Add memory diff dump for kernel record-replay (PR #70667)

2023-11-04 Thread via lldb-commits

https://github.com/nmustakin updated 
https://github.com/llvm/llvm-project/pull/70667

>From 153c6d812939cd23bb71e53c71378117ed5b23c7 Mon Sep 17 00:00:00 2001
From: Nafis Mustakin 
Date: Mon, 30 Oct 2023 07:50:59 -0700
Subject: [PATCH 1/5] Add memory diff dump for kernel record-replay

---
 .../PluginInterface/PluginInterface.cpp   | 65 +++
 1 file changed, 54 insertions(+), 11 deletions(-)

diff --git 
a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
 
b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
index 0243f0205dbf0e5..8469e8eaf1593cd 100644
--- 
a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
+++ 
b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
@@ -83,7 +83,7 @@ struct RecordReplayTy {
 return Plugin::success();
   }
 
-  void dumpDeviceMemory(StringRef Filename) {
+  void dumpDeviceMemory(StringRef Filename, bool saveDiff) {
 ErrorOr> DeviceMemoryMB =
 WritableMemoryBuffer::getNewUninitMemBuffer(MemorySize);
 if (!DeviceMemoryMB)
@@ -93,15 +93,58 @@ struct RecordReplayTy {
 MemoryStart, MemorySize, nullptr);
 if (Err)
   report_fatal_error("Error retrieving data for target pointer");
-
-StringRef DeviceMemory(DeviceMemoryMB.get()->getBufferStart(), MemorySize);
-std::error_code EC;
-raw_fd_ostream OS(Filename, EC);
-if (EC)
+
+std::error_code EC; 
+raw_fd_ostream OS(Filename, EC); 
+if(EC)
   report_fatal_error("Error dumping memory to file " + Filename + " :" +
  EC.message());
-OS << DeviceMemory;
-OS.close();
+
+if (saveDiff){
+  //Get the pre-record memory filename  
+  SmallString<128> InputFilename = {Filename.split('.').first, ".memory"};
+  //read the pre-record memorydump
+  auto InputFileBuffer = MemoryBuffer::getFileOrSTDIN(InputFilename); 
+  if(std::error_code EC = InputFileBuffer.getError())
+report_fatal_error("Error reading pre-record device memory");
+  
+  StringRef InputBufferContents = (*InputFileBuffer)->getBuffer(); 
+  if(InputBufferContents.size() != MemorySize) 
+report_fatal_error("Error: Pre-record device memory size mismatch");
+  
+  //get current memory contents
+  StringRef DeviceMemoryContents(DeviceMemoryMB.get()->getBuffer().data(),
+ DeviceMemoryMB.get()->getBuffer().size());
+  
+  //compare pre-record memorydump to current contents
+  size_t i = 0;
+  while(i < MemorySize){
+//if mismatch found, create a new diff line
+//current format - location, size, differences ...
+if(InputBufferContents[i] != DeviceMemoryContents[i]){
+  OS << i << " "; //marks the start offset
+  SmallVector modified; 
+  modified.push_back(DeviceMemoryContents[i]);
+  size_t j = 1;
+  //loop until next match is found
+  while(InputBufferContents[i+j] != DeviceMemoryContents[i+j]){
+modified.push_back(DeviceMemoryContents[i+j]);
+j++;
+  }
+  OS << j << " "; //marks the length of the mismatching sequence
+  for(const auto &value : modified)
+OS << value << " ";
+  OS << "\n"; 
+  i+=j+1; 
+}
+else i++; 
+  }
+}
+else {
+  StringRef DeviceMemory(DeviceMemoryMB.get()->getBufferStart(), 
MemorySize);
+  OS << DeviceMemory;
+}
+OS.close();  
   }
 
 public:
@@ -209,7 +252,7 @@ struct RecordReplayTy {
 JsonKernelInfo["ArgOffsets"] = json::Value(std::move(JsonArgOffsets));
 
 SmallString<128> MemoryFilename = {Name, ".memory"};
-dumpDeviceMemory(MemoryFilename);
+dumpDeviceMemory(MemoryFilename, false);
 
 SmallString<128> GlobalsFilename = {Name, ".globals"};
 dumpGlobals(GlobalsFilename, Image);
@@ -227,7 +270,7 @@ struct RecordReplayTy {
   void saveKernelOutputInfo(const char *Name) {
 SmallString<128> OutputFilename = {
 Name, (isRecording() ? ".original.output" : ".replay.output")};
-dumpDeviceMemory(OutputFilename);
+dumpDeviceMemory(OutputFilename, true);
   }
 
   void *alloc(uint64_t Size) {
@@ -1307,7 +1350,7 @@ Error GenericDeviceTy::launchKernel(void *EntryPtr, void 
**ArgPtrs,
 GenericKernel.getName(), GenericKernel.getImage(), ArgPtrs, ArgOffsets,
 KernelArgs.NumArgs, KernelArgs.NumTeams[0], KernelArgs.ThreadLimit[0],
 KernelArgs.Tripcount);
-
+   
   if (RecordReplay.isRecording())
 RecordReplay.saveImage(GenericKernel.getName(), GenericKernel.getImage());
 

>From 8daffad57074dd09287d321acd79c74a667eb65f Mon Sep 17 00:00:00 2001
From: Nafis Mustakin 
Date: Mon, 30 Oct 2023 08:39:40 -0700
Subject: [PATCH 2/5] Fix clang-formatting issues, accept reviewed suggestions

---
 .../PluginInterface/PluginInterface.cpp   | 78 

[Lldb-commits] [lldb] [lldb] Move DownloadObjectAndSymbolFile to SymbolLocator plugin (PR #71267)

2023-11-04 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/71267

>From 8b237e9be1a17008b45d87596ee7384633d78f71 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 3 Nov 2023 14:21:24 -0700
Subject: [PATCH 1/2] [lldb] Move DownloadObjectAndSymbolFile to SymbolLocator
 plugin

This builds on top of the work started in c3a302d to convert
LocateSymbolFile to a SymbolLocator plugin. This commit moves
DownloadObjectAndSymbolFile.
---
 lldb/include/lldb/Core/PluginManager.h|   7 +
 lldb/include/lldb/Symbol/LocateSymbolFile.h   |  14 -
 lldb/include/lldb/lldb-private-interfaces.h   |   4 +
 lldb/source/API/SBTarget.cpp  |   6 +-
 lldb/source/Commands/CommandObjectTarget.cpp  |   5 +-
 lldb/source/Core/DynamicLoader.cpp|   4 +-
 lldb/source/Core/PluginManager.cpp|  22 +-
 .../DynamicLoaderDarwinKernel.cpp |   4 +-
 .../DynamicLoaderFreeBSDKernel.cpp|   3 +-
 .../Process/MacOSX-Kernel/ProcessKDP.cpp  |   4 +-
 .../SymbolLocatorDebugSymbols.cpp | 365 +-
 .../DebugSymbols/SymbolLocatorDebugSymbols.h  |  12 +
 .../Default/SymbolLocatorDefault.cpp  |  10 +-
 .../Default/SymbolLocatorDefault.h|  12 +
 lldb/source/Symbol/CMakeLists.txt |  10 -
 lldb/source/Symbol/LocateSymbolFile.cpp   |  23 +-
 lldb/source/Symbol/LocateSymbolFileMacOSX.cpp | 649 --
 17 files changed, 446 insertions(+), 708 deletions(-)
 delete mode 100644 lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

diff --git a/lldb/include/lldb/Core/PluginManager.h 
b/lldb/include/lldb/Core/PluginManager.h
index 50f0662f4bb2b95..318f8b63c251a19 100644
--- a/lldb/include/lldb/Core/PluginManager.h
+++ b/lldb/include/lldb/Core/PluginManager.h
@@ -353,6 +353,8 @@ class PluginManager {
   nullptr,
   SymbolLocatorLocateExecutableSymbolFile locate_executable_symbol_file =
   nullptr,
+  SymbolLocatorDownloadObjectAndSymbolFile download_object_symbol_file =
+  nullptr,
   SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle = 
nullptr);
 
   static bool UnregisterPlugin(SymbolLocatorCreateInstance create_callback);
@@ -366,6 +368,11 @@ class PluginManager {
   LocateExecutableSymbolFile(const ModuleSpec &module_spec,
  const FileSpecList &default_search_paths);
 
+  static bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
+  Status &error,
+  bool force_lookup = true,
+  bool copy_executable = true);
+
   static FileSpec FindSymbolFileInBundle(const FileSpec &dsym_bundle_fspec,
  const UUID *uuid,
  const ArchSpec *arch);
diff --git a/lldb/include/lldb/Symbol/LocateSymbolFile.h 
b/lldb/include/lldb/Symbol/LocateSymbolFile.h
index 3c0f069c27f8dd6..9247814a9e2ab22 100644
--- a/lldb/include/lldb/Symbol/LocateSymbolFile.h
+++ b/lldb/include/lldb/Symbol/LocateSymbolFile.h
@@ -24,20 +24,6 @@ class UUID;
 
 class Symbols {
 public:
-  // Locate the object and symbol file given a module specification.
-  //
-  // Locating the file can try to download the file from a corporate build
-  // repository, or using any other means necessary to locate both the
-  // unstripped object file and the debug symbols. The force_lookup argument
-  // controls whether the external program is called unconditionally to find
-  // the symbol file, or if the user's settings are checked to see if they've
-  // enabled the external program before calling.
-  //
-  static bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
-  Status &error,
-  bool force_lookup = true,
-  bool copy_executable = true);
-
   /// Locate the symbol file for the given UUID on a background thread. This
   /// function returns immediately. Under the hood it uses the debugger's
   /// thread pool to call DownloadObjectAndSymbolFile. If a symbol file is
diff --git a/lldb/include/lldb/lldb-private-interfaces.h 
b/lldb/include/lldb/lldb-private-interfaces.h
index 4c324a721029075..65181dec644e20d 100644
--- a/lldb/include/lldb/lldb-private-interfaces.h
+++ b/lldb/include/lldb/lldb-private-interfaces.h
@@ -96,6 +96,10 @@ typedef std::optional 
(*SymbolLocatorFindSymbolFileInBundle)(
 const FileSpec &dsym_bundle_fspec, const UUID *uuid, const ArchSpec *arch);
 typedef std::optional (*SymbolLocatorLocateExecutableSymbolFile)(
 const ModuleSpec &module_spec, const FileSpecList &default_search_paths);
+
+typedef bool (*SymbolLocatorDownloadObjectAndSymbolFile)(
+ModuleSpec &module_spec, Status &error, bool force_lookup,
+bool copy_executable);
 typedef bool (*BreakpointHitCallback)(void *baton,
   Stoppoint

[Lldb-commits] [lldb] [lldb] Move DownloadObjectAndSymbolFile to SymbolLocator plugin (PR #71267)

2023-11-04 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere closed 
https://github.com/llvm/llvm-project/pull/71267
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] e7c6147 - [lldb] Move DownloadObjectAndSymbolFile to SymbolLocator plugin (#71267)

2023-11-04 Thread via lldb-commits

Author: Jonas Devlieghere
Date: 2023-11-04T17:58:35-07:00
New Revision: e7c61479cecb3d4359936d323ae1c62c217a6e20

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

LOG: [lldb] Move DownloadObjectAndSymbolFile to SymbolLocator plugin (#71267)

This builds on top of the work started in c3a302d to convert
LocateSymbolFile to a SymbolLocator plugin. This commit moves
DownloadObjectAndSymbolFile.

Added: 


Modified: 
lldb/include/lldb/Core/PluginManager.h
lldb/include/lldb/Symbol/LocateSymbolFile.h
lldb/include/lldb/lldb-private-interfaces.h
lldb/source/API/SBTarget.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Core/DynamicLoader.cpp
lldb/source/Core/PluginManager.cpp

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp

lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.h
lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.h
lldb/source/Symbol/CMakeLists.txt
lldb/source/Symbol/LocateSymbolFile.cpp

Removed: 
lldb/source/Symbol/LocateSymbolFileMacOSX.cpp



diff  --git a/lldb/include/lldb/Core/PluginManager.h 
b/lldb/include/lldb/Core/PluginManager.h
index 50f0662f4bb2b95..318f8b63c251a19 100644
--- a/lldb/include/lldb/Core/PluginManager.h
+++ b/lldb/include/lldb/Core/PluginManager.h
@@ -353,6 +353,8 @@ class PluginManager {
   nullptr,
   SymbolLocatorLocateExecutableSymbolFile locate_executable_symbol_file =
   nullptr,
+  SymbolLocatorDownloadObjectAndSymbolFile download_object_symbol_file =
+  nullptr,
   SymbolLocatorFindSymbolFileInBundle find_symbol_file_in_bundle = 
nullptr);
 
   static bool UnregisterPlugin(SymbolLocatorCreateInstance create_callback);
@@ -366,6 +368,11 @@ class PluginManager {
   LocateExecutableSymbolFile(const ModuleSpec &module_spec,
  const FileSpecList &default_search_paths);
 
+  static bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
+  Status &error,
+  bool force_lookup = true,
+  bool copy_executable = true);
+
   static FileSpec FindSymbolFileInBundle(const FileSpec &dsym_bundle_fspec,
  const UUID *uuid,
  const ArchSpec *arch);

diff  --git a/lldb/include/lldb/Symbol/LocateSymbolFile.h 
b/lldb/include/lldb/Symbol/LocateSymbolFile.h
index 3c0f069c27f8dd6..9247814a9e2ab22 100644
--- a/lldb/include/lldb/Symbol/LocateSymbolFile.h
+++ b/lldb/include/lldb/Symbol/LocateSymbolFile.h
@@ -24,20 +24,6 @@ class UUID;
 
 class Symbols {
 public:
-  // Locate the object and symbol file given a module specification.
-  //
-  // Locating the file can try to download the file from a corporate build
-  // repository, or using any other means necessary to locate both the
-  // unstripped object file and the debug symbols. The force_lookup argument
-  // controls whether the external program is called unconditionally to find
-  // the symbol file, or if the user's settings are checked to see if they've
-  // enabled the external program before calling.
-  //
-  static bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
-  Status &error,
-  bool force_lookup = true,
-  bool copy_executable = true);
-
   /// Locate the symbol file for the given UUID on a background thread. This
   /// function returns immediately. Under the hood it uses the debugger's
   /// thread pool to call DownloadObjectAndSymbolFile. If a symbol file is

diff  --git a/lldb/include/lldb/lldb-private-interfaces.h 
b/lldb/include/lldb/lldb-private-interfaces.h
index 4c324a721029075..53d5fbb84cc92d3 100644
--- a/lldb/include/lldb/lldb-private-interfaces.h
+++ b/lldb/include/lldb/lldb-private-interfaces.h
@@ -96,6 +96,9 @@ typedef std::optional 
(*SymbolLocatorFindSymbolFileInBundle)(
 const FileSpec &dsym_bundle_fspec, const UUID *uuid, const ArchSpec *arch);
 typedef std::optional (*SymbolLocatorLocateExecutableSymbolFile)(
 const ModuleSpec &module_spec, const FileSpecList &default_search_paths);
+typedef bool (*SymbolLocatorDownloadObjectAndSymbolFile)(
+ModuleSpec &module_spec, Status &error, bool force_lookup,
+bool copy_executable);
 typedef bool (*BreakpointHitCallback)(void

[Lldb-commits] [lldb] [lldb] Remove LocateSymbolFile (PR #71301)

2023-11-04 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/71301

This completes the conversion of LocateSymbolFile into a SymbolLocator plugin. 
The only remaining function is DownloadSymbolFileAsync which doesn't really fit 
into the plugin model, and therefore moves into the SymbolLocator class, while 
still relying on the plugins to do the underlying work.

>From e26b213d1556f91e2fb04d880939bd2c7f00b506 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 3 Nov 2023 14:35:34 -0700
Subject: [PATCH] [lldb] Remove LocateSymbolFile

This completes the conversion of LocateSymbolFile into a SymbolLocator
plugin. The only remaining function is DownloadSymbolFileAsync which
doesn't really fit into the plugin model, and therefore moves into the
SymbolLocator class, while still relying on the plugins to do the
underlying work.
---
 lldb/include/lldb/Symbol/SymbolLocator.h  |  8 +++
 lldb/source/API/SBTarget.cpp  |  1 -
 lldb/source/Commands/CommandObjectTarget.cpp  |  1 -
 lldb/source/Core/DynamicLoader.cpp|  1 -
 lldb/source/Core/Module.cpp   |  4 ++--
 lldb/source/Core/ModuleList.cpp   |  1 -
 .../DynamicLoaderDarwinKernel.cpp |  1 -
 .../DynamicLoaderFreeBSDKernel.cpp|  1 -
 .../ObjectFile/Mach-O/ObjectFileMachO.cpp |  1 -
 .../Platform/MacOSX/PlatformDarwin.cpp|  1 -
 .../Process/MacOSX-Kernel/ProcessKDP.cpp  |  1 -
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |  1 -
 .../Process/mach-core/ProcessMachCore.cpp |  1 -
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  |  1 -
 .../SymbolLocatorDebugSymbols.cpp |  1 -
 .../Default/SymbolLocatorDefault.cpp  |  1 -
 .../SymbolVendor/ELF/SymbolVendorELF.cpp  |  1 -
 .../MacOSX/SymbolVendorMacOSX.cpp |  1 -
 .../PECOFF/SymbolVendorPECOFF.cpp |  1 -
 .../SymbolVendor/wasm/SymbolVendorWasm.cpp|  1 -
 lldb/source/Symbol/CMakeLists.txt |  2 +-
 ...LocateSymbolFile.cpp => SymbolLocator.cpp} | 21 +++
 .../unittests/Symbol/LocateSymbolFileTest.cpp |  1 -
 23 files changed, 14 insertions(+), 40 deletions(-)
 rename lldb/source/Symbol/{LocateSymbolFile.cpp => SymbolLocator.cpp} (65%)

diff --git a/lldb/include/lldb/Symbol/SymbolLocator.h 
b/lldb/include/lldb/Symbol/SymbolLocator.h
index e8376ff7568809c..8d2f26b3d0cca32 100644
--- a/lldb/include/lldb/Symbol/SymbolLocator.h
+++ b/lldb/include/lldb/Symbol/SymbolLocator.h
@@ -10,12 +10,20 @@
 #define LLDB_SYMBOL_SYMBOLLOCATOR_H
 
 #include "lldb/Core/PluginInterface.h"
+#include "lldb/Utility/UUID.h"
 
 namespace lldb_private {
 
 class SymbolLocator : public PluginInterface {
 public:
   SymbolLocator() = default;
+
+  /// Locate the symbol file for the given UUID on a background thread. This
+  /// function returns immediately. Under the hood it uses the debugger's
+  /// thread pool to call DownloadObjectAndSymbolFile. If a symbol file is
+  /// found, this will notify all target which contain the module with the
+  /// given UUID.
+  static void DownloadSymbolFileAsync(const UUID &uuid);
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 03134ff54fa46e9..2d029554492a05c 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -7,7 +7,6 @@
 
//===--===//
 
 #include "lldb/API/SBTarget.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Utility/Instrumentation.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/lldb-public.h"
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 180210d45b7e000..8f052d0a7b837e2 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -36,7 +36,6 @@
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/FuncUnwinders.h"
 #include "lldb/Symbol/LineTable.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/UnwindPlan.h"
diff --git a/lldb/source/Core/DynamicLoader.cpp 
b/lldb/source/Core/DynamicLoader.cpp
index f4baaf450ee97ac..7871be6fc451d0a 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core/DynamicLoader.cpp
@@ -14,7 +14,6 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Platform.h"
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 96c1411010b0737..e6279a0feda8684 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -23,11 +23,11 @@
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol

[Lldb-commits] [lldb] [lldb] Remove LocateSymbolFile (PR #71301)

2023-11-04 Thread Jonas Devlieghere via lldb-commits

JDevlieghere wrote:

The fifth and final installment in the SymbolLocator saga.

https://github.com/llvm/llvm-project/pull/71301
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove LocateSymbolFile (PR #71301)

2023-11-04 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

This completes the conversion of LocateSymbolFile into a SymbolLocator plugin. 
The only remaining function is DownloadSymbolFileAsync which doesn't really fit 
into the plugin model, and therefore moves into the SymbolLocator class, while 
still relying on the plugins to do the underlying work.

---
Full diff: https://github.com/llvm/llvm-project/pull/71301.diff


23 Files Affected:

- (modified) lldb/include/lldb/Symbol/SymbolLocator.h (+8) 
- (modified) lldb/source/API/SBTarget.cpp (-1) 
- (modified) lldb/source/Commands/CommandObjectTarget.cpp (-1) 
- (modified) lldb/source/Core/DynamicLoader.cpp (-1) 
- (modified) lldb/source/Core/Module.cpp (+2-2) 
- (modified) lldb/source/Core/ModuleList.cpp (-1) 
- (modified) 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp 
(-1) 
- (modified) 
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp 
(-1) 
- (modified) lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (-1) 
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (-1) 
- (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (-1) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (-1) 
- (modified) lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp (-1) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (-1) 
- (modified) 
lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp 
(-1) 
- (modified) lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp 
(-1) 
- (modified) lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp (-1) 
- (modified) lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp 
(-1) 
- (modified) lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp 
(-1) 
- (modified) lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp (-1) 
- (modified) lldb/source/Symbol/CMakeLists.txt (+1-1) 
- (renamed) lldb/source/Symbol/SymbolLocator.cpp (+3-18) 
- (modified) lldb/unittests/Symbol/LocateSymbolFileTest.cpp (-1) 


``diff
diff --git a/lldb/include/lldb/Symbol/SymbolLocator.h 
b/lldb/include/lldb/Symbol/SymbolLocator.h
index e8376ff7568809c..8d2f26b3d0cca32 100644
--- a/lldb/include/lldb/Symbol/SymbolLocator.h
+++ b/lldb/include/lldb/Symbol/SymbolLocator.h
@@ -10,12 +10,20 @@
 #define LLDB_SYMBOL_SYMBOLLOCATOR_H
 
 #include "lldb/Core/PluginInterface.h"
+#include "lldb/Utility/UUID.h"
 
 namespace lldb_private {
 
 class SymbolLocator : public PluginInterface {
 public:
   SymbolLocator() = default;
+
+  /// Locate the symbol file for the given UUID on a background thread. This
+  /// function returns immediately. Under the hood it uses the debugger's
+  /// thread pool to call DownloadObjectAndSymbolFile. If a symbol file is
+  /// found, this will notify all target which contain the module with the
+  /// given UUID.
+  static void DownloadSymbolFileAsync(const UUID &uuid);
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 03134ff54fa46e9..2d029554492a05c 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -7,7 +7,6 @@
 
//===--===//
 
 #include "lldb/API/SBTarget.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Utility/Instrumentation.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/lldb-public.h"
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 180210d45b7e000..8f052d0a7b837e2 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -36,7 +36,6 @@
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/FuncUnwinders.h"
 #include "lldb/Symbol/LineTable.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/UnwindPlan.h"
diff --git a/lldb/source/Core/DynamicLoader.cpp 
b/lldb/source/Core/DynamicLoader.cpp
index f4baaf450ee97ac..7871be6fc451d0a 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core/DynamicLoader.cpp
@@ -14,7 +14,6 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Platform.h"
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 96c1411010b0737..e6279a0feda8684 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -23,11 +23,11 @@
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
-#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #inc

[Lldb-commits] [lldb] [lldb] Check for abstract methods implementation in Scripted Plugin Objects (PR #71260)

2023-11-04 Thread Med Ismail Bennani via lldb-commits


@@ -26,6 +26,10 @@ class ScriptedPlatformInterface : virtual public 
ScriptedInterface {
 return {llvm::make_error()};
   }
 
+  llvm::SmallVector GetAbstractMethods() const override {
+return {};
+  }

medismailben wrote:

If we don't implement this, we can't instantiate `ScriptedPlatformInterface` 
and that would be a problem for 
`ScriptedInterpreter::CreateScripted{Platform,Process,Thread}Interface`.

What do you think we should if we don't link against python and hence don't 
have a `ScriptedInterpreterPython` instance, to get each of the python 
interface instances ?

https://github.com/llvm/llvm-project/pull/71260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Check for abstract methods implementation in Scripted Plugin Objects (PR #71260)

2023-11-04 Thread Med Ismail Bennani via lldb-commits


@@ -32,6 +32,42 @@ class ScriptedPythonInterface : virtual public 
ScriptedInterface {
   ScriptedPythonInterface(ScriptInterpreterPythonImpl &interpreter);
   ~ScriptedPythonInterface() override = default;
 
+  enum class AbstractMethodCheckerCases {
+eNotImplemented,
+eNotAllocated,
+eNotCallable,
+eValid
+  };
+
+  llvm::Expected>
+  CheckAbstractMethodImplementation(
+  const python::PythonDictionary &class_dict) const {
+
+using namespace python;
+
+std::map checker;
+#define HANDLE_ERROR(method_name, error)   
\
+  {
\
+checker[method_name] = error;  
\
+continue;  
\
+  }

medismailben wrote:

I feel like it helps with reducing code duplication. I'm renaming it as you 
suggested but lets see what other reviewers think of this.

https://github.com/llvm/llvm-project/pull/71260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Check for abstract methods implementation in Scripted Plugin Objects (PR #71260)

2023-11-04 Thread Med Ismail Bennani via lldb-commits

https://github.com/medismailben edited 
https://github.com/llvm/llvm-project/pull/71260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Check for abstract methods implementation in Scripted Plugin Objects (PR #71260)

2023-11-04 Thread Med Ismail Bennani via lldb-commits


@@ -103,24 +139,79 @@ class ScriptedPythonInterface : virtual public 
ScriptedInterface {
   "Resulting object is not initialized.");
 
   std::apply(
-  [&method, &expected_return_object](auto &&...args) {
+  [&init, &expected_return_object](auto &&...args) {
 llvm::consumeError(expected_return_object.takeError());
-expected_return_object = method(args...);
+expected_return_object = init(args...);
   },
   transformed_args);
 
-  if (llvm::Error e = expected_return_object.takeError())
-return std::move(e);
-  result = std::move(expected_return_object.get());
+  if (!expected_return_object)
+return expected_return_object.takeError();
+  result = expected_return_object.get();
 }
 
-if (!result.IsValid())
+if (!result.IsValid() || !result.HasAttribute("__class__"))
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Resulting object is not a valid Python Object.");
+
+PythonObject obj_class = result.GetAttributeValue("__class__");
+
+PythonString obj_class_name =
+obj_class.GetAttributeValue("__name__").AsType();
+
+PythonObject object_class_mapping_proxy =
+obj_class.GetAttributeValue("__dict__");
+
+PythonCallable dict_converter = PythonModule::BuiltinsModule()
+.ResolveName("dict")
+.AsType();
+if (!dict_converter.IsAllocated())
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Resulting object is not a valid Python Object.");

medismailben wrote:

I was planning on updating the error message before making the PR but I forgot 
😅 Thanks for catching that!

https://github.com/llvm/llvm-project/pull/71260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Check for abstract methods implementation in Scripted Plugin Objects (PR #71260)

2023-11-04 Thread Med Ismail Bennani via lldb-commits


@@ -0,0 +1,19 @@
+import os
+
+
+class MissingMethodsScriptedProcess:
+def __init__(self, exe_ctx, args):
+pass
+
+
+def __lldb_init_module(debugger, dict):
+if not "SKIP_SCRIPTED_PROCESS_LAUNCH" in os.environ:

medismailben wrote:

Not really, this is how we've been doing it for all the ScriptedProcess tests.

https://github.com/llvm/llvm-project/pull/71260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Check for abstract methods implementation in Scripted Plugin Objects (PR #71260)

2023-11-04 Thread Med Ismail Bennani via lldb-commits


@@ -663,6 +663,18 @@ bool PythonDictionary::Check(PyObject *py_obj) {
   return PyDict_Check(py_obj);
 }
 
+bool PythonDictionary::HasKey(const llvm::Twine &key) const {
+  if (!IsValid())
+return false;
+  PythonString key_object(key.str().data());

medismailben wrote:

Good point!

https://github.com/llvm/llvm-project/pull/71260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Check for abstract methods implementation in Scripted Plugin Objects (PR #71260)

2023-11-04 Thread Med Ismail Bennani via lldb-commits


@@ -663,6 +663,18 @@ bool PythonDictionary::Check(PyObject *py_obj) {
   return PyDict_Check(py_obj);
 }
 
+bool PythonDictionary::HasKey(const llvm::Twine &key) const {

medismailben wrote:

`PythonDictionary::GetItem` have an overload that takes a `const llvm::Twine&` 
for the `key` parameter, so I wanted to stay consistent with the rest of the 
class. Also, I feel like `llvm::Twine` are offer more flexibility than 
`llvm::StringRef` and they offer an implicit constructor that takes a 
`llvm::StringRef`, so why not use it ?

https://github.com/llvm/llvm-project/pull/71260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove LocateSymbolFile (PR #71301)

2023-11-04 Thread Alex Langford via lldb-commits

https://github.com/bulbazord approved this pull request.

🚀 

https://github.com/llvm/llvm-project/pull/71301
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Check for abstract methods implementation in Scripted Plugin Objects (PR #71260)

2023-11-04 Thread Med Ismail Bennani via lldb-commits

https://github.com/medismailben updated 
https://github.com/llvm/llvm-project/pull/71260

>From 4a95ebbcd2547f802558b4051ef703add9f58ee0 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Sat, 4 Nov 2023 20:37:13 -0700
Subject: [PATCH] [lldb] Check for abstract methods implementation in Scripted
 Plugin Objects

This patch enforces that every scripted object implements all the
necessary abstract methods.

Every scripted affordance language interface can implement a list of
abstract methods name that checked when the object is instanciated.

Since some scripting affordances implementations can be derived from
template base classes, we can't check the object dictionary since it
will contain the definition of the base class, so instead, this checks
the scripting class dictionary.

Previously, for the various python interfaces, we used `ABC.abstractmethod`
decorators but this is too language specific and doesn't work for
scripting affordances that are not derived from template base classes
(i.e OperatingSystem, ScriptedThreadPlan, ...), so this patch provides
generic/language-agnostic checks for every scripted affordance.

Signed-off-by: Med Ismail Bennani 
---
 .../Interfaces/ScriptedInterface.h|   2 +
 .../Interfaces/ScriptedPlatformInterface.h|   4 +
 .../Interfaces/ScriptedProcessInterface.h |   4 +
 .../Interfaces/ScriptedThreadInterface.h  |   4 +
 .../OperatingSystemPythonInterface.h  |   7 +
 .../ScriptedPlatformPythonInterface.h |   6 +
 .../ScriptedProcessPythonInterface.h  |   5 +
 .../Interfaces/ScriptedPythonInterface.h  | 156 ++
 .../ScriptedThreadPythonInterface.h   |   5 +
 .../Python/PythonDataObjects.cpp  |  12 ++
 .../Python/PythonDataObjects.h|   2 +
 .../scripted_process/TestScriptedProcess.py   |  46 ++
 .../missing_methods_scripted_process.py   |  19 +++
 .../Python/PythonDataObjectsTests.cpp |   3 +
 14 files changed, 246 insertions(+), 29 deletions(-)
 create mode 100644 
lldb/test/API/functionalities/scripted_process/missing_methods_scripted_process.py

diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h 
b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h
index e4816352daa5db6..9753a916243b7be 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h
@@ -30,6 +30,8 @@ class ScriptedInterface {
 return m_object_instance_sp;
   }
 
+  virtual llvm::SmallVector GetAbstractMethods() const = 
0;
+
   template 
   static Ret ErrorWithMessage(llvm::StringRef caller_name,
   llvm::StringRef error_msg, Status &error,
diff --git 
a/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h 
b/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h
index 2dcbb47ffa6de85..e73565db88b0ce0 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h
@@ -26,6 +26,10 @@ class ScriptedPlatformInterface : virtual public 
ScriptedInterface {
 return {llvm::make_error()};
   }
 
+  llvm::SmallVector GetAbstractMethods() const override {
+return {};
+  }
+
   virtual StructuredData::DictionarySP ListProcesses() { return {}; }
 
   virtual StructuredData::DictionarySP GetProcessInfo(lldb::pid_t) {
diff --git 
a/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h 
b/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h
index a429cacd862f121..767ad50ba0978e2 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h
@@ -28,6 +28,10 @@ class ScriptedProcessInterface : virtual public 
ScriptedInterface {
 return {llvm::make_error()};
   }
 
+  llvm::SmallVector GetAbstractMethods() const override {
+return {};
+  }
+
   virtual StructuredData::DictionarySP GetCapabilities() { return {}; }
 
   virtual Status Attach(const ProcessAttachInfo &attach_info) {
diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h 
b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h
index 107e593b5561ef7..17bf8bc42490e84 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h
@@ -27,6 +27,10 @@ class ScriptedThreadInterface : virtual public 
ScriptedInterface {
 return {llvm::make_error()};
   }
 
+  llvm::SmallVector GetAbstractMethods() const override {
+return {};
+  }
+
   virtual lldb::tid_t GetThreadID() { return LLDB_INVALID_THREAD_ID; }
 
   virtual std::optional GetName() { return std::nullopt; }
diff --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h
 
b/lldb/source/Plugins/ScriptInterpreter/Python/Interfa

[Lldb-commits] [lldb] [lldb] Remove LocateSymbolFile (PR #71301)

2023-11-04 Thread Logan Abernathy via lldb-commits

https://github.com/M4ximumPizza approved this pull request.


https://github.com/llvm/llvm-project/pull/71301
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Check for abstract methods implementation in Scripted Plugin Objects (PR #71260)

2023-11-04 Thread Med Ismail Bennani via lldb-commits

https://github.com/medismailben updated 
https://github.com/llvm/llvm-project/pull/71260

>From a2beec8a508712ff92191e47eb4ee214617cbfe4 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Sat, 4 Nov 2023 23:53:58 -0700
Subject: [PATCH] [lldb] Check for abstract methods implementation in Scripted
 Plugin Objects

This patch enforces that every scripted object implements all the
necessary abstract methods.

Every scripted affordance language interface can implement a list of
abstract methods name that checked when the object is instanciated.

Since some scripting affordances implementations can be derived from
template base classes, we can't check the object dictionary since it
will contain the definition of the base class, so instead, this checks
the scripting class dictionary.

Previously, for the various python interfaces, we used `ABC.abstractmethod`
decorators but this is too language specific and doesn't work for
scripting affordances that are not derived from template base classes
(i.e OperatingSystem, ScriptedThreadPlan, ...), so this patch provides
generic/language-agnostic checks for every scripted affordance.

Signed-off-by: Med Ismail Bennani 
---
 .../Interfaces/ScriptedInterface.h|   2 +
 .../Interfaces/ScriptedPlatformInterface.h|   4 +
 .../Interfaces/ScriptedProcessInterface.h |   4 +
 .../Interfaces/ScriptedThreadInterface.h  |   4 +
 .../OperatingSystemPythonInterface.h  |   7 +
 .../ScriptedPlatformPythonInterface.h |   6 +
 .../ScriptedProcessPythonInterface.h  |   5 +
 .../Interfaces/ScriptedPythonInterface.h  | 156 ++
 .../ScriptedThreadPythonInterface.h   |   5 +
 .../Python/PythonDataObjects.cpp  |  12 ++
 .../Python/PythonDataObjects.h|   2 +
 .../scripted_process/TestScriptedProcess.py   |  49 ++
 .../missing_methods_scripted_process.py   |  19 +++
 .../Python/PythonDataObjectsTests.cpp |   3 +
 14 files changed, 249 insertions(+), 29 deletions(-)
 create mode 100644 
lldb/test/API/functionalities/scripted_process/missing_methods_scripted_process.py

diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h 
b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h
index e4816352daa5db6..9753a916243b7be 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h
@@ -30,6 +30,8 @@ class ScriptedInterface {
 return m_object_instance_sp;
   }
 
+  virtual llvm::SmallVector GetAbstractMethods() const = 
0;
+
   template 
   static Ret ErrorWithMessage(llvm::StringRef caller_name,
   llvm::StringRef error_msg, Status &error,
diff --git 
a/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h 
b/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h
index 2dcbb47ffa6de85..e73565db88b0ce0 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h
@@ -26,6 +26,10 @@ class ScriptedPlatformInterface : virtual public 
ScriptedInterface {
 return {llvm::make_error()};
   }
 
+  llvm::SmallVector GetAbstractMethods() const override {
+return {};
+  }
+
   virtual StructuredData::DictionarySP ListProcesses() { return {}; }
 
   virtual StructuredData::DictionarySP GetProcessInfo(lldb::pid_t) {
diff --git 
a/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h 
b/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h
index a429cacd862f121..767ad50ba0978e2 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h
@@ -28,6 +28,10 @@ class ScriptedProcessInterface : virtual public 
ScriptedInterface {
 return {llvm::make_error()};
   }
 
+  llvm::SmallVector GetAbstractMethods() const override {
+return {};
+  }
+
   virtual StructuredData::DictionarySP GetCapabilities() { return {}; }
 
   virtual Status Attach(const ProcessAttachInfo &attach_info) {
diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h 
b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h
index 107e593b5561ef7..17bf8bc42490e84 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h
@@ -27,6 +27,10 @@ class ScriptedThreadInterface : virtual public 
ScriptedInterface {
 return {llvm::make_error()};
   }
 
+  llvm::SmallVector GetAbstractMethods() const override {
+return {};
+  }
+
   virtual lldb::tid_t GetThreadID() { return LLDB_INVALID_THREAD_ID; }
 
   virtual std::optional GetName() { return std::nullopt; }
diff --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h
 
b/lldb/source/Plugins/ScriptInterpreter/Python/Interfa