[Lldb-commits] [lldb] [lldb] Move DownloadObjectAndSymbolFile to SymbolLocator plugin (PR #71267)
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)
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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
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