[Lldb-commits] [PATCH] D78801: [LLDB] Add class WasmProcess for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

Thanks @xujuntwt95329! I am very happy that this was useful for WebAssembly 
Micro Runtime!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801

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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class WasmProcess for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

Hi @xujuntwt95329,
Honestly I wasn't thinking to support multithreading with this initial patch so 
I am not surprised that it doesn't work. You are right, we should add thread 
information to the messages.
Actually I am a little surprised that the patch still works after all this time 
(more than one year), and that there have not been small changes that caused 
merge errors.
I am afraid that manually patching LLDB might not be a sustainable solution. :(

@labath, @jingham 
Given that the debugging feature is being supported also by WebAssembly Micro 
Runtime  now, maybe we 
could work together to finalize an implementation of this patch (or also some 
other different solution) that will be less intrusive and more acceptable to be 
merged in LLDB?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801

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

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 243271.
paolosev marked 5 inline comments as done.
paolosev added a comment.

In D72751#1864138 , @labath wrote:

> Ok, let's give this one more try. I have a couple of inline comments for the 
> further simplification of the test case.


Thank you again, I fixed the tests. If it's ok for you, I will then merge this 
patch (I got commit access now :)).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_sym.yaml
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm_embedded_debug_sections.yaml
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm_external_debug_sections.yaml
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/test/Shell/ObjectFile/wasm/basic.yaml
  lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -38,6 +38,7 @@
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h"
 #include "Plugins/Instruction/ARM/EmulateInstructionARM.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
 #include "Plugins/Instruction/MIPS/EmulateInstructionMIPS.h"
@@ -244,6 +245,7 @@
   DynamicLoaderMacOSXDYLD::Initialize();
   DynamicLoaderMacOS::Initialize();
   DynamicLoaderPOSIXDYLD::Initialize();
+  wasm::DynamicLoaderWasmDYLD::Initialize(); // before DynamicLoaderStatic.
   DynamicLoaderStatic::Initialize();
   DynamicLoaderWindowsDYLD::Initialize();
 
@@ -332,6 +334,7 @@
   DynamicLoaderMacOSXDYLD::Terminate();
   DynamicLoaderMacOS::Terminate();
   DynamicLoaderPOSIXDYLD::Terminate();
+  wasm::DynamicLoaderWasmDYLD::Terminate();
   DynamicLoaderStatic::Terminate();
   DynamicLoaderWindowsDYLD::Terminate();
 
Index: lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
@@ -13,11 +13,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
+# CHECK: Executable: false
 # CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Type: shared library
 # CHECK: Strata: user
-# CHECK: Base VM address: 0xa
+# CHECK: Base VM address: 0x0
 
 # CHECK: Name: code
 # CHECK: Type: code
Index: lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
@@ -4,9 +4,9 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Type: shared library
 # CHECK: Strata: user
 # CHECK: Base VM address: 0x0
 
Index: lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
@@ -4,11 +4,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Type: shared library
 # CHECK: Strata: user
-# CHECK: Base VM address: 0xa
+# CHECK: Base VM address: 0x0
 
 # CHECK: Name: code
 # CHECK: Type: code
Index: lldb/test/Shell/ObjectFile/wasm/basic.yaml
===
--- lldb/test/Shell/Obje

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 244300.
paolosev added a comment.

Fix patch after rebasing:

- Use macros to initialize plugins in SystemInitializer.
- Move tests from 
/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/ to 
/test/API/functionalities/gdb_remote_client/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/test/API/functionalities/gdb_remote_client/TestWasm.py
  lldb/test/API/functionalities/gdb_remote_client/test_sym.yaml
  
lldb/test/API/functionalities/gdb_remote_client/test_wasm_embedded_debug_sections.yaml
  
lldb/test/API/functionalities/gdb_remote_client/test_wasm_external_debug_sections.yaml
  lldb/test/Shell/ObjectFile/wasm/basic.yaml
  lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -99,6 +99,7 @@
 LLDB_PLUGIN_DECLARE(DynamicLoaderMacOS)
 LLDB_PLUGIN_DECLARE(DynamicLoaderPOSIXDYLD)
 LLDB_PLUGIN_DECLARE(DynamicLoaderStatic)
+LLDB_PLUGIN_DECLARE(DynamicLoaderWasmDYLD)
 LLDB_PLUGIN_DECLARE(DynamicLoaderWindowsDYLD)
 
 using namespace lldb_private;
@@ -236,6 +237,7 @@
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderMacOSXDYLD);
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderMacOS);
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderPOSIXDYLD);
+  LLDB_PLUGIN_INITIALIZE(DynamicLoaderWasmDYLD); // Before DynamicLoaderStatic.
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderStatic);
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderWindowsDYLD);
 
@@ -324,6 +326,7 @@
   LLDB_PLUGIN_TERMINATE(DynamicLoaderMacOSXDYLD);
   LLDB_PLUGIN_TERMINATE(DynamicLoaderMacOS);
   LLDB_PLUGIN_TERMINATE(DynamicLoaderPOSIXDYLD);
+  LLDB_PLUGIN_TERMINATE(DynamicLoaderWasmDYLD);
   LLDB_PLUGIN_TERMINATE(DynamicLoaderStatic);
   LLDB_PLUGIN_TERMINATE(DynamicLoaderWindowsDYLD);
 
Index: lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
@@ -13,11 +13,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
+# CHECK: Executable: false
 # CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Type: shared library
 # CHECK: Strata: user
-# CHECK: Base VM address: 0xa
+# CHECK: Base VM address: 0x0
 
 # CHECK: Name: code
 # CHECK: Type: code
Index: lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
@@ -4,9 +4,9 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Type: shared library
 # CHECK: Strata: user
 # CHECK: Base VM address: 0x0
 
Index: lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
@@ -4,11 +4,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Type: shared library
 # CHECK: Strata: user
-# CHECK: Base VM address: 0xa
+# CHECK: Base VM address: 0x0
 
 # CHECK: Name: code
 # CHECK: Type: code
Index: lldb/test/Shell/ObjectFile/wasm/basic.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/basic.yaml
+++ lldb/test/Shell/ObjectFile/wasm/basic.yaml
@@ -4,11 +4,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Type:

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D72751#1869268 , @labath wrote:

> yeah, go for it.


Hi @labath, can I ask you the favor to land this patch for me?  I have rebased 
it because the logic of SystemInitializers have changed with new macros.

I asked commit access and I should have obtained it, but when I try to land 
this patch either with 'arc land' or with 'git push' I get a permission error:

  git push --dry-run
  Password for 'https://paolosevm...@github.com':
  remote: Permission to llvm/llvm-project.git denied to paolosevMSFT.
  fatal: unable to access 
'https://paolosevm...@github.com/llvm/llvm-project.git/': The requested URL 
returned error: 403

Maybe there is something I have not understood in the process...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

@JDevlieghere added inline comments:

> lldb/source/API/SystemInitializerFull.cpp 244
>  What's the rationale here? Plugins shouldn't rely on the order in which they 
> are initialized. This breaks when the initializers are auto generated. Can we 
> remove this dependency?

This was discussed in one of the comments above. If DynamicLoaderStatic 
preceeds DynamicLoaderWasmDYLD then it is recognized as a valid loader for a 
triple like "wasm32-unknown-unknown-wasm" because the Triple::OS is 
llvm::Triple::UnknownOS.
There is an explicit check for UnknownOS in 
DynamicLoaderStatic::CreateInstance().
Should ProcessGDBRemote::GetDynamicLoader behave differently just when the 
architecture is wasm32, as a workaround?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

> I think it depends on whether the `DynamicLoaderStatic` should be a fallback. 
> If it doesn't make sense then yes, I think we should reject that triple there.

It should not be a fallback. Ok! I'll create a new patch with this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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

[Lldb-commits] [PATCH] D74780: [lldb/Plugin] Reject WASM and Hexagon in DynamicLoaderStatic

[Paolo Severini via Phabricator via lldb-commits]

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

In D74780#1881297 , @JDevlieghere 
wrote:

> Sorting the plugins so the static loader is initialized before the WASM 
> loader should exercise this code path.


Yes, test //functionalities/gdb_remote_client/TestWasm.py// fails when the 
order of plugins is sorted.

LGTM. Could you also sort the initialization of DynamicLoaderStatic and 
DynamicLoaderWasmDYLD in lldb\tools\lldb-test\SystemInitializerTest.cpp and 
lldb\source\API\SystemInitializerFull.cpp?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D74780



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

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D72751#1892514 , @vsk wrote:

> Hi @paolosev, the lldb sanitized bot is flagging a container-overflow error 
> here. I know that this /can/ have FPs when sanitized and unsanitized code is 
> mixed, but we should be in purely sanitized code here, and this looks like a 
> valid report. PTAL.
>
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-sanitized/992/testReport/junit/lldb-api/functionalities_gdb_remote_client/TestWasm_py/


Thanks for reporting! 
Looking...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

> Hi @paolosev, the lldb sanitized bot is flagging a container-overflow error 
> here. I know that this /can/ have FPs when sanitized and unsanitized code is 
> mixed, but we should be in purely sanitized code here, and this looks like a 
> valid report. PTAL.

I think I might have found a problem with the existing code to cache memory 
read from a remote process. Looking at:

> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-sanitized/992/testReport/junit/lldb-api/functionalities_gdb_remote_client/TestWasm_py/

the error is easily explained. From ProcessGDBRemote::LoadModules we read the 
initial chunk (512 bytes) of an object file:

_lldb.pyd!lldb_private::MemoryCache::Read(unsigned __int64 addr, void * 
dst, unsigned __int64 dst_len, lldb_private::Status & error) Line 239   C++
_lldb.pyd!lldb_private::Process::ReadMemory(unsigned __int64 addr, void 
* buf, unsigned __int64 size, lldb_private::Status & error) Line 1953   C++
_lldb.pyd!lldb_private::Module::GetMemoryObjectFile(const 
std::shared_ptr & process_sp, unsigned __int64 
header_addr, lldb_private::Status & error, unsigned __int64 size_to_read) Line 
300  C++
_lldb.pyd!lldb_private::Process::ReadModuleFromMemory(const 
lldb_private::FileSpec & file_spec, unsigned __int64 header_addr, unsigned 
__int64 size_to_read) Line 2402  C++
_lldb.pyd!lldb_private::DynamicLoader::LoadModuleAtAddress(const 
lldb_private::FileSpec & file, unsigned __int64 link_map_addr, unsigned __int64 
base_addr, bool base_addr_is_offset) Line 212  C++

_lldb.pyd!lldb_private::process_gdb_remote::ProcessGDBRemote::LoadModuleAtAddress(const
 lldb_private::FileSpec & file, unsigned __int64 link_map, unsigned __int64 
base_addr, bool value_is_offset) Line 4769   C++

_lldb.pyd!lldb_private::process_gdb_remote::ProcessGDBRemote::LoadModules() 
Line 4803   C++

In MemoryCache::Read, since this data is not cached yet, we call 
`m_process.ReadMemoryFromInferior` to actually read the memory (lines 174-241), 
look at the bottom of:

  while (bytes_left > 0) {
[...]
BlockMap::const_iterator pos = m_L2_cache.find(curr_addr);
BlockMap::const_iterator end = m_L2_cache.end();
  
if (pos != end) {
  size_t curr_read_size = cache_line_byte_size - cache_offset;
  if (curr_read_size > bytes_left)
curr_read_size = bytes_left;
  
  memcpy(dst_buf + dst_len - bytes_left,
 pos->second->GetBytes() + cache_offset, curr_read_size);
  [...]
}
  
// We need to read from the process
  
if (bytes_left > 0) {
  assert((curr_addr % cache_line_byte_size) == 0);
  std::unique_ptr data_buffer_heap_up(
  new DataBufferHeap(cache_line_byte_size, 0));
  size_t process_bytes_read = m_process.ReadMemoryFromInferior(
  curr_addr, data_buffer_heap_up->GetBytes(),
  data_buffer_heap_up->GetByteSize(), error);
  if (process_bytes_read == 0)
return dst_len - bytes_left;
  
  if (process_bytes_read != cache_line_byte_size)
data_buffer_heap_up->SetByteSize(process_bytes_read);
  m_L2_cache[curr_addr] = DataBufferSP(data_buffer_heap_up.release());
  // We have read data and put it into the cache, continue through the
  // loop again to get the data out of the cache...
}

First, we allocate a DataBufferHeap with the size of our cache_line_byte_size, 
512 size and we pass it to ReadMemoryFromInferior().
The problem is that in this test the whole object file is only 0x84 bytes, so 
we resize `data_buffer_heap_up` to a smaller size with 
`data_buffer_heap_up->SetByteSize(process_bytes_read)`.
Then we iterate back up in the while loop, and try to read from this 
reallocated buffer. But we still try to read curr_read_size==512 bytes, so read 
past the buffer size. In fact the overflow is at address 0x61516184 for a 
buffer that starts at 0x61516100.

This should be very simple to fix; it's getting a bit late today but I should 
have a patch early tomorrow. 
Who is the owner of this MemoryCache code I should ask for a review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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

[Lldb-commits] [PATCH] D75200: AddressSanitizer failure in MemoryCache

[Paolo Severini via Phabricator via lldb-commits]

paolosev created this revision.
paolosev added reviewers: labath, vsk, JDevlieghere, clayborg.
paolosev added a project: LLDB.
Herald added subscribers: lldb-commits, sunfish, aheejin.

The original discussion for this issues is here 
.

The lldb sanitized bot is flagging a container-overflow error after we 
introduced test `TestWasm.py`:
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-sanitized/992/testReport/junit/lldb-api/functionalities_gdb_remote_client/TestWasm_py/




11283==ERROR: AddressSanitizer: container-overflow on address 0x61516184 at 
pc 0x00010b4608f0 bp 0x7ffee4f00130 sp 0x7ffee4eff8f8
-

READ of size 512 at 0x61516184 thread T0

  #0 0x10b4608ef in __asan_memcpy+0x1af 
(libclang_rt.asan_osx_dynamic.dylib:x86_64+0x418ef)
  #1 0x1116486d5 in lldb_private::MemoryCache::Read(unsigned long long, void*, 
unsigned long, lldb_private::Status&) Memory.cpp:189
  #2 0x9d0e9 in 
lldb_private::Module::GetMemoryObjectFile(std::__1::shared_ptr
 const&, unsigned long long, lldb_private::Status&, unsigned long) 
Module.cpp:298
  #3 0x11169eeef in 
lldb_private::Process::ReadModuleFromMemory(lldb_private::FileSpec const&, 
unsigned long long, unsigned long) Process.cpp:2402
  #4 0x3337b in 
lldb_private::DynamicLoader::LoadModuleAtAddress(lldb_private::FileSpec const&, 
unsigned long long, unsigned long long, bool) DynamicLoader.cpp:212
  #5 0x111ed53da in 
lldb_private::process_gdb_remote::ProcessGDBRemote::LoadModuleAtAddress(lldb_private::FileSpec
 const&, unsigned long long, unsigned long long, bool) ProcessGDBRemote.cpp:4767
  #6 0x111ed59b8 in 
lldb_private::process_gdb_remote::ProcessGDBRemote::LoadModules() 
ProcessGDBRemote.cpp:4801
  #7 0x1119c59aa in lldb_private::wasm::DynamicLoaderWasmDYLD::DidAttach() 
DynamicLoaderWasmDYLD.cpp:63
  #8 0x1116a3a97 in lldb_private::Process::CompleteAttach() Process.cpp:2930
  #9 0x1116a6bdf in lldb_private::Process::ConnectRemote(lldb_private::Stream*, 
llvm::StringRef) Process.cpp:3015
  #10 0x110a362ee in lldb::SBTarget::ConnectRemote(lldb::SBListener&, char 
const*, char const*, lldb::SBError&) SBTarget.cpp:559

There is actually a problem in the MemoryCache code. From 
ProcessGDBRemote::LoadModules we read the initial chunk (512 bytes) of an 
object file. In MemoryCache::Read, since this data is not cached yet, we call 
m_process.ReadMemoryFromInferior to actually read the memory (lines 174-241), 
look at the bottom of:

  size_t MemoryCache::Read(addr_t addr, void *dst, size_t dst_len,
   Status &error) {
  [...]
  while (bytes_left > 0) {
[...]
BlockMap::const_iterator pos = m_L2_cache.find(curr_addr);
BlockMap::const_iterator end = m_L2_cache.end();
  
if (pos != end) {
  size_t curr_read_size = cache_line_byte_size - cache_offset;
  if (curr_read_size > bytes_left)
curr_read_size = bytes_left;
  
  memcpy(dst_buf + dst_len - bytes_left,
 pos->second->GetBytes() + cache_offset, curr_read_size);
  [...]
}
  
// We need to read from the process
  
if (bytes_left > 0) {
  assert((curr_addr % cache_line_byte_size) == 0);
  std::unique_ptr data_buffer_heap_up(
  new DataBufferHeap(cache_line_byte_size, 0));
  size_t process_bytes_read = m_process.ReadMemoryFromInferior(
  curr_addr, data_buffer_heap_up->GetBytes(),
  data_buffer_heap_up->GetByteSize(), error);
  if (process_bytes_read == 0)
return dst_len - bytes_left;
  
  if (process_bytes_read != cache_line_byte_size)
data_buffer_heap_up->SetByteSize(process_bytes_read);
  m_L2_cache[curr_addr] = DataBufferSP(data_buffer_heap_up.release());
  // We have read data and put it into the cache, continue through the
  // loop again to get the data out of the cache...
}

First, we allocate a DataBufferHeap with the size of our cache_line_byte_size, 
512 bytes, and we pass it to ReadMemoryFromInferior().
The problem is that in this test the whole object file is only 0x84 bytes, so 
we resize data_buffer_heap_up to a smaller size with 
data_buffer_heap_up->SetByteSize(process_bytes_read).
Then we iterate back up in the while loop, and try to read from this 
reallocated buffer. But we still try to read curr_read_size==512 bytes, so read 
past the buffer size. In fact the overflow is at address 0x61516184 for a 
buffer that starts at 0x61516100.

Note that the simple fix of just reading the available bytes doesn't work: 
Module::GetMemoryObjectFile expects to always be able to read at least 512 
bytes, and it fails if the object file is smaller:

  ObjectFile *Module::GetMemoryObjectFile(const lldb::ProcessSP &process_sp,

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D72751#1892925 , @labath wrote:

> > Who is the owner of this MemoryCache code I should ask for a review?
>
> It sounds like the fix will be fairly straightforward. I think anyone on this 
> thread should be able to review that.


Patch created: https://reviews.llvm.org/D75200.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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

[Lldb-commits] [PATCH] D75200: AddressSanitizer failure in MemoryCache

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 246868.
paolosev added a comment.

In D75200#1894162 , @clayborg wrote:

> Not sure this is the right fix. The calling code should listen to how many 
> bytes were actually read from memory and avoid touching any bytes. So we 
> should fix Module::GetMemoryObjectFile to resize its buffer back to only the 
> size that was read. S


I agree that the previous fix wasn't great, but it had the advantage that it 
did not change the semantics of `MemoryCache::Read`. I was afraid there could 
be other places where the caller might assume that MemoryCache::Read fills the 
whole buffer even when it does not have enough data to copy, even though it was 
a strange semantics.

(By the way, I am having some difficulties in running //all// the tests with 
'ninja check-lldb' to check this fix, both on Windows and on Linux/WSL2, there 
must be something wrong with my CMake configurations).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75200

Files:
  lldb/source/Core/Module.cpp
  lldb/source/Target/Memory.cpp


Index: lldb/source/Target/Memory.cpp
===
--- lldb/source/Target/Memory.cpp
+++ lldb/source/Target/Memory.cpp
@@ -232,8 +232,13 @@
 if (process_bytes_read == 0)
   return dst_len - bytes_left;
 
-if (process_bytes_read != cache_line_byte_size)
+if (process_bytes_read != cache_line_byte_size) {
+  if (process_bytes_read < data_buffer_heap_up->GetByteSize()) {
+dst_len -= data_buffer_heap_up->GetByteSize() - process_bytes_read;
+bytes_left = process_bytes_read;
+  }
   data_buffer_heap_up->SetByteSize(process_bytes_read);
+}
 m_L2_cache[curr_addr] = DataBufferSP(data_buffer_heap_up.release());
 // We have read data and put it into the cache, continue through the
 // loop again to get the data out of the cache...
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -297,7 +297,9 @@
   const size_t bytes_read =
   process_sp->ReadMemory(header_addr, data_up->GetBytes(),
  data_up->GetByteSize(), readmem_error);
-  if (bytes_read == size_to_read) {
+  if (bytes_read < size_to_read)
+data_up->SetByteSize(bytes_read);
+  if (data_up->GetByteSize() > 0) {
 DataBufferSP data_sp(data_up.release());
 m_objfile_sp = ObjectFile::FindPlugin(shared_from_this(), process_sp,
   header_addr, data_sp);


Index: lldb/source/Target/Memory.cpp
===
--- lldb/source/Target/Memory.cpp
+++ lldb/source/Target/Memory.cpp
@@ -232,8 +232,13 @@
 if (process_bytes_read == 0)
   return dst_len - bytes_left;
 
-if (process_bytes_read != cache_line_byte_size)
+if (process_bytes_read != cache_line_byte_size) {
+  if (process_bytes_read < data_buffer_heap_up->GetByteSize()) {
+dst_len -= data_buffer_heap_up->GetByteSize() - process_bytes_read;
+bytes_left = process_bytes_read;
+  }
   data_buffer_heap_up->SetByteSize(process_bytes_read);
+}
 m_L2_cache[curr_addr] = DataBufferSP(data_buffer_heap_up.release());
 // We have read data and put it into the cache, continue through the
 // loop again to get the data out of the cache...
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -297,7 +297,9 @@
   const size_t bytes_read =
   process_sp->ReadMemory(header_addr, data_up->GetBytes(),
  data_up->GetByteSize(), readmem_error);
-  if (bytes_read == size_to_read) {
+  if (bytes_read < size_to_read)
+data_up->SetByteSize(bytes_read);
+  if (data_up->GetByteSize() > 0) {
 DataBufferSP data_sp(data_up.release());
 m_objfile_sp = ObjectFile::FindPlugin(shared_from_this(), process_sp,
   header_addr, data_sp);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75200: AddressSanitizer failure in MemoryCache

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

The Harbormaster errors don't seem to be real failures:

  
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/lldb/source/Core/Module.cpp:9:10:
 error: 'lldb/Core/Module.h' file not found [clang-diagnostic-error]
  #include "lldb/Core/Module.h"
   ^
  
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/lldb/source/Target/Memory.cpp:9:10:
 error: 'lldb/Target/Memory.h' file not found [clang-diagnostic-error]
  #include "lldb/Target/Memory.h"
   ^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75200



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

[Lldb-commits] [PATCH] D75200: [LLDB] AddressSanitizer failure in MemoryCache

[Paolo Severini via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG256e61699b19: [LLDB] Fix AddressSanitizer failure in 
MemoryCache (authored by paolosev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75200

Files:
  lldb/source/Core/Module.cpp
  lldb/source/Target/Memory.cpp


Index: lldb/source/Target/Memory.cpp
===
--- lldb/source/Target/Memory.cpp
+++ lldb/source/Target/Memory.cpp
@@ -232,8 +232,13 @@
 if (process_bytes_read == 0)
   return dst_len - bytes_left;
 
-if (process_bytes_read != cache_line_byte_size)
+if (process_bytes_read != cache_line_byte_size) {
+  if (process_bytes_read < data_buffer_heap_up->GetByteSize()) {
+dst_len -= data_buffer_heap_up->GetByteSize() - process_bytes_read;
+bytes_left = process_bytes_read;
+  }
   data_buffer_heap_up->SetByteSize(process_bytes_read);
+}
 m_L2_cache[curr_addr] = DataBufferSP(data_buffer_heap_up.release());
 // We have read data and put it into the cache, continue through the
 // loop again to get the data out of the cache...
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -297,7 +297,9 @@
   const size_t bytes_read =
   process_sp->ReadMemory(header_addr, data_up->GetBytes(),
  data_up->GetByteSize(), readmem_error);
-  if (bytes_read == size_to_read) {
+  if (bytes_read < size_to_read)
+data_up->SetByteSize(bytes_read);
+  if (data_up->GetByteSize() > 0) {
 DataBufferSP data_sp(data_up.release());
 m_objfile_sp = ObjectFile::FindPlugin(shared_from_this(), process_sp,
   header_addr, data_sp);


Index: lldb/source/Target/Memory.cpp
===
--- lldb/source/Target/Memory.cpp
+++ lldb/source/Target/Memory.cpp
@@ -232,8 +232,13 @@
 if (process_bytes_read == 0)
   return dst_len - bytes_left;
 
-if (process_bytes_read != cache_line_byte_size)
+if (process_bytes_read != cache_line_byte_size) {
+  if (process_bytes_read < data_buffer_heap_up->GetByteSize()) {
+dst_len -= data_buffer_heap_up->GetByteSize() - process_bytes_read;
+bytes_left = process_bytes_read;
+  }
   data_buffer_heap_up->SetByteSize(process_bytes_read);
+}
 m_L2_cache[curr_addr] = DataBufferSP(data_buffer_heap_up.release());
 // We have read data and put it into the cache, continue through the
 // loop again to get the data out of the cache...
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -297,7 +297,9 @@
   const size_t bytes_read =
   process_sp->ReadMemory(header_addr, data_up->GetBytes(),
  data_up->GetByteSize(), readmem_error);
-  if (bytes_read == size_to_read) {
+  if (bytes_read < size_to_read)
+data_up->SetByteSize(bytes_read);
+  if (data_up->GetByteSize() > 0) {
 DataBufferSP data_sp(data_up.release());
 m_objfile_sp = ObjectFile::FindPlugin(shared_from_this(), process_sp,
   header_addr, data_sp);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D78978: [LLDB] Add support for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D78978#2481358 , @vwzm228 wrote:

> Is there any progress about such patch and D78801 
> ？
>
> I have implemented the debugging feature in our Wasm VM based on  
> https://reviews.llvm.org/D78801, and it already work to attach, set 
> breakpoint, step, show variable value, backtrace...
>
> I am not sure if I need to change LLDB part to this one, or keep using D78801 
> .
>
> But if both patches wont be merged, I have to maintain a private LLDB 
> version

Unfortunately I have not received any more feedback for this patch, or the 
alternative version D78801 , so I assumed it 
won't move forward :(
Meanwhile, I have created a personal fork 
(https://github.com/paolosevMSFT/llvm-project/tree/WasmDbg).

I still think that it would be very useful to add support for Wasm debugging to 
LLDB. Especially in scenarios where Wasm is not used as part of a web app, but 
server side (node.js) or running on micro runtime for IoT devices.
I know that there is the problem that LLDB does not support segmented address 
spaces, and so this patch requires a couple of tiny but smelly changes to core 
code.
From the mailing list I seem to remember that somebody was working to add 
support for segmented addresses, but I don't know what is the current state of 
the initiative.

I'd be happy to keep working on this, please let me know what I could do to 
progress toward an acceptable solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78978

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

[Lldb-commits] [PATCH] D78978: [LLDB] Add support for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D78978#2483354 , @dblaikie wrote:

> Usually the thing is to ping the review thread at most weekly & maybe search 
> around for specific reviewers to ask if you're met with a lot of silence.

There was not a lot of feedback on this thread, but if you look at 
https://reviews.llvm.org/D78801, the discussion there was huge :)
But it's a good point! I'll post the same message there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78978

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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class WasmProcess for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a subscriber: vwzm228.
paolosev added a comment.

In D78978#2481358 , @vwzm228 wrote:

> Is there any progress about such patch and D78801 
> ？
>
> I have implemented the debugging feature in our Wasm VM based on  
> https://reviews.llvm.org/D78801, and it already work to attach, set 
> breakpoint, step, show variable value, backtrace...
>
> I am not sure if I need to change LLDB part to this one, or keep using D78801 
> .
>
> But if both patches wont be merged, I have to maintain a private LLDB 
> version

Unfortunately I have not received any more feedback for this patch, or the 
alternative version D78978 , so I assumed it 
won't move forward :(
Meanwhile, I have created a personal fork 
(https://github.com/paolosevMSFT/llvm-project/tree/WasmDbg).

I still think that it would be very useful to add support for Wasm debugging to 
LLDB. Especially in scenarios where Wasm is not used as part of a web app, but 
server side (node.js) or running on micro runtime for IoT devices.
I know that there is the problem that LLDB does not support segmented address 
spaces, and so this patch requires a couple of tiny but smelly changes to core 
code.
From the mailing list I seem to remember that somebody was working to add 
support for segmented addresses, but I don't know what is the current state of 
the initiative.

I'd be happy to keep working on this, please let me know what I could do to 
progress toward an acceptable solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801

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

[Lldb-commits] [PATCH] D78978: [LLDB] Add support for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D78978#2488134 , @aprantl wrote:

> What's the testing story for WASM going to be?

Thanks for the feedback @aprantl!
This patch was also create to show a possible alternative to 
https://reviews.llvm.org/D78801, but D78801  
has evolved since then and I should probably close/abandon this one.

I had not looked at the testing story for Wasm yet because it seemed that the 
changes to the core code could have been a showstopper, but we would need to 
test a process connected to LLDB through a GDBRemote connection.
I see that this is normally done using the //lldbsuite// package, 
//GdbRemoteTestCaseBase//, which actually launches the debuggee process.
We cannot do the same for Wasm, we cannot run a JS engine that implements a GDB 
Stub here. We should instead write code similar to GdbRemoteTestCaseBase that 
simulates a Wasm process, implementing a GDB stub, handling GDBRemote messages 
(both generic and Wasm-specific).
(This is the opposite of the work done to test the implementation of the GDB 
Stub in V8: there we had the process and we had to simulate the debugger).

Furthermore, we could also extend:

- test\Shell\Reproducer\TestGDBRemoteRepro.test
- test\API\functionalities\gdb_remote_client\TestGDBRemoteClient.py

with other Wasm-specific tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78978

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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev created this revision.
paolosev added reviewers: clayborg, labath.
paolosev added a project: LLDB.
Herald added subscribers: lldb-commits, sunfish, aheejin, jgravelle-google, 
sbc100, aprantl, mgorny.
paolosev marked 2 inline comments as done.
paolosev added a comment.

What is the best way to test classes WasmProcessGDBRemote and UnwindWasm?




Comment at: lldb/source/Plugins/Process/wasm/ProcessWasm.cpp:92
+size_t buffer_size, size_t &size) {
+  // TODO(paolosev): Implement in GDBRemoteComunicationClient
+  return false;

This will be implemented as:
```
  return GetGDBRemote().GetWasmLocal(frame_index, index, buf, buffer_size, 
size);
```
as soon as `GetWasmLocal` can be added to GDBRemoteCommunicationClient.



Comment at: lldb/source/Plugins/Process/wasm/UnwindWasm.cpp:34-35
+
+IWasmProcess *wasm_process =
+static_cast(GetThread().GetProcess().get());
+if (wasm_process)

This cast works but it is ugly. Is there a better coding pattern I could use 
here?


This is the fourth in a series of patches to enable LLDB debugging of 
WebAssembly code that runs in a WebAssembly engine. Previous patches added 
ObjectFile, SymbolVendor and DynamicLoader plugin classes for Wasm, see:  
D71575 , D72751 
, D72650 .

The idea is to use the GDB-remote protocol to connect to a Wasm engine that 
implements a GDB-remote stub that offers the ability to access the engine 
runtime internal state. This patch introduce a new Process plugin //wasm//, 
with:

- class `IWasmProcess` that defines the interface with functions to access the 
Wasm engine state.
- class `WasmProcessGDBRemote` that inherits from `ProcessGDBRemote` and that 
will implement `IWasmProcess` by forwarding requests to the Wasm engine through 
a GDBRemote connection.
- class `UnwindWasm` that manages stack unwinding for Wasm.

Note that the GDB-remote protocol needs to be extended with a few Wasm-specific 
custom query commands, used to access Wasm-specific constructs like the Wasm 
memory, Wasm locals and globals. A patch for this with changes to 
`GDBRemoteCommunicationClient` will be proposed separately, like also a patch 
to `DWARFExpression` to handle Wasm-specific DWARF location expression 
operators, like `DW_OP_WASM_location`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78801

Files:
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/Plugins.def.in
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/wasm/CMakeLists.txt
  lldb/source/Plugins/Process/wasm/ProcessWasm.cpp
  lldb/source/Plugins/Process/wasm/ProcessWasm.h
  lldb/source/Plugins/Process/wasm/UnwindWasm.cpp
  lldb/source/Plugins/Process/wasm/UnwindWasm.h
  lldb/source/Target/Thread.cpp

Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "lldb/Target/Thread.h"
+#include "Plugins/Process/wasm/UnwindWasm.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/FormatEntity.h"
@@ -1853,8 +1854,13 @@
 }
 
 Unwind &Thread::GetUnwinder() {
-  if (!m_unwinder_up)
-m_unwinder_up.reset(new UnwindLLDB(*this));
+  if (!m_unwinder_up) {
+if (CalculateTarget()->GetArchitecture().GetMachine() ==
+llvm::Triple::wasm32)
+  m_unwinder_up.reset(new wasm::UnwindWasm(*this));
+else
+  m_unwinder_up.reset(new UnwindLLDB(*this));
+  }
   return *m_unwinder_up;
 }
 
Index: lldb/source/Plugins/Process/wasm/UnwindWasm.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/wasm/UnwindWasm.h
@@ -0,0 +1,49 @@
+//===-- UnwindWasm.h *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef lldb_UnwindWasm_h_
+#define lldb_UnwindWasm_h_
+
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/Unwind.h"
+#include 
+
+namespace lldb_private {
+namespace wasm {
+
+class UnwindWasm : public lldb_private::Unwind {
+public:
+  UnwindWasm(lldb_private::Thread &thread)
+  : Unwind(thread), m_frames(), m_unwind_complete(false) {}
+  ~UnwindWasm() override = default;
+
+protected:
+  void DoClear() override {
+m_frames.clear();
+m_unwind_complete = false;
+  }
+
+  uint32_t DoGetFrameCount() override;
+

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev marked 2 inline comments as done.
paolosev added a comment.

What is the best way to test classes WasmProcessGDBRemote and UnwindWasm?




Comment at: lldb/source/Plugins/Process/wasm/ProcessWasm.cpp:92
+size_t buffer_size, size_t &size) {
+  // TODO(paolosev): Implement in GDBRemoteComunicationClient
+  return false;

This will be implemented as:
```
  return GetGDBRemote().GetWasmLocal(frame_index, index, buf, buffer_size, 
size);
```
as soon as `GetWasmLocal` can be added to GDBRemoteCommunicationClient.



Comment at: lldb/source/Plugins/Process/wasm/UnwindWasm.cpp:34-35
+
+IWasmProcess *wasm_process =
+static_cast(GetThread().GetProcess().get());
+if (wasm_process)

This cast works but it is ugly. Is there a better coding pattern I could use 
here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 260216.
paolosev added a comment.

I am adding all the pieces to this patch to make the whole picture clearer; I 
thought to add a piece at the time to simplify reviews, but probably it ended 
up making things more obscure. I can always split this patch later and I need 
to refactor everything anyway.

So, the idea is to use DWARF as debug info for Wasm, as it is already supported 
by LLVM and Emscripten. For this we introduced some time ago the plugin classes 
ObjectFileWasm, SymbolVendorWasm and DynamicLoaderWasmDYLD. However, 
WebAssembly is peculiarly different from the native targets. When source code 
is compiled to Wasm, Clang produces a module that contains Wasm bytecode (a bit 
like it happens with Java and C#) and the DWARF info refers to this bytecode.
The Wasm module then runs in a Wasm runtime. (It is also possible to 
AoT-compile Wasm to native, but this is outside the scope of this patch).

Therefore, LLDB cannot debug Wasm by just controlling the inferior process, but 
it needs to talk with the Wasm engine to query the Wasm engine state. For 
example, for backtrace, only the runtime knows what is the current call stack. 
Hence the idea of using the gdb-remote protocol: if a Wasm engine has a GDB 
stub LLDB can connect to it to start a debugging session and access its state.

Wasm execution is defined in terms of a stack machine. There are no registers 
(besides the implicit IP) and most Wasm instructions push/pop values into/from 
a virtual stack. Besides the stack the other possible stores are a set of 
parameters and locals defined in the function, a set of global variables 
defined in the module and the module memory, which is separated from the code 
address space.

The DWARF debug info to evaluate the value of variables is defined in terms of 
these constructs. For example, we can have something like this in DWARF:

  0x5a88:  DW_TAG_variable
DW_AT_location  (0x06f3: 
   [0x0840, 0x0850): DW_OP_WASM_location 
0x0 +8, DW_OP_stack_value)
DW_AT_name  ("xx")
DW_AT_type  (0x2b17 "float")
[…]

Which says that on that address range the value of ‘xx’ can be evaluated as the 
content of the 8th local. Here DW_OP_WASM_location is a Wasm-specific opcode, 
with two args, the first defines the store (0: Local, 1: Global, 2: the operand 
stack) and the index in that store. In most cases the value of the variable 
could be retrieved from the Wasm memory instead.

So, when LLDB wants to evaluate this variable, in 
`DWARFExpression::Evaluate()`, it needs to know what is the current the value 
of the Wasm locals, or to access the memory, and for this it needs to query the 
Wasm engine.

This is why there are changes to DWARFExpression::Evaluate(), to support the 
DW_OP_WASM_location case, and this is also why I created a class that derives 
from ProcessGDBRemote and overrides ReadMemory() in order to query the wasm 
engine. Also Value::GetValueAsData() needs to be modified when the value is 
retrieved from Wasm memory.

`GDBRemoteCommunicationClient` needs to be extended with a few Wasm-specific 
query packets:

- qWasmGlobal: query the value of a Wasm global variable
- qWasmLocal: query the value of a Wasm function argument or local
- qWasmStackValue: query the value in the Wasm operand stack
- qWasmMem: read from a Wasm memory
- qWasmCallStack: retrieve the Wasm call stack.

These are all the changes we need to fully support Wasm debugging.

Why the `IWasmProcess` interface? I was not sure whether gdb-remote should be 
the only way to access the engine state. In the future LLDB could also use some 
other (and less chatty) mechanisms to communicate with a Wasm engine. I did not 
want to put a dependency on GDBRemote in a class like DWARFExpression or Value, 
which should not care about these details. Therefore, I thought that the new 
class WasmProcessGDBRemote could implement the IWasmProcess interface, 
forwarding requests through the base class ProcessGDBRemote which then send the 
new gdb-remote query packets. But I agree that this makes the code certainly 
more convoluted and quite ugly.

My initial idea was to keep all the Wasm-related code as much as possible 
isolated in plugin classes. Now, I guess that the next steps instead would be 
to refactor the code to eliminate the new classes WasmProcessGDBRemote and 
UnwindWasm and modify existing ProcessGDBRemote and ThreadGDBRemote instead. 
However, I am not sure if this is possible without touching also the base 
classes Process and Thread. For example, let’s consider function 
DWARFExpression::Evaluate(). There, when the DWARF opcode is 
DW_OP_WASM_location, we need to access the Wasm state.  We can get to the 
Process object with frame->CalculateProcess() and then can we assume the 
process must always be a ProcessGDBRemote if the target mac

[Lldb-commits] [PATCH] D78978: [LLDB] Add support for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev created this revision.
paolosev added reviewers: labath, clayborg.
paolosev added a project: LLDB.
Herald added subscribers: lldb-commits, sunfish, aheejin, jgravelle-google, 
sbc100, aprantl, mgorny, dschuff.
paolosev edited the summary of this revision.

This patch is a refactoring of  https://reviews.llvm.org/D78801. As suggested, 
I have created a separate patch to more easily compare the implementations.
Even here, the goal is to use the GDB-remote protocol to connect to a Wasm 
engine that implements a GDB-remote stub that offers the ability to access the 
engine runtime internal state.

Here I am removing the interface IWasmProcess and the new class 
WasmProcessGDBRemote and moving most of the Wasm logic in `ProcessGDBRemote`.
I am still adding the new Unwind class `UnwindWasm`, which now however is in 
core and not in plugins code.  Having a Unwind-derived class for Wasm unwinding 
seems to be the cleanest solution.

The advantage with this new design is that the required changes are smaller and 
isolated in a few places.
The drawback is that three 'core' classes (UnwindWasm, Value and 
DWARFExpression) have now a dependency on ProcessGDBRemote and/or 
ThreadGDBRemote. This could be avoided re-introducing a core interface like 
IWasmProcess. We could add a couple of virtual method  `bool IsWasmProcess()` 
and `IWasmProcess* AsWasmProcess()` to Process, which would be overridden by 
ProcessGDBRemote. Then these classes could just query this interface, removing 
direct dependencies on GDBRemote.

I am also adding a new `qSupported` flag named "wasm" that should tell LLDB 
whether the remote supports Wasm debugging.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78978

Files:
  lldb/include/lldb/Target/UnwindWasm.h
  lldb/source/Core/Value.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Thread.cpp
  lldb/source/Target/UnwindWasm.cpp

Index: lldb/source/Target/UnwindWasm.cpp
===
--- /dev/null
+++ lldb/source/Target/UnwindWasm.cpp
@@ -0,0 +1,75 @@
+//===-- UnwindWasm.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Target/UnwindWasm.h"
+#include "lldb/Target/Thread.h"
+
+#include "Plugins/Process/gdb-remote/ProcessGDBRemote.h"
+#include "Plugins/Process/gdb-remote/ThreadGDBRemote.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace process_gdb_remote;
+
+class WasmGDBRemoteRegisterContext : public GDBRemoteRegisterContext {
+public:
+  WasmGDBRemoteRegisterContext(ThreadGDBRemote &thread,
+   uint32_t concrete_frame_idx,
+   GDBRemoteDynamicRegisterInfo ®_info,
+   uint64_t pc)
+  : GDBRemoteRegisterContext(thread, concrete_frame_idx, reg_info, false,
+ false) {
+PrivateSetRegisterValue(0, pc);
+  }
+};
+
+lldb::RegisterContextSP
+UnwindWasm::DoCreateRegisterContextForFrame(lldb_private::StackFrame *frame) {
+  if (m_frames.size() <= frame->GetFrameIndex()) {
+return lldb::RegisterContextSP();
+  }
+
+  ThreadSP thread = frame->GetThread();
+  ThreadGDBRemote *gdb_thread = static_cast(thread.get());
+  ProcessGDBRemote *gdb_process =
+  static_cast(thread->GetProcess().get());
+  std::shared_ptr reg_ctx_sp =
+  std::make_shared(
+  *gdb_thread, frame->GetConcreteFrameIndex(),
+  gdb_process->m_register_info, m_frames[frame->GetFrameIndex()]);
+  return reg_ctx_sp;
+}
+
+uint32_t UnwindWasm::DoGetFrameCount() {
+  if (!m_unwind_complete) {
+m_unwind_complete = true;
+m_frames.clear();
+
+ProcessGDBRemote *gdb_process =
+static_cast(GetThread().GetProcess().get());
+if (!gdb_process->GetWasmCallStack(m_frames))
+  return 0;
+  }
+  return m_frames.size();
+}
+
+bool UnwindWasm::DoGetFrameInfoAtIndex(uint32_t frame_idx, lldb::addr_t &cfa,
+   lldb::addr_t &pc,
+   bool &behaves_like_zeroth_frame) {
+  if (m_frames.size() == 0) {
+DoGetFrameCount();
+  }
+
+  if (frame_idx < m_frames.size()) {
+behaves_like_zeroth_frame = (frame_idx == 0);
+cfa = 0;
+pc = m_frames[frame_idx];
+return true;
+  }
+  return false;
+}
\ No newline at end of file
Index: lldb/source/Target/Thread.cpp
==

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

Thanks for your comments!
I have refactored this code in a separate patch, 
https://reviews.llvm.org/D78978, removing WasmProcessGDBRemote, moving part of 
the logic into ProcessGDBRemote but still keeping class UnwindWasm.
Let me know what you think...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D78801#2007083 , @clayborg wrote:

> So is there memory to be read from the WASM runtime? Couldn't 
> DW_OP_WASM_location 0x0 +8 be turned into an address that can be used to read 
> the variable? It is also unclear what DW_OP_stack_value is used for here. The 
> DWARF expression has no idea how many bytes to read for this value unless 
> each virtual stack location knows how big it is? What happens if you have an 
> array of a million items? That will not fit on the DWARF expression stack and 
> each member would need to be read from memory?
>
> It seems like the DW_OP_WASM_location + args should result in the address of 
> the variable being pushed into the stack and the DW_OP_stack_value should be 
> removed. This would mean at the end of the expression the address of the 
> variable is on the stack and LLDB will just read it using the normal memory 
> read? Am I missing something? Are there multiple memory regions? Are 
> variables not considered to be in memory?


`DW_OP_WASM_location 0x0 +8` is not really in memory, or more precisely, its 
runtime representation is an internal detail of the Wasm runtime.
WebAssembly code has a peculiar structure, see for example 
https://developer.mozilla.org/en-US/docs/WebAssembly/Understanding_the_text_format
 for more details.
Ignoring memory for a moment, there are no registers in Wasm and instead Wasm 
instructions read/write from/to function locals, module globals and stack 
operands, which can only have one of these types:

- i32: 32-bit integer
- i64: 64-bit integer
- f32: 32-bit floating point
- f64: 64-bit floating point

There is still is ongoing work in LLVM 
(https://reviews.llvm.org/D77353/new/#change-OJue38RNV2Gz) to define the 
perfect representation of these Wasm constructs in DWARF, but currently what is 
generated by LLVM has this format:

  DW_OP_WASM_location wasm-op index

Where:

  DW_OP_WASM_location := 0xED
  wasm-op := wasm-local | wasm-global | wasm-operand-stack
  
  wasm-local := 0x00 i:uleb128(The value is located in the 
currently executing function’s index-th local)
  wasm-global := 0x01 i:uleb128   (The value is located in the index-th 
global)
  wasm-operand-stack := 0x02 i:uleb128(The value is located in the indexth 
entry on the operand stack)

https://yurydelendik.github.io/webassembly-dwarf/ describes the rationale 
behind the addition of DW_OP_WASM_location to DWARF.

For example a function like:

  int add(int a, int b) { return a + b; }

Could be compiled to:

  (func $add (param $lhs i32) (param $rhs i32) (result i32)
local.get $lhs
local.get $rhs
i32.add)

and the corresponding DWARF would describe that:

- the value of `a` can be retrieved as DW_OP_WASM_location 0 0 (first local in 
the function)
- the value of `b` can be retrieved as DW_OP_WASM_location 0 1 (second local in 
the function)

Of course DW_OP_WASM_location cannot represent the values of complex types. For 
a complex type like a C++ array with 1M items:

  uint8_t* p = new uint8_t[100];

DWARF would describe the location of the pointer `p` (for example it could be 
in a local) and then the debugger would find DWARF info that describes its 
type, it would then send a request like `qWasmLocal` to get the value from the 
Wasm runtime, and receive the value of p, let’s say 0x8000c000.
From there LLDB might query to read chunks of memory starting from 0x8000c000, 
if the user asks to explore the content of the array.

Note that not all Wasm code requires the new location description 
`DW_OP_WASM_location`. In many cases locations are encoded using preexisting 
codes. For example when compiling without optimizations, -O0, almost all 
variables are encoded as a delta from the frame pointer register. But the frame 
pointer register itself is often defined as a DW_OP_WASM_location:

  0x0112:   DW_TAG_subprogram
  DW_AT_low_pc  (0x0761)
  DW_AT_high_pc (0x07db)
  DW_AT_frame_base  (DW_OP_WASM_location 0x0 +4, 
DW_OP_stack_value)
  DW_AT_linkage_name
("_Z10quick_sortI4NodeIyE4lessIS1_EEvPT_xT0_")
  DW_AT_name("quick_sort, 
less > >")
  DW_AT_decl_file   
("C:\dev\test\emscripten_tests\sort\.\sort.h")
  DW_AT_decl_line   (45)
  DW_AT_external(true)
  
  0x012a: DW_TAG_formal_parameter
DW_AT_location  (DW_OP_fbreg +20)
DW_AT_name  ("array")
DW_AT_type  (0x03bb "Node*")
…

This would also work because LLDB would send a qWasmLocal to calculate the 
value of the frame register.

> Why do we need to override read memory? Is there more than one address space? 
> Can't the DWARF expression DW_OP_WASM_location + args turn into an address 
> that normal read memory can access? Or are the virtual s

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D78801#2009103 , @clayborg wrote:

> Thanks for the explanations if everything WASM related. I now understand much 
> better what you have.
>
> > Read Wasm memory.
> >  IN : $qWasmMem:frame_index;addr;len
> >  OUT: $xx..xx
>
> frame index seems weird in a memory read packet. Seems like the module ID 
> should be passed instead of the frame index.
>
> Reading memory could be handled with memory identifiers, or segments. 
> Currently the packets for m and M only have an address, but someone out there 
> must support reading from CODE and DATA address segments in the DSP world. I 
> have an email out to Ted Woodward to see what they do for Qualcomm's hexagon 
> DSPs. I'll let you know what I find. Maybe each WASM module can identify N 
> segments it needs and each module would have its own unique segments. Are 
> module ID's just 1 based indexes?


True, for qWasmMem (and actually also for qWasmGlobal) it would be sufficient 
to pass the moduleId, but qWasmLocal and qWasmStackValue really need a frame 
index, and it is very easy for the runtime to find the module from a frame 
index, that's why I was passing only frame indices for uniformity. Easy to 
change :)

For reading memory, yes, I came up with ad hoc mechanism that works for Wasm, 
but if I could reuse existing mechanisms already used by other architectures 
and supported by Wasm, obviously it would be better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D78801#2009128 , @clayborg wrote:

> Could we just always use memory reading and have the address contain more 
> info? Right now you have the top 32 bits for the module ID. Could it be 
> something like:
>
>   struct WasmAddress {
> uint64_t module_id:16;
> uint64_t space:4; // 0 == code, 1 == data, 2 == global, 3==local, 4 == 
> stack
> uint64_t frame_id:??;
> uint64_t addr: ??;
>   }
>
>
> This would be a bitfield that would all fit into a 64 bit value and could 
> then be easily sent to the GDB server with the standard m and M packets.


This is interesting. We could certainly use a few bits to specify the space `0 
== code, 1 == data, 2 == global, 3==local, 4 == stack` and then have either the 
module_id or the frame_index, according to the space, and just send "m" packets.

  struct WasmAddress {
uint64_t scope:3;
uint64_t module_id_or_frame_index:29;
uint64_t address: 32;
  }

But then these "m" packets would have to be interpreted according to these 
rules by a Wasm-aware GDB-remote stub, so I am not sure we would gain much 
besides avoiding the introduction of four new custom query commands.
In a function like `Value::GetValueAsData` we would still have to have 
Wasm-specific code to generate the memory address in this format, and actually 
there it is easier to pass the current frame_index, which is readily available, 
rather than calculating the corresponding module_index.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D78801#2009856 , @labath wrote:

> What if the wasm engine actually made some effort to present a more 
> "conventional" view of the wasm "process"? I.e., in addition to the "implied" 
> PC, we could have an "implied" SP (and maybe an FP too). Then it could lay 
> out the call stack and the function arguments in "memory", and point the "SP" 
> to it so that lldb is able to reconstruct the frames&variables using the 
> "normal" algorithm. For the sake of exposition, lets assume the following 
> "encoding" of the 64 bit memory space:
>
>   unsigned:16 type; // 0x = stack
>   unsigned:16 tid; // are there threads in wasm?
>   unsigned:16 frame; // grows down, 0x = bottommost frame (main), 0x0001 
> = second from bottom, etc.
>   unsigned:16 address;
>
>
> Then the engine could say that the "SP" of thread 0x1234 is 
> `0x12340005`, "FP" is `0x123400048010 and the have the memory 
> contents be
>
>   0x123400048020: // third "local" (of frame 4)
>   0x123400048018: // second "local" (of frame 4)
>   0x123400048010: // first "local" (of frame 4)
>   0x123400048008: 0x123400038010 // previous FP (frame 3)
>   0x123400048000:  // previous PC (frame 3)
>   0x123400040010: // third "argument" (of frame 4)
>   0x123400040008: // second "argument" (of frame 4)
>   0x12340004: // first "argument" (of frame 4)
>   0x123400038020: // third "local" (of frame 3)
>   0x123400038018: // second "local" (of frame 3)
>   0x123400038010: // first "local" (of frame 3)
>   0x123400038008: 0x123400028010 // previous FP (frame 2)
>   0x123400038000:  // previous PC (frame 2)
>   etc.
>
>
> Then all it would be needed is to translate `DW_OP_WASM_location` into an 
> appropriate `FP+offset` combo. Somehow...
>
> I realize that this is basically throwing the problem "over the fence", and 
> asking the other side to deal with things, but I am starting to get sceptical 
> that we will be able to come up with a satisfactory solution within lldb.


When you say

> translate DW_OP_WASM_location into an appropriate FP+offset combo.

do you mean that LLVM should generate these `FP+offset` combos rather than 
`DW_OP_WASM_location` or that LLDB should somehow do this translation?
I think the engine can do more to help, here, but not a lot more; I am afraid. 
Yes, it could expose an implied “SP” and “FP”, and that should be sufficient to 
represent locals and arguments and make stack walking more orthodox. But 
DW_OP_WASM_location also describes locations in the set of wasm globals and in 
the Wasm operand stack, so we would need at least a second. parallel stack to 
represent the operand stack.

Also, for C++ LLVM emits code to maintain a “shadow stack” in the linear memory 
of the module, and location expressions like `DW_OP_fbreg +N` are already used 
to describe the location of a parameter or a local variable in that shadow 
stack. The stack frame pointer for that function is described with 
`DW_AT_frame_base`, expressed as a DW_OP_WASM_location expression.

In the end walking the stack is not a big problem, its logic can already be 
encapsulated in a Unwind-derived plugin class. The issues are:

- in `DWARFExpression::Evaluate`, where we need to handle DW_OP_WASM_location 
somehow, and
- in `Value::GetValueAsData`, where we need to read from the memory of the 
current Wasm module, which is a space separated from the address space of code.

I understand that it is not easy to plug in this functionality in a very neat 
way, and maybe I am missing something else here, but if there are no other 
places involved maybe we can come up with a clean solution.

> And I believe the current problems are just the tip of the iceberg. I can't 
> imagine what hoops we'll need to jump through once we start evaluating 
> expressions...

Expression evaluation works, in my prototype, for simple expressions.  For 
complex expressions I see logged errors like this, in 
`IRInterpreter::CanInterpret()`:

  Unsupported instruction: %call = call float 
@_ZNK4Vec3IfE3dotERKS0_(%class.Vec3* %7, %class.Vec3* dereferenceable(12) %8)

It’s not clear to me if the problem is caused by the debug symbols or by the IR 
generated for Wasm… is there any doc where I could learn more about expression 
evaluation in LLDB? It’s a topic that really interests me, even outside the 
scope of this Wasm work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D78801#2010536 , @ted wrote:

> Hi Paulo,
>  @clayborg asked me to look at this, because I've worked with systems that 
> have multiple address spaces. I was thinking, instead of a WebAssembly 
> specific memory read, we should implement an optional generic memory read and 
> write with memory space support.
>
> So instead of qWasmMem:frame_index;addr;len, we have something like 
> qMemSpaceRead:addr;space;len and qMemSpaceWrite:addr;space;len;data .
>
> "space" is stub dependent - it's just a number, and can mean different things 
> for different targets. It could be different modules, like you talk about 
> here, or different physical memory areas like in a Motorola 56K DSP, or 
> different ways to get at the same memory (physical/virtual/cacheable, or 
> directly controlling MESI bits instead of relying on TLB entries) like I 
> implemented in FSLDBG.
>
> If we do this, other targets that want to work with memory spaces can use 
> them instead of having to implement their own extensions.
>
> About the expression problem - the IR Interpreter doesn't handle some complex 
> expressions. It needs work, but most targets use JIT. On Hexagon, we only 
> recently enabled JIT in the compiler, and haven't done it in the debugger 
> yet, so we use the IR Interpreter for everything. Unfortunately I haven't had 
> time to dive into it to make it better.


Hi Ted,

I really like the idea of defining generic commands for reading and writing 
memory on systems with multiple address spaces! In fact there is no reason why 
that should be Wasm-specific.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

Also, to eliminate `qWasmCallStack` we could maybe add the call stack (a list 
of PCs) to the stop packet. The format of a stop packet is:

  T AA n1:r1;n2:r2;…
  
  The program received signal number AA (a two-digit hexadecimal number). [...] 
Each ‘n:r’ pair is interpreted as follows:
  If n is a hexadecimal number, it is a register number, and the corresponding 
r gives that register’s value. [...]
  If n is ‘thread’, then r is the thread-id of the stopped thread, as specified 
in thread-id syntax.
  If n is ‘core’, then r is the hexadecimal number of the core on which the 
stop event was detected.
  Otherwise, GDB should ignore this ‘n:r’ pair and go on to the next; this 
allows us to extend the protocol in the future.

So adding a 'stack:x...xxx' pair should be possible. If we reuse `m`, `M` 
in place of qWasmLocal, qWasmGlobal and qWasmStackValue and add generic 
qMemSpaceRead/qMemSpaceWrite for the memory, then there would be no new 
Wasm-specific command to add to the protocol.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

> The thing I am resisting is to put all of this stuff in the the 
> ProcessGDBRemote class. Instead of trying to generalize it so that it can 
> handle everything (and generalize to the wrong thing), I very much like the 
> idea of introducing a `WasmProcess` plugin class that handles all wasm stuff. 
> If that class happens to use parts of gdb-remote, then so be it, but it means 
> that it's not a problem for that class to use a dialect of the gdb-remote 
> protocol, which includes as many wasm-specific packets as it needs. Then this 
> class can create it's own implementation of the "Unwind" interface, which 
> will use some WasmProcess-specific apis to undwind, but that will also be ok, 
> since both classes will be wasm-specific.

I think that this would solve a lot of problems. `WasmProcess` could inherit 
from GDBRemoteProcess and could send itself Wasm-specific commands like 
qWasmMem just by calling `GetGDBRemote().SendPacketAndWaitForResponse()` Then 
there would be no changes to ProcessGDBRemote at all.
For walking the call stack I would then keep the current design with the 
Unwind-derived class that sends a command to get the call stack from the stub. 
It’s true that normally it is the client that calculates call stacks, but it 
does so even because it cannot really ask them to the stub, but here we have a 
runtime that can provide this information.

> If dwarf had a notion of address spaces then there'd probably be no need for 
> DW_OP_WASM_location. If the dwarf committee hasn't been able to come up with 
> a unified way to represent address spaces, then I'm not sure we will do 
> better. And we'll still be left with the job of translating dwarf (and other) 
> entries into this other concept.

Yes, it is possible to come up with some representation of a unified address 
space for Wasm locals, globals and stack items (and also code and memory). But 
it's also true that locals, globals and stack items don’t really have a memory 
location. The reason for the introduction of `DW_OP_WASM_location` is indeed 
because there was nothing in DWARF that could well represent these entities. 
While there are certainly other architectures with multiple memory spaces, the 
execution model of WebAssembly is quite peculiar in this aspect, I think.

> The question is whether something similar can be done for the other two 
> cases. I believe it might be possible for the DWARFExpression. We just need 
> to have a way to separate the creation of the dwarf expression data from the 
> process of evaluating it. Right now, both of these things happens in 
> SymbolFileDWARF -- it creates a DWARFExpression object which both holds the 
> data and knows how to evaluate it. But I don't believe SymbolFileDWARF needs 
> the second part. If we could make it so that something else is responsible 
> for creating the evaluator for dwarf expressions, that something could create 
> a WasmDWARFExpression which would know about DW_OP_WASM_location and 
> WasmProcess, and could evaluate it.

Separating the DWARFExpression data and the logic to evaluate it would 
certainly make sense. I think that this must be a general problem: since DWARF 
provides the way to define custom expression location, different architectures 
might define specific DWARF codes that they need to handle, so it would be 
great if we had a `DWARFEvaluator`  that was also pluggable. I don’t know how 
complex would be to refactor DWARFExpression in this way but I can investigate.

> The `GetValueAsData` problem is trickier, particularly as I'm not even sure 
> the current implementation is correct. Are you sure that you really need the 
> _current_ module there? What happens if I use "target variable" to display a 
> variable from a different module? What if I then dereference that variable? 
> If the dereference should happen in the context of the other module, then I 
> guess the "module" should be a property of the value, not of the current 
> execution context. And it sounds like some address space-y thing would help. 
> But that might require teaching a lot of places in lldb about address spaces, 
> in particular that a dereferencing a value in one address space, should 
> produce a value in the same address space (at least for "near" pointer or 
> something).
>  If we should really use the current frame as the context, then I guess we'd 
> need some sort of a interfaces to ask a stack frame to get the value of a 
> "Value".

The fact that the code address space is separated from the memory address space 
is really what makes things complicated. However, we know for sure that every 
time that all memory reads made while evaluating DWARF expressions or variables 
always target the module memory space, never the code space.

I must confess that had not really tested `target variable` so far; I did it 
today and I found that it already //almost// works. What happens is that the 
location of the global variable is calculated with DWA

[Lldb-commits] [PATCH] D78801: [LLDB] Add class WasmProcess for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.






Comment at: lldb/source/Plugins/Process/wasm/WasmProcess.cpp:71-76
+  if (vm_addr < 0x1) {
+if (WasmReadMemory(0 /*frame_index*/, vm_addr, buf, size)) {
+  return size;
+}
+return 0;
+  } else

clayborg wrote:
> This seems flaky to me. How are you ever going to get the frame index right 
> unless we encode it into the "vm_addr" using bitfields like we spoke about 
> before. And if we encode it all into the 64 bit address, then we don't need 
> this special read. Seems like we need to figure out if we are going to encode 
> everything into an uint64_t or not. That will be the easiest way to integrate 
> this into LLDB as all memory reads take a "lldb::addr_t" right now (no memory 
> space information). We would change ReadMemory and WriteMemory to start 
> taking a more complex type like:
> 
> ```
> AddressSpecifier {
>   lldb::addr_t addr;
>   uint64_t segment;
> };
> ```
> But that will be a huge change to the LLDB source code that should be done in 
> a separate patch before we do anything here.
I look forward to implementing more cleanly in the future with 
AddressSpecifiers in ReadMemory and WriteMemory; for the moment I have 
implemented this with bitfields as suggested:
```
enum WasmAddressType {
  Code = 0x00,
  Data = 0x01,
  Memory = 0x02,
  Invalid = 0x03
};

struct wasm_addr_t {
  uint64_t type : 2;
  uint64_t module_id : 30;
  uint64_t offset : 32;
};
``` 
I still need to override `ReadMemory` here because it is not always possible to 
generate a full `wasm_addr_t` without making too many changes. Sometimes, for 
example in `ValueObjectChild::UpdateValue -> ValueObject::GetPointerValue` the 
existing code generates addresses without the knowledge of module_ids. But this 
is not a big problem because all these reads always refer to the Wasm Memory 
and the module_id can easily be retrieved from the current execution context. 
This is always true because a Wasm module can never read/write the memory of 
another module in the same process, at most Memories can be shared, but this is 
transparent to the debugger.

However, this requires a small changes to `ReadMemory`: I would introduce 
`ExecutionContext *exe_ctx` as an optional final parameter, null by default, 
passed by `Value` and `ValueObject`, ignored by all other process classes and 
utilized by ProcessWasm to deduce the module.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class WasmProcess for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

Hello @clayborg , @labath,
Any thoughts on this latest patch? :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801



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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class WasmProcess for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.



> What about replacing ProcessReadMemory(addr_t addr, ...) with 
> ProcessReadMemory(Address addr, ...), or even banning the use of lldb::addr_t 
> on everywhere except in the bowels of Process subclasses and as an 
> interchange for getting addresses as text from users.  You might want to 
> store lldb::addr_t's for Symbol values for space concerns, but presumably the 
> Symbol File would know the relevant address space, so it would fix that up 
> and always hand out Addresses.
> 
> This would be a pretty big but mostly formal change.  It would seem more 
> natural, Address is our abstraction for addresses in a process.  Instead we 
> have this odd mix of API's that take lldb::addr_t and some that take Address.

Before all, I really apologize for the huge delay of this reply to your last 
comments. I had to focus on different tasks in the last couple of weeks and I 
needed to find some time to understand how `Process::ReadMemory` (and 
`WriteMemory`) could be modified to work with `Address` objects and not with 
`addr_t`.
It is certainly possible, but a problem, IMHO, is that class `Address` can be 
several things:

- An offset in a section in a file
- An offset in a loaded section
- An absolute address in memory

If we modify ReadMemory to be:

  Process::ReadMemory(const Address& address, void *buf, size_t size, Status 
&error)

then there we need to convert this Address into a concrete `addr_t`, to 
actually read from the MemoryCache (MemoryCache::Read) or from the debuggee 
(ReadMemoryFromInferior).
But how do we do this conversion? If we are dealing with a file we should use 
`lldb::addr_t Address::GetFileAddress()`. Otherwise we should call 
`lldb::addr_t Address::GetLoadAddress(Target *target)` I don't know if we have 
enough information in Process::ReadMemory() to always be able to correctly 
differentiate between the two cases. And in the latter case we should rely on 
Process::GetTarget() returning the valid target to pass to 
Address::GetLoadAddress.

The question then is: should `Address` know if it needs to be interpreted as a 
file address or load address?  Should we have an `AddressType` field in Address?
We already have this enum that maybe we could reuse:

  enum AddressType {
eAddressTypeInvalid = 0,
eAddressTypeFile, /// Address is an address as found in an object or symbol 
file
eAddressTypeLoad, /// Address is an address as in the current target 
inferior process
eAddressTypeHost  /// Address is an address in the process that is running 
code
  };

Process::ReadMemory is called by many places in the LLDB code. In some of them, 
like `Value::GetValueAsData`, `DWARFExpression::Evaluate`, 
`ValueObject::GetPointeeData`, it should be fairly simple to construct the 
Address to read from. (And that should solve the problems for Wasm).
But in other places it is not so clear what we should do. Some code works with 
absolute addresses and does not need Sections (see table below). In these 
cases, we could rely on the Address constructor that accepts just an absolute 
addr_t as argument, and sets `m_section_wp` as null:

  Address(lldb::addr_t abs_addr);

The existence of this constructor already makes most of the LLDB compile even 
after changing function ReadMemory to use an Address as argument, in class 
Process and in Process-derived classes. But then class `Address` would be in 
many cases just a wrapper over an `addr_t`.  It seems to me that would go 
against the idea of using just Address as our abstraction for all addresses in 
a process.

This table describes where Process::ReadMemory is called and how the addresses 
we pass to ReadMemory originate:

| ABI\AArch64\ABIMacOSX_arm64.cpp  | 
LoadValueFromConsecutiveGPRRegisters| 
addr_t from reg_ctx 
|
| ABI\AArch64\ABISysV_arm64.cpp| 
LoadValueFromConsecutiveGPRRegisters| 
addr_t from RegisterContext 
|
| ABI\ARM\ABISysV_arm.cpp  | 
GetReturnValuePassedInMemory| 
addr_t from RegisterContext 
|
| ABI\PowerPC\ABISysV_ppc64.cpp| 
GetStructValueObject| 
addr_t from Register
|
| Commands\CommandObjectMemory.cpp | 
CommandObjectMemoryFind::ProcessMemoryIterator:: operator[] | 
addr_t from m_base_data_address 
|
| DynamicLoader\MacOSX-DYLD\DynamicLoaderMacOSXDYLD.cpp| 
DoInitialImageFetch   

[Lldb-commits] [PATCH] D71575: [LLDB] Add initial support for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev created this revision.
paolosev added reviewers: jasonmolenda, clayborg.
paolosev added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits, sunfish, aheejin, 
jgravelle-google, sbc100, aprantl, mgorny, dschuff.

Current versions of Clang emit (partial) DWARF debug information in WebAssembly 
modules and we can leverage this debug information to give LLDB the ability to 
do source-level debugging of Wasm code that runs in a WebAssembly engine.

A way to do this could be to use the remote debugging functionalities provided 
by LLDB via the GDB-remote protocol. Remote debugging can indeed be useful not 
only to connect a debugger to a process running on a remote machine, but also to
connect the debugger to a managed VM or script engine that runs locally, 
provided that the engine implements a GDB-remote stub that offers the ability 
to access the engine runtime internal state.

To make this work, the GDB-remote protocol would need to be extended with a few 
Wasm-specific custom query commands, used to access aspects of the Wasm engine 
state (like the Wasm memory, Wasm local and global variables, and so on).
Furthermore, the DWARF format would need to be enriched with a few 
Wasm-specific extensions, here detailed: 
https://yurydelendik.github.io/webassembly-dwarf.

This CL only introduces classes ObjectFileWasm, SymbolVendorWasm and 
DynamicLoaderWasmDYLD as a first step to enable LLDB debugging of WebAssembly 
targets. In more detail:

- Class **ObjectFileWasm** represents a WebAssembly module loaded in the 
debuggee process. It knows how to parse Wasm module and store the Code section 
and the DWARF-specific sections.

- Class **SymbolVendorWasm** takes care of DWARF data for a ObjectFileWasm, 
which can be  either embedded in the modules, or stripped into a separate Wasm 
module which only contains DWARF sections.

- In WebAssembly, linear memory is disjointed from code space. The VM can load 
multiple instances of a module, which logically share the same code.

Class **DynamicLoaderWasmDYLD** manages the mapping of Wasm code address in the 
debuggee address space.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71575

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/WASM/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.h
  lldb/source/Plugins/SymbolVendor/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/WASM/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/WASM/SymbolVendorWasm.cpp
  lldb/source/Plugins/SymbolVendor/WASM/SymbolVendorWasm.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/unittests/ObjectFile/CMakeLists.txt
  lldb/unittests/ObjectFile/WASM/CMakeLists.txt
  lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp
  llvm/include/llvm/BinaryFormat/ELF.h

Index: llvm/include/llvm/BinaryFormat/ELF.h
===
--- llvm/include/llvm/BinaryFormat/ELF.h
+++ llvm/include/llvm/BinaryFormat/ELF.h
@@ -311,6 +311,7 @@
   EM_RISCV = 243, // RISC-V
   EM_LANAI = 244, // Lanai 32-bit processor
   EM_BPF = 247,   // Linux kernel bpf virtual machine
+  EM_WASM = 248,  // WebAssembly
 };
 
 // Object file classes.
Index: lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp
===
--- /dev/null
+++ lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp
@@ -0,0 +1,282 @@
+//===-- TestObjectFileWasm.cpp ---*- C++
+//-*-===//
+//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/ObjectFile/WASM/ObjectFileWasm.h"
+#include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+class ObjectFileWasmTest : public testing::Test {
+public:
+  void Se

[Lldb-commits] [PATCH] D71575: [LLDB] Add initial support for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 234463.
paolosev marked 2 inline comments as done.
paolosev removed a project: LLVM.
paolosev added a comment.

Addressed first review comments.


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

https://reviews.llvm.org/D71575

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/WASM/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.h
  lldb/source/Plugins/SymbolVendor/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/WASM/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/WASM/SymbolVendorWasm.cpp
  lldb/source/Plugins/SymbolVendor/WASM/SymbolVendorWasm.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/unittests/ObjectFile/CMakeLists.txt
  lldb/unittests/ObjectFile/WASM/CMakeLists.txt
  lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp

Index: lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp
===
--- /dev/null
+++ lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp
@@ -0,0 +1,281 @@
+//===-- TestObjectFileWasm.cpp --*- C++ -*-===//
+//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/ObjectFile/WASM/ObjectFileWasm.h"
+#include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+class ObjectFileWasmTest : public testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+wasm::ObjectFileWasm::Initialize();
+  }
+
+  void TearDown() override {
+wasm::ObjectFileWasm::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+
+TEST_F(ObjectFileWasmTest, SectionsResolveConsistently) {
+  auto ExpectedFile = TestFile::fromYaml(R"(
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+  - Type:TYPE
+Signatures:
+  - Index:   0
+ParamTypes:
+  - I32
+ReturnTypes:
+  - I32
+  - Type:FUNCTION
+FunctionTypes:   [ 0 ]
+  - Type:TABLE
+Tables:
+  - ElemType:FUNCREF
+Limits:
+  Flags:   [ HAS_MAX ]
+  Initial: 0x0001
+  Maximum: 0x0001
+  - Type:MEMORY
+Memories:
+  - Initial: 0x0002
+  - Type:GLOBAL
+Globals:
+  - Index:   0
+Type:I32
+Mutable: true
+InitExpr:
+  Opcode:  I32_CONST
+  Value:   66560
+  - Type:EXPORT
+Exports:
+  - Name:memory
+Kind:MEMORY
+Index:   0
+  - Name:square
+Kind:FUNCTION
+Index:   0
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:238080808000210141102102200120026B21032003200036020C200328020C2104200328020C2105200420056C210620060F0B
+  - Type:CUSTOM
+Name:name
+FunctionNames:
+  - Index:   0
+Name:square
+  - Type:CUSTOM
+Name:producers
+Languages:
+  - Name:C99
+Version: ''
+Tools:
+  - Name:clang
+Version: '10.0.0'
+  - Type:CUSTOM
+Name:external_debug_info
+Payload: 0F7371756172655F73796D2E7761736D
+...
+)");
+
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  ArchSpec arch("wasm32-unknown-unknown-wasm");
+  Platform::SetHostPlatfor

[Lldb-commits] [PATCH] D71575: [LLDB] Add initial support for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev marked 43 inline comments as done.
paolosev added a comment.

Thanks for all the comments! I am updating the code following your suggestions.
Next step will be to split this into three distinct patches, as suggested by 
Pavel.




Comment at: 
lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.cpp:1
+//===-- DynamicLoaderWasmDYLD.cpp *- C++
+//-*-===//

aprantl wrote:
> 1. This hangs over the line
> 2. a -*- C++ -*- is only necessary for .h files where C vs. C++ is ambiguous
> 
Fixed, but I see the -*- C++ -*- in all files.



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:1
+//===-- ObjectFileWasm.cpp  -*- C++ -*-===//
+//

aprantl wrote:
> ditto
But all the C++ files seem to have "-*- C++ -*-===//" in the first line.



Comment at: lldb/source/Utility/ArchSpec.cpp:107
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm, ArchSpec::eCore_arm_armv8l,
+ "armv8l"},
 {eByteOrderLittle, 4, 4, 4, llvm::Triple::aarch64_32,

sbc100 wrote:
> Is this just clang format being greedy?
Yes, it was edited by clang format.



Comment at: llvm/include/llvm/BinaryFormat/ELF.h:314
   EM_BPF = 247,   // Linux kernel bpf virtual machine
+  EM_WASM = 248,  // WebAssembly
 };

labath wrote:
> sbc100 wrote:
> > This seems like an odd place to add this, given that are not using or 
> > relying on ELF anywhere.  Does this make sense?
> Indeed, that looks very unexpected, and begs an explanation.
Oops... my mistake. This is a relic from an old version where I expected Wasm 
stripped debug symbol might also come in an ELF file. Removed.


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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 234471.
paolosev retitled this revision from "[LLDB] Add initial support for 
WebAssembly debugging" to "[LLDB] Add ObjectFileWasm plugin for WebAssembly 
debugging".
paolosev edited the summary of this revision.

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

https://reviews.llvm.org/D71575

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/unittests/ObjectFile/CMakeLists.txt
  lldb/unittests/ObjectFile/wasm/CMakeLists.txt
  lldb/unittests/ObjectFile/wasm/TestObjectFileWasm.cpp

Index: lldb/unittests/ObjectFile/wasm/TestObjectFileWasm.cpp
===
--- /dev/null
+++ lldb/unittests/ObjectFile/wasm/TestObjectFileWasm.cpp
@@ -0,0 +1,281 @@
+//===-- TestObjectFileWasm.cpp --*- C++ -*-===//
+//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/ObjectFile/WASM/ObjectFileWasm.h"
+#include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+class ObjectFileWasmTest : public testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+wasm::ObjectFileWasm::Initialize();
+  }
+
+  void TearDown() override {
+wasm::ObjectFileWasm::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+
+TEST_F(ObjectFileWasmTest, SectionsResolveConsistently) {
+  auto ExpectedFile = TestFile::fromYaml(R"(
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+  - Type:TYPE
+Signatures:
+  - Index:   0
+ParamTypes:
+  - I32
+ReturnTypes:
+  - I32
+  - Type:FUNCTION
+FunctionTypes:   [ 0 ]
+  - Type:TABLE
+Tables:
+  - ElemType:FUNCREF
+Limits:
+  Flags:   [ HAS_MAX ]
+  Initial: 0x0001
+  Maximum: 0x0001
+  - Type:MEMORY
+Memories:
+  - Initial: 0x0002
+  - Type:GLOBAL
+Globals:
+  - Index:   0
+Type:I32
+Mutable: true
+InitExpr:
+  Opcode:  I32_CONST
+  Value:   66560
+  - Type:EXPORT
+Exports:
+  - Name:memory
+Kind:MEMORY
+Index:   0
+  - Name:square
+Kind:FUNCTION
+Index:   0
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:238080808000210141102102200120026B21032003200036020C200328020C2104200328020C2105200420056C210620060F0B
+  - Type:CUSTOM
+Name:name
+FunctionNames:
+  - Index:   0
+Name:square
+  - Type:CUSTOM
+Name:producers
+Languages:
+  - Name:C99
+Version: ''
+Tools:
+  - Name:clang
+Version: '10.0.0'
+  - Type:CUSTOM
+Name:external_debug_info
+Payload: 0F7371756172655F73796D2E7761736D
+...
+)");
+
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  ArchSpec arch("wasm32-unknown-unknown-wasm");
+  Platform::SetHostPlatform(
+  platform_gdb_server::PlatformRemoteGDBServer::CreateInstance(true,
+   &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  TargetSP target_sp;
+  PlatformSP platform_sp;
+  Status error = debugger_sp->GetTargetList().CreateTarget(
+  *debugger_sp, "", arch, eLoadDependentsNo, platform

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 234474.
paolosev set the repository for this revision to rG LLVM Github Monorepo.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/unittests/ObjectFile/CMakeLists.txt
  lldb/unittests/ObjectFile/wasm/CMakeLists.txt
  lldb/unittests/ObjectFile/wasm/TestObjectFileWasm.cpp

Index: lldb/unittests/ObjectFile/wasm/TestObjectFileWasm.cpp
===
--- /dev/null
+++ lldb/unittests/ObjectFile/wasm/TestObjectFileWasm.cpp
@@ -0,0 +1,280 @@
+//===-- TestObjectFileWasm.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/ObjectFile/WASM/ObjectFileWasm.h"
+#include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+class ObjectFileWasmTest : public testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+wasm::ObjectFileWasm::Initialize();
+  }
+
+  void TearDown() override {
+wasm::ObjectFileWasm::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+
+TEST_F(ObjectFileWasmTest, SectionsResolveConsistently) {
+  auto ExpectedFile = TestFile::fromYaml(R"(
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+  - Type:TYPE
+Signatures:
+  - Index:   0
+ParamTypes:
+  - I32
+ReturnTypes:
+  - I32
+  - Type:FUNCTION
+FunctionTypes:   [ 0 ]
+  - Type:TABLE
+Tables:
+  - ElemType:FUNCREF
+Limits:
+  Flags:   [ HAS_MAX ]
+  Initial: 0x0001
+  Maximum: 0x0001
+  - Type:MEMORY
+Memories:
+  - Initial: 0x0002
+  - Type:GLOBAL
+Globals:
+  - Index:   0
+Type:I32
+Mutable: true
+InitExpr:
+  Opcode:  I32_CONST
+  Value:   66560
+  - Type:EXPORT
+Exports:
+  - Name:memory
+Kind:MEMORY
+Index:   0
+  - Name:square
+Kind:FUNCTION
+Index:   0
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:238080808000210141102102200120026B21032003200036020C200328020C2104200328020C2105200420056C210620060F0B
+  - Type:CUSTOM
+Name:name
+FunctionNames:
+  - Index:   0
+Name:square
+  - Type:CUSTOM
+Name:producers
+Languages:
+  - Name:C99
+Version: ''
+Tools:
+  - Name:clang
+Version: '10.0.0'
+  - Type:CUSTOM
+Name:external_debug_info
+Payload: 0F7371756172655F73796D2E7761736D
+...
+)");
+
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  ArchSpec arch("wasm32-unknown-unknown-wasm");
+  Platform::SetHostPlatform(
+  platform_gdb_server::PlatformRemoteGDBServer::CreateInstance(true,
+   &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  TargetSP target_sp;
+  PlatformSP platform_sp;
+  Status error = debugger_sp->GetTargetList().CreateTarget(
+  *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp);
+  ASSERT_TRUE(target_sp);
+  ASSERT_TRUE(target_sp->GetArchitecture().IsValid(

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 234920.
paolosev marked 7 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/unittests/ObjectFile/CMakeLists.txt
  lldb/unittests/ObjectFile/wasm/CMakeLists.txt
  lldb/unittests/ObjectFile/wasm/TestObjectFileWasm.cpp

Index: lldb/unittests/ObjectFile/wasm/TestObjectFileWasm.cpp
===
--- /dev/null
+++ lldb/unittests/ObjectFile/wasm/TestObjectFileWasm.cpp
@@ -0,0 +1,280 @@
+//===-- TestObjectFileWasm.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/ObjectFile/WASM/ObjectFileWasm.h"
+#include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+class ObjectFileWasmTest : public testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+wasm::ObjectFileWasm::Initialize();
+  }
+
+  void TearDown() override {
+wasm::ObjectFileWasm::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+
+TEST_F(ObjectFileWasmTest, SectionsResolveConsistently) {
+  auto ExpectedFile = TestFile::fromYaml(R"(
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+  - Type:TYPE
+Signatures:
+  - Index:   0
+ParamTypes:
+  - I32
+ReturnTypes:
+  - I32
+  - Type:FUNCTION
+FunctionTypes:   [ 0 ]
+  - Type:TABLE
+Tables:
+  - ElemType:FUNCREF
+Limits:
+  Flags:   [ HAS_MAX ]
+  Initial: 0x0001
+  Maximum: 0x0001
+  - Type:MEMORY
+Memories:
+  - Initial: 0x0002
+  - Type:GLOBAL
+Globals:
+  - Index:   0
+Type:I32
+Mutable: true
+InitExpr:
+  Opcode:  I32_CONST
+  Value:   66560
+  - Type:EXPORT
+Exports:
+  - Name:memory
+Kind:MEMORY
+Index:   0
+  - Name:square
+Kind:FUNCTION
+Index:   0
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:238080808000210141102102200120026B21032003200036020C200328020C2104200328020C2105200420056C210620060F0B
+  - Type:CUSTOM
+Name:name
+FunctionNames:
+  - Index:   0
+Name:square
+  - Type:CUSTOM
+Name:producers
+Languages:
+  - Name:C99
+Version: ''
+Tools:
+  - Name:clang
+Version: '10.0.0'
+  - Type:CUSTOM
+Name:external_debug_info
+Payload: 0F7371756172655F73796D2E7761736D
+...
+)");
+
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  ArchSpec arch("wasm32-unknown-unknown-wasm");
+  Platform::SetHostPlatform(
+  platform_gdb_server::PlatformRemoteGDBServer::CreateInstance(true,
+   &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  TargetSP target_sp;
+  PlatformSP platform_sp;
+  Status error = debugger_sp->GetTargetList().CreateTarget(
+  *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp);
+  ASSERT_TRUE(target_sp);
+  ASSERT_TRUE(target_sp->GetArchitecture().IsValid());
+  ASSERT_TRUE(platform_sp)

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:45
+
+/// Reads a LEB128 variable-length unsigned integer, limited to 7 bits.
+llvm::Optional GetVaruint7(DataExtractor §ion_header_data,

aprantl wrote:
> The LLVM coding style requests that doxygen comments should be on the 
> declaration in the header file and not in the implementation.
This function is not declared in the header. Probably it doesn't need a doxygen 
comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev marked 10 inline comments as done.
paolosev added a comment.

In D71575#1793791 , @labath wrote:

> A bunch more comments from me. :)
>
> A higher level question I have is whether there's something suitable within 
> llvm for parsing wasm object files that could be reused. I know this can be 
> tricky with files loaded from memory etc., so it's fine if it isn't possible 
> to do that, but I am wondering if you have considered this option.


I have considered this option, there is indeed some code duplication because 
there is already a Wasm parser in class WasmObjectFile 
(llvm/include/llvm/Object/Wasm.h).
However class WasmObjectFile is initialized with a memory buffer that contains 
the whole module, and this is not ideal. LLDB might create an ObjectFileWasm 
either from a file or by retrieving specific module chunks from a remote, but 
it doesn't often need to load the whole module in memory.
I think this is the reason why other kind of object files (like ObjectfileELF) 
are re-implemented in LLDB rather than reusing existing code in LLVM (like 
ELFFile in llvm/include/llvm/Object/ELF.h).




Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:40
+
+  const uint32_t *version = reinterpret_cast(
+  data_sp->GetBytes() + sizeof(llvm::wasm::WasmMagic));

labath wrote:
> labath wrote:
> > This looks like it will cause problems on big endian hosts..
> One way to get around that would be to use something like 
> `llvm::support::read32le`.
Good point, modified to use read32le.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:252
+
+  ModuleSpec spec(file, ArchSpec("wasm32-unknown-unknown-wasm"));
+  specs.Append(spec);

labath wrote:
> I take it that wasm files don't have anything like a build id, uuid or 
> something similar?
Not yet. There is a proposal to add a uuid to wasm modules in a custom section, 
but it's not part of the standard yet.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h:116
+
+  typedef struct section_info {
+lldb::offset_t offset;

labath wrote:
> We don't use this typedef style (except possibly in some old code which we 
> shouldn't emulate).
Yes, this was copied from old code :). Removed. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:46-63
+llvm::Optional GetVaruint7(DataExtractor §ion_header_data,
+lldb::offset_t *offset_ptr) {
+  lldb::offset_t initial_offset = *offset_ptr;
+  uint64_t value = section_header_data.GetULEB128(offset_ptr);
+  if (*offset_ptr == initial_offset || value > 127)
+return llvm::None;
+  return static_cast(value);

labath wrote:
> Maybe merge these and make the maximum width a function argument?
I'd keep the functions separate, it's better if they return different sized 
integers.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:342-343
+section_type,  // Section type.
+sect_info.offset & 0x, // VM address.
+sect_info.size, // VM size in bytes of this section.
+sect_info.offset &

labath wrote:
> Are the debug info sections actually loaded into memory for wasm? Should 
> these be zero (that's what they are for elf)?
Yes, I was thinking that the debug sections should be loaded into memory; not 
sure how this works for ELF, how does the debugger find the debug info in that 
case?



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:382
+
+  DecodeSections(load_address);
+

labath wrote:
> This is strange.. I wouldn't expect that the section decoding logic should 
> depend on the actual address that the object is loaded in memory. Can you 
> explain the reasoning here?
There is a reason for this: DecodeNextSection() calls:
```ReadImageData(offset, size)```
and when the debug info sections are embedded in the wasm module (not loaded 
from a separated symbol file), ReadImageData() calls Process::ReadMemory() 
that, for a GDB remote connection, goes to ProcessGDBRemote::DoReadMemory(). 
Here I pass the offset because it also represents the specific id of the module 
loaded in the target debuggee process, as explained in the comment above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 236237.
paolosev marked 8 inline comments as done.
paolosev added a comment.

Addressed more review comments:

- removed code to manage "external_debug_info" sections; logic for this will be 
implemented in the symbol vendor code.
- modified test code, from unittests to be Shell "lit" tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/wasm/basic.yaml
  lldb/test/Shell/ObjectFile/wasm/debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -56,6 +56,7 @@
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
 #include "Plugins/Platform/Android/PlatformAndroid.h"
 #include "Plugins/Platform/FreeBSD/PlatformFreeBSD.h"
 #include "Plugins/Platform/Linux/PlatformLinux.h"
@@ -152,6 +153,7 @@
   ObjectFileELF::Initialize();
   ObjectFileMachO::Initialize();
   ObjectFilePECOFF::Initialize();
+  wasm::ObjectFileWasm::Initialize();
 
   ScriptInterpreterNone::Initialize();
 
@@ -345,6 +347,7 @@
   ObjectFileELF::Terminate();
   ObjectFileMachO::Terminate();
   ObjectFilePECOFF::Terminate();
+  wasm::ObjectFileWasm::Terminate();
 
   // Now shutdown the common parts, in reverse order.
   SystemInitializerCommon::Terminate();
Index: lldb/test/Shell/ObjectFile/wasm/debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/debug-sections.yaml
@@ -0,0 +1,109 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0x40
+
+# CHECK: Name: code
+# CHECK: Type: code
+# CHECK: VM address: 0x0
+# CHECK: VM size: 56
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x87
+# CHECK: VM size: 80
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0xe7
+# CHECK: VM size: 67
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x138
+# CHECK: VM size: 85
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x19b
+# CHECK: VM size: 167
+
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+  - Type:TYPE
+Signatures:
+  - Index:   0
+ParamTypes:
+  - I32
+ReturnTypes:
+  - I32
+  - Type:FUNCTION
+FunctionTypes:   [ 0 ]
+  - Type:TABLE
+Tables:
+  - ElemType:FUNCREF
+Limits:
+  Flags:   [ HAS_MAX ]
+  Initial: 0x0001
+  Maximum: 0x0001
+  - Type:MEMORY
+Memories:
+  - Initial: 0x0002
+  - Type:GLOBAL
+Globals:
+  - Index:   0
+Type:I32
+Mutable: true
+InitExpr:
+  Opcode:  I32_CONST
+  Value:   66560
+  - Type:EXPORT
+Exports:
+  - Name:memory
+Kind:MEMORY
+Index:   0
+  - Name:square
+Kind:FUNCTION
+Index:   0
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:238080808000210141102102200120026B21032003200036020C200328020C2104200328020C2105200420056C210620060F0B
+  - Type:CUSTOM
+Name:.debug_info
+Payload: 4C00040004010C005D0075000200360002020036009600010148000302230CA100010148049D00050400
+  - Type:CUSTOM
+Name:.debug_abbrev
+Payload: 011101250E1305030E10171B0E11011206022E0111011206030E3A0B3B0B271949133F190305000218030E3A0B3B0B4913042400030E3E0B0B0B00
+  - Type:CUSTOM
+Name:.debug_line
+Payload: 510004002F00010101FB0E0D0001010101000101673A5

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:49
+  lldb::offset_t initial_offset = *offset_ptr;
+  uint64_t value = section_header_data.GetULEB128(offset_ptr);
+  if (*offset_ptr == initial_offset || value > 127)

clayborg wrote:
> Is it ok if we consume more than 1 byte here?  What is the offset points to a 
> larger ULEB, are we ok with advancing the offset by multiple bytes or should 
> we back it up and return llvm::None?
> 
> This might be a good candidate to add to DataExtractor directly as:
> 
> ```
> /// Extract a ULEB128 number with a specified max value. If the extracted 
> value exceeds 
> /// "max_value" the offset will be left unchanged and llvm::None will be 
> returned.
> llvm::Optional DataExtractor::GetULEB128(uint64_t *offset_ptr, 
> uint64_t max_value);
> ```
> 
> There are many places where we extract a uint64_t, but only need a uint16_t 
> (like in the DWARF parser where all DW_TAG_, DW_AT_XXX and DW_FORM_XXX 
> values must only be uint16_t values but are encoded as ULEB128 values. So 
> this could be used elsewhere if we do put it into DataExtractor.
Modified in DataExtractor, as suggested. However we need to return a 
`llvm::Optional`, which can be not ideal because the result could 
need to be casted to another integer type.
We could also male `DataExtractor::GetULEB128` templatized on the integer type, 
getting `max_value` as `std::numeric_limits::max()`, but I don't want to 
overly complicate the code.




Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:162-163
+  // - the actual contents.
+  llvm::Optional section_id =
+  GetVaruint7(section_header_data, &offset);
+  if (!section_id)

clayborg wrote:
> Is this a one byte section ID or is it a ULEB? Not sure why it would be 
> encoded as a ULEB if it is always one byte? IF this really is just a one byte 
> value, then replace with:
> 
> ```
> uint8_t section_id = data.GetU8(&offset);
> ```
You are right, it is a one-bye section. I cannot use `data.GetU8(&offset);` 
though, because it returns 0 on failure. 



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:342-343
+section_type,  // Section type.
+sect_info.offset & 0x, // VM address.
+sect_info.size, // VM size in bytes of this section.
+sect_info.offset &

labath wrote:
> paolosev wrote:
> > labath wrote:
> > > Are the debug info sections actually loaded into memory for wasm? Should 
> > > these be zero (that's what they are for elf)?
> > Yes, I was thinking that the debug sections should be loaded into memory; 
> > not sure how this works for ELF, how does the debugger find the debug info 
> > in that case?
> Are you referring to the load-from-memory, or load-from-file scenario? 
> Normally, the debug info is not loaded into memory, and if lldb is loading 
> the module from a file, then the debug info is loaded from the file (and we 
> use the "file offset" field to locate them). Loading files from memory does 
> not work in general (because we can't find the whole file there -- e.g., 
> section headers are missing). The only case it does work is in case of jitted 
> files. I'm not 100% sure how that works, but given that there is no 
> `if(memory)` block in the section creation code, those sections must too get 
> `vm size = 0`.
> 
> In practice, I don't think it does not matter that much what you put here -- 
> I expect things will mostly work regardless. I am just trying to make this 
> consistent in some way. If these sections are not found in the memory at the 
> address which you set here (possibly adjusted by the load bias) then I think 
> it makes sense to set `vm_size=vm_addr=0`. If they are indeed present there, 
> then setting it to those values is perfectly fine.
Modified, but I am not sure I completely understood the difference of file 
addresses and vm addresses.

In the case of Wasm, we can have two cases:
a) Wasm module contains executable code and also DWARF data embedded in DWARF 
sections
b) Wasm module contains code and has a custom section that points to a 
separated Wasm module that only contains DWARF sections

The file of Wasm modules that contains code should never be loaded directly by 
LLDB; LLDB should use GDB-remote to connect to the Wasm engine that loaded the 
module and retrieve the module content from the engine.
But when the DWARF data has been stripped into a separate Wasm file, that file 
should be directly loaded by LLDB in order to load the debug sections.
So, if I am not wrong, only in the first case we should assume that the debug 
info is loaded into memory, and have vm_size != 0?



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:382
+
+  DecodeSections(load_address);
+

la

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 236868.
paolosev marked 20 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/include/lldb/Utility/DataExtractor.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/source/Utility/DataExtractor.cpp
  lldb/test/Shell/ObjectFile/wasm/basic.yaml
  lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -56,6 +56,7 @@
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
 #include "Plugins/Platform/Android/PlatformAndroid.h"
 #include "Plugins/Platform/FreeBSD/PlatformFreeBSD.h"
 #include "Plugins/Platform/Linux/PlatformLinux.h"
@@ -152,6 +153,7 @@
   ObjectFileELF::Initialize();
   ObjectFileMachO::Initialize();
   ObjectFilePECOFF::Initialize();
+  wasm::ObjectFileWasm::Initialize();
 
   ScriptInterpreterNone::Initialize();
 
@@ -345,6 +347,7 @@
   ObjectFileELF::Terminate();
   ObjectFileMachO::Terminate();
   ObjectFilePECOFF::Terminate();
+  wasm::ObjectFileWasm::Terminate();
 
   // Now shutdown the common parts, in reverse order.
   SystemInitializerCommon::Terminate();
Index: lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
@@ -0,0 +1,54 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0x0
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 3
+
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+
+  - Type:CUSTOM
+Name:.debug_info
+Payload: 4C00
+  - Type:CUSTOM
+Name:.debug_abbrev
+Payload: 0111
+  - Type:CUSTOM
+Name:.debug_line
+Payload: 5100
+  - Type:CUSTOM
+Name:.debug_str
+Payload: 636CFF
+...
Index: lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
@@ -0,0 +1,67 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0xa
+
+# CHECK: Name: code
+# CHECK: Type: code
+# CHECK: VM address: 0x0
+# CHECK: VM size: 56
+# CHECK: File size: 56
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 3
+
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:238080808000210141102102200120026B21032003200036020C200328020C2104200328020C2105200420056C210620060F0B
+  - Type:CUSTOM
+Name:

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev marked 2 inline comments as done.
paolosev added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:342-343
+section_type,  // Section type.
+sect_info.offset & 0x, // VM address.
+sect_info.size, // VM size in bytes of this section.
+sect_info.offset &

labath wrote:
> paolosev wrote:
> > labath wrote:
> > > paolosev wrote:
> > > > labath wrote:
> > > > > Are the debug info sections actually loaded into memory for wasm? 
> > > > > Should these be zero (that's what they are for elf)?
> > > > Yes, I was thinking that the debug sections should be loaded into 
> > > > memory; not sure how this works for ELF, how does the debugger find the 
> > > > debug info in that case?
> > > Are you referring to the load-from-memory, or load-from-file scenario? 
> > > Normally, the debug info is not loaded into memory, and if lldb is 
> > > loading the module from a file, then the debug info is loaded from the 
> > > file (and we use the "file offset" field to locate them). Loading files 
> > > from memory does not work in general (because we can't find the whole 
> > > file there -- e.g., section headers are missing). The only case it does 
> > > work is in case of jitted files. I'm not 100% sure how that works, but 
> > > given that there is no `if(memory)` block in the section creation code, 
> > > those sections must too get `vm size = 0`.
> > > 
> > > In practice, I don't think it does not matter that much what you put here 
> > > -- I expect things will mostly work regardless. I am just trying to make 
> > > this consistent in some way. If these sections are not found in the 
> > > memory at the address which you set here (possibly adjusted by the load 
> > > bias) then I think it makes sense to set `vm_size=vm_addr=0`. If they are 
> > > indeed present there, then setting it to those values is perfectly fine.
> > Modified, but I am not sure I completely understood the difference of file 
> > addresses and vm addresses.
> > 
> > In the case of Wasm, we can have two cases:
> > a) Wasm module contains executable code and also DWARF data embedded in 
> > DWARF sections
> > b) Wasm module contains code and has a custom section that points to a 
> > separated Wasm module that only contains DWARF sections
> > 
> > The file of Wasm modules that contains code should never be loaded directly 
> > by LLDB; LLDB should use GDB-remote to connect to the Wasm engine that 
> > loaded the module and retrieve the module content from the engine.
> > But when the DWARF data has been stripped into a separate Wasm file, that 
> > file should be directly loaded by LLDB in order to load the debug sections.
> > So, if I am not wrong, only in the first case we should assume that the 
> > debug info is loaded into memory, and have vm_size != 0?
> You're right, this is getting pretty confusing, as a lot of the concepts 
> we're talking about here are overloaded (and not even consistently used 
> within lldb). Let me try to elaborate here.
> 
> I'll start by describing various Section fields (I'll use ELF as a reference):
> - file offset (`m_file_offset`) - where the section is physically located in 
> the file. It does not matter if the file is loaded from inferior memory or 
> not (in the former case, this is an offset from location of the first byte of 
> the file). In elf this corresponds to the `sh_offset` field.
> - file size (`m_file_size`) - size of the section in the file. Some sections 
> don't take up any space in the file (`.bss`), so they have this zero. This is 
> roughly the `sh_size` field in elf (but it is adjusted for SHT_NOBITS 
> sections like .bss)
> - file address (`m_file_addr`) - The address where the object file says this 
> section should be loaded to. Note that this may not be the final address due 
> to ASLR or such. It is the job of the SetLoadAddress function to compute the 
> actual final address (which is then called the "load address"). This 
> corresponds to the `sh_addr` field in elf, but it is also sometimes called 
> the "vm address", because of the `p_vaddr` program header field
> - vm size (`m_byte_size) size of the section when it gets loaded into memory. 
> Sections which don't get loaded into memory have this as 0. This is also 
> "rougly" `sh_size`, but only for SHF_ALLOC sections.
> 
> All of these fields are really geared for the case where lldb opens a file 
> from disk, and then uses that to understand the contents of process memory. 
> They become pretty redundant if you have loaded the file from memory, but it 
> should still be possible to assign meaningful values to each of them.
> 
> The file offset+file size combo should reflect the locations of the sections 
> within the file (regardless of whether it's a "real" file or just a chunk of 
> memory). And the file address+vm size combo should reflect t

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev marked an inline comment as done.
paolosev added a comment.

I apologize for the noob question, but how do I schedule a build for this diff 
with Harbormaster?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev created this revision.
paolosev added reviewers: clayborg, labath, aprantl, sbc100, teemperor.
paolosev added a project: LLDB.
Herald added subscribers: lldb-commits, sunfish, aheejin, jgravelle-google, 
mgorny.

Add plugin class SymbolVendorWasm, with the logic to manage debug symbols for 
Wasm modules.

Depends on D71575 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72650

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Plugins/SymbolVendor/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.h

Index: lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.h
===
--- /dev/null
+++ lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.h
@@ -0,0 +1,42 @@
+//===-- SymbolVendorWasm.h --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_SymbolVendorWasm_h_
+#define liblldb_SymbolVendorWasm_h_
+
+#include "lldb/Symbol/SymbolVendor.h"
+#include "lldb/lldb-private.h"
+
+namespace lldb_private {
+namespace wasm {
+
+class SymbolVendorWasm : public lldb_private::SymbolVendor {
+public:
+  SymbolVendorWasm(const lldb::ModuleSP &module_sp);
+
+  static void Initialize();
+  static void Terminate();
+  static lldb_private::ConstString GetPluginNameStatic();
+  static const char *GetPluginDescriptionStatic();
+
+  static lldb_private::SymbolVendor *
+  CreateInstance(const lldb::ModuleSP &module_sp,
+ lldb_private::Stream *feedback_strm);
+
+  // PluginInterface protocol
+  lldb_private::ConstString GetPluginName() override;
+  uint32_t GetPluginVersion() override;
+
+private:
+  DISALLOW_COPY_AND_ASSIGN(SymbolVendorWasm);
+};
+
+} // namespace wasm
+} // namespace lldb_private
+
+#endif // liblldb_SymbolVendorWasm_h_
Index: lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
===
--- /dev/null
+++ lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
@@ -0,0 +1,145 @@
+//===-- SymbolVendorWasm.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SymbolVendorWasm.h"
+
+#include 
+
+#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/Host.h"
+#include "lldb/Symbol/LocateSymbolFile.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/StreamString.h"
+#include "lldb/Utility/Timer.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::wasm;
+
+// SymbolVendorWasm constructor
+SymbolVendorWasm::SymbolVendorWasm(const lldb::ModuleSP &module_sp)
+: SymbolVendor(module_sp) {}
+
+void SymbolVendorWasm::Initialize() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+GetPluginDescriptionStatic(), CreateInstance);
+}
+
+void SymbolVendorWasm::Terminate() {
+  PluginManager::UnregisterPlugin(CreateInstance);
+}
+
+lldb_private::ConstString SymbolVendorWasm::GetPluginNameStatic() {
+  static ConstString g_name("WASM");
+  return g_name;
+}
+
+const char *SymbolVendorWasm::GetPluginDescriptionStatic() {
+  return "Symbol vendor for WASM that looks for dwo files that match "
+ "executables.";
+}
+
+// CreateInstance
+//
+// Platforms can register a callback to use when creating symbol vendors to
+// allow for complex debug information file setups, and to also allow for
+// finding separate debug information files.
+SymbolVendor *
+SymbolVendorWasm::CreateInstance(const lldb::ModuleSP &module_sp,
+ lldb_private::Stream *feedback_strm) {
+  if (!module_sp)
+return nullptr;
+
+  ObjectFileWasm *obj_file =
+  llvm::dyn_cast_or_null(module_sp->GetObjectFile());
+  if (!obj_file)
+return nullptr;
+
+  // If the main object file already contains debug info, then we are done.
+  if (obj_file->GetSectionList()->FindSectionByType(
+  lldb::eSectionTypeDWARFDebugInfo, true))
+return nullptr;
+
+  static Timer::Category f

[Lldb-commits] [PATCH] D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 238128.
paolosev added a comment.

Added "lldb-test object-file" test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72650

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Plugins/SymbolVendor/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.h
  lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
  lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
  lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -76,6 +76,7 @@
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
 #include "Plugins/SymbolVendor/ELF/SymbolVendorELF.h"
+#include "Plugins/SymbolVendor/wasm/SymbolVendorWasm.h"
 #include "Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.h"
 #include "Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h"
 #include "Plugins/UnwindAssembly/x86/UnwindAssembly-x86.h"
@@ -201,6 +202,7 @@
   SymbolFileDWARF::Initialize();
   SymbolFilePDB::Initialize();
   SymbolFileSymtab::Initialize();
+  wasm::SymbolVendorWasm::Initialize();
   UnwindAssemblyInstEmulation::Initialize();
   UnwindAssembly_x86::Initialize();
   EmulateInstructionARM64::Initialize();
@@ -288,6 +290,7 @@
   SymbolFileDWARF::Terminate();
   SymbolFilePDB::Terminate();
   SymbolFileSymtab::Terminate();
+  wasm::SymbolVendorWasm::Terminate();
   UnwindAssembly_x86::Terminate();
   UnwindAssemblyInstEmulation::Terminate();
   EmulateInstructionARM64::Terminate();
Index: lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
@@ -0,0 +1,50 @@
+# RUN: rm -rf %t.dir
+# RUN: mkdir %t.dir
+# RUN: cd %t.dir
+# RUN: yaml2obj %p/Inputs/wasm-external_debug_info.yaml > %t.dir/test.wasm
+# RUN: yaml2obj %p/Inputs/wasm-stripped-debug-info.yaml > %t.dir/test_sym.wasm
+# RUN: lldb-test object-file %t.dir/test.wasm | FileCheck %s
+
+# This test checks that SymbolVendorWasm correctly loads DWARF debug sections
+# that have been stripped out into a separated Wasm module. The original Wasm
+# module contains a "external_debug_info" custom section with the absolute or
+# relative path of the debug module.
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0xa
+
+# CHECK: Name: code
+# CHECK: Type: code
+# CHECK: VM address: 0x0
+# CHECK: VM size: 56
+# CHECK: File size: 56
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 3
\ No newline at end of file
Index: lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
@@ -0,0 +1,18 @@
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+
+  - Type:CUSTOM
+Name:.debug_info
+Payload: 4C00
+  - Type:CUSTOM
+Name:.debug_abbrev
+Payload: 0111
+  - Type:CUSTOM
+Name:.debug_line
+Payload: 5100
+  - Type:CUSTOM
+Name:.debug_str
+Payload: 636CFF
+...
Index: lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
@@ -0,0 +1,15 @@
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:23808080800021014110210220

[Lldb-commits] [PATCH] D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D72650#1819403 , @labath wrote:

> The patch looks pretty good. A reasonable way to test this would be again via 
> `lldb-test object-file` . The command dumps the "unified section list" of the 
> module, so if the debug info sections show up there, you know the symbol 
> vendor has done it's job. You can look at 
> `test/Shell/ObjectFile/ELF/build-id-case.yaml` for inspiration.


Thank you for the suggestion! I added a test. 
I would have liked to use `llvm-objcopy --strip-all` in my test, but 
llvm-objcopy does not support wasm yet (I started 
 working on this feature but I found out that 
there was already an ongoing effort: https://reviews.llvm.org/D70930, and 
https://reviews.llvm.org/D70970) .
So I created with two separated yaml files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72650



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 238168.
paolosev marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/wasm/basic.yaml
  lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -56,6 +56,7 @@
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
 #include "Plugins/Platform/Android/PlatformAndroid.h"
 #include "Plugins/Platform/FreeBSD/PlatformFreeBSD.h"
 #include "Plugins/Platform/Linux/PlatformLinux.h"
@@ -152,6 +153,7 @@
   ObjectFileELF::Initialize();
   ObjectFileMachO::Initialize();
   ObjectFilePECOFF::Initialize();
+  wasm::ObjectFileWasm::Initialize();
 
   ScriptInterpreterNone::Initialize();
 
@@ -345,6 +347,7 @@
   ObjectFileELF::Terminate();
   ObjectFileMachO::Terminate();
   ObjectFilePECOFF::Terminate();
+  wasm::ObjectFileWasm::Terminate();
 
   // Now shutdown the common parts, in reverse order.
   SystemInitializerCommon::Terminate();
Index: lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
@@ -0,0 +1,54 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0x0
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 3
+
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+
+  - Type:CUSTOM
+Name:.debug_info
+Payload: 4C00
+  - Type:CUSTOM
+Name:.debug_abbrev
+Payload: 0111
+  - Type:CUSTOM
+Name:.debug_line
+Payload: 5100
+  - Type:CUSTOM
+Name:.debug_str
+Payload: 636CFF
+...
Index: lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
@@ -0,0 +1,67 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0xa
+
+# CHECK: Name: code
+# CHECK: Type: code
+# CHECK: VM address: 0x0
+# CHECK: VM size: 56
+# CHECK: File size: 56
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 3
+
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:238080808000210141102102200120026B21032003200036020C200328020C2104200328020C2105200420056C210620060F0B
+  - Type:CUSTOM
+Name:.debug_info
+Payload: 4C00
+  - Type:CUSTOM
+Name:   

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added inline comments.



Comment at: lldb/source/Utility/DataExtractor.cpp:914
+/// Extract an unsigned LEB128 number with a specified max value. If the
+/// extracted value exceeds "max_value" the offset will be left unchanged and
+/// llvm::None will be returned.

labath wrote:
> It doesn't look like this actually happens, does it? (If max_value is 
> exceeded, the offset will still be updated, right?).
> 
> And overall, I am not very happy with backdooring an api inconsistent with 
> the rest of the DataExtractor (I am aware it was clayborg's idea). Overall, 
> it would probably be better to use the llvm DataExtractor class, which has 
> the Cursor interface designed to solve some of the problems you have here (it 
> can handle EOF, it cannot check the uleb magnitude). And it tries to minimize 
> the number of times you need to error check everything. The usage of it could 
> be something like:
> ```
> llvm::DataExtractor llvm_data = lldb_data.GetAsLLVM();
> llvm::DataExtractor::Cursor c(0);
> unsigned id = llvm_data.GetU8(c);
> unsigned payload_len = llvm_data.GetULEB128(c);
> if (!c)
>   return c.takeError();
> // id and payload_len are valid here
> if (id == 0) {
>   unsigned name_len = llvm_data.GetULEB128(c);
>   SmallVector name_storage;
>   llvm_data.GetU8(c, name_storage, name_len);
>   if (!c)
> return c.takeError();
>   // name_len and name valid here
>   StringRef name = toStringRef(makeArrayRef(name_storage));
>   unsigned section_length = ...;
>   m_sect_infos.push_back(...)
> }
> ```
> This won't handle the uleb magnitude check, but these checks seem irrelevant 
> and/or subsumable by other, more useful checks: a) Checking the name length 
> is not necessary, as the code will fail for any names longer 1024 anyway (as 
> that's the amount of data you read); b) instead of `section_len < 2^32` it 
> seems more useful to check that `*offset_ptr + section_len` is less than 
> `2^32`, to make sure we don't wrap the `module_id` part of the "address".
Good points! Changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 238177.
paolosev added a comment.

Rebasing from D71575 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72650

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Plugins/SymbolVendor/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.h
  lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
  lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
  lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -76,6 +76,7 @@
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
 #include "Plugins/SymbolVendor/ELF/SymbolVendorELF.h"
+#include "Plugins/SymbolVendor/wasm/SymbolVendorWasm.h"
 #include "Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.h"
 #include "Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h"
 #include "Plugins/UnwindAssembly/x86/UnwindAssembly-x86.h"
@@ -201,6 +202,7 @@
   SymbolFileDWARF::Initialize();
   SymbolFilePDB::Initialize();
   SymbolFileSymtab::Initialize();
+  wasm::SymbolVendorWasm::Initialize();
   UnwindAssemblyInstEmulation::Initialize();
   UnwindAssembly_x86::Initialize();
   EmulateInstructionARM64::Initialize();
@@ -288,6 +290,7 @@
   SymbolFileDWARF::Terminate();
   SymbolFilePDB::Terminate();
   SymbolFileSymtab::Terminate();
+  wasm::SymbolVendorWasm::Terminate();
   UnwindAssembly_x86::Terminate();
   UnwindAssemblyInstEmulation::Terminate();
   EmulateInstructionARM64::Terminate();
Index: lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
@@ -0,0 +1,50 @@
+# RUN: rm -rf %t.dir
+# RUN: mkdir %t.dir
+# RUN: cd %t.dir
+# RUN: yaml2obj %p/Inputs/wasm-external_debug_info.yaml > %t.dir/test.wasm
+# RUN: yaml2obj %p/Inputs/wasm-stripped-debug-info.yaml > %t.dir/test_sym.wasm
+# RUN: lldb-test object-file %t.dir/test.wasm | FileCheck %s
+
+# This test checks that SymbolVendorWasm correctly loads DWARF debug sections
+# that have been stripped out into a separated Wasm module. The original Wasm
+# module contains a "external_debug_info" custom section with the absolute or
+# relative path of the debug module.
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0xa
+
+# CHECK: Name: code
+# CHECK: Type: code
+# CHECK: VM address: 0x0
+# CHECK: VM size: 56
+# CHECK: File size: 56
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 3
\ No newline at end of file
Index: lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-stripped-debug-info.yaml
@@ -0,0 +1,18 @@
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+
+  - Type:CUSTOM
+Name:.debug_info
+Payload: 4C00
+  - Type:CUSTOM
+Name:.debug_abbrev
+Payload: 0111
+  - Type:CUSTOM
+Name:.debug_line
+Payload: 5100
+  - Type:CUSTOM
+Name:.debug_str
+Payload: 636CFF
+...
Index: lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/Inputs/wasm-external_debug_info.yaml
@@ -0,0 +1,15 @@
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:238080

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

What is the best way to test this class?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev created this revision.
paolosev added reviewers: labath, clayborg, aprantl, sbc100, teemperor.
paolosev added a project: LLDB.
Herald added subscribers: lldb-commits, sunfish, aheejin, jgravelle-google, 
mgorny.
paolosev added a comment.

What is the best way to test this class?


Add a dynamic loader plug-in class for WebAssembly modules.

Depends on D72650 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72751

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h

Index: lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
===
--- /dev/null
+++ lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
@@ -0,0 +1,52 @@
+//===-- DynamicLoaderWasmDYLD.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_Plugins_DynamicLoaderWasmDYLD_h_
+#define liblldb_Plugins_DynamicLoaderWasmDYLD_h_
+
+#include "lldb/Target/DynamicLoader.h"
+
+namespace lldb_private {
+namespace wasm {
+
+class DynamicLoaderWasmDYLD : public DynamicLoader {
+public:
+  DynamicLoaderWasmDYLD(Process *process);
+
+  static void Initialize();
+  static void Terminate() {}
+
+  static ConstString GetPluginNameStatic();
+  static const char *GetPluginDescriptionStatic();
+
+  static DynamicLoader *CreateInstance(Process *process, bool force);
+
+  /// DynamicLoader
+  /// \{
+  lldb::ModuleSP LoadModuleAtAddress(const lldb_private::FileSpec &file,
+ lldb::addr_t link_map_addr,
+ lldb::addr_t base_addr,
+ bool base_addr_is_offset) override;
+  void DidAttach() override;
+  void DidLaunch() override {}
+  Status CanLoadImage() override { return Status(); }
+  lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
+  bool stop) override;
+  /// \}
+
+  /// PluginInterface protocol.
+  /// \{
+  ConstString GetPluginName() override { return GetPluginNameStatic(); }
+  uint32_t GetPluginVersion() override { return 1; }
+  /// \}
+};
+
+} // namespace wasm
+} // namespace lldb_private
+
+#endif // liblldb_Plugins_DynamicLoaderWasmDYLD_h_
Index: lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
===
--- /dev/null
+++ lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
@@ -0,0 +1,132 @@
+//===-- DynamicLoaderWasmDYLD.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "DynamicLoaderWasmDYLD.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/Log.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::wasm;
+
+DynamicLoaderWasmDYLD::DynamicLoaderWasmDYLD(Process *process)
+: DynamicLoader(process) {}
+
+void DynamicLoaderWasmDYLD::Initialize() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+GetPluginDescriptionStatic(), CreateInstance);
+}
+
+ConstString DynamicLoaderWasmDYLD::GetPluginNameStatic() {
+  static ConstString g_plugin_name("wasm-dyld");
+  return g_plugin_name;
+}
+
+const char *DynamicLoaderWasmDYLD::GetPluginDescriptionStatic() {
+  return "Dynamic loader plug-in that watches for shared library "
+ "loads/unloads in WebAssembly engines.";
+}
+
+DynamicLoader *DynamicLoaderWasmDYLD::CreateInstance(Process *process,
+ bool force) {
+  bool should_create = force;
+  if (!should_create) {
+should_create =
+(process->GetTarget().GetArchitecture().GetTriple().getArch() ==
+ llvm::Triple::wasm32);
+  }
+
+  if (should_create)
+return new DynamicLoaderWasmDYLD(process);
+
+  return nullptr;
+}
+
+/// In WebAssembly, linear memory is disjointed from code space. The VM can load
+/// multiple instances of a module, which logically share the same code.
+/// Currently we only support wa

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D71575#1821312 , @labath wrote:

> Thanks. I think this is looking very good now. Excited to have this ready.
>
> Do you have commit access?


No, I certainly don't have commit access, this would be my first accepted 
patch. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 238342.
paolosev added a comment.

Modified test to have two "inlined" yaml files and use "yaml2obj --docnum".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72650

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Plugins/SymbolVendor/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
  lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.h
  lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -76,6 +76,7 @@
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
 #include "Plugins/SymbolVendor/ELF/SymbolVendorELF.h"
+#include "Plugins/SymbolVendor/wasm/SymbolVendorWasm.h"
 #include "Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.h"
 #include "Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h"
 #include "Plugins/UnwindAssembly/x86/UnwindAssembly-x86.h"
@@ -201,6 +202,7 @@
   SymbolFileDWARF::Initialize();
   SymbolFilePDB::Initialize();
   SymbolFileSymtab::Initialize();
+  wasm::SymbolVendorWasm::Initialize();
   UnwindAssemblyInstEmulation::Initialize();
   UnwindAssembly_x86::Initialize();
   EmulateInstructionARM64::Initialize();
@@ -288,6 +290,7 @@
   SymbolFileDWARF::Terminate();
   SymbolFilePDB::Terminate();
   SymbolFileSymtab::Terminate();
+  wasm::SymbolVendorWasm::Terminate();
   UnwindAssembly_x86::Terminate();
   UnwindAssemblyInstEmulation::Terminate();
   EmulateInstructionARM64::Terminate();
Index: lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
@@ -0,0 +1,85 @@
+# RUN: yaml2obj -docnum=1 %s > test.wasm
+# RUN: yaml2obj -docnum=2 %s > test_sym.wasm
+# RUN: lldb-test object-file test.wasm | FileCheck %s
+
+# This test checks that SymbolVendorWasm correctly loads DWARF debug sections
+# that have been stripped out into a separated Wasm module. The original Wasm
+# module contains a "external_debug_info" custom section with the absolute or
+# relative path of the debug module.
+
+# CHECK: Plugin name: wasm
+# CHECK: Architecture: wasm32-unknown-unknown-wasm
+# CHECK: UUID: 
+# CHECK: Executable: true
+# CHECK: Stripped: true
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK: Base VM address: 0xa
+
+# CHECK: Name: code
+# CHECK: Type: code
+# CHECK: VM address: 0x0
+# CHECK: VM size: 56
+# CHECK: File size: 56
+
+# CHECK: Name: .debug_info
+# CHECK: Type: dwarf-info
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_abbrev
+# CHECK: Type: dwarf-abbrev
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_line
+# CHECK: Type: dwarf-line
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 2
+
+# CHECK: Name: .debug_str
+# CHECK: Type: dwarf-str
+# CHECK: VM address: 0x0
+# CHECK: VM size: 0
+# CHECK: File size: 3
+
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:238080808000210141102102200120026B21032003200036020C200328020C2104200328020C2105200420056C210620060F0B
+  - Type:CUSTOM
+Name:external_debug_info
+Payload: 0D746573745F73796D2E7761736D  # test_sym.wasm
+
+...
+
+
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+
+  - Type:CUSTOM
+Name:.debug_info
+Payload: 4C00
+  - Type:CUSTOM
+Name:.debug_abbrev
+Payload: 0111
+  - Type:CUSTOM
+Name:.debug_line
+Payload: 5100
+  - Type:CUSTOM
+Name:.debug_str
+Payload: 636CFF
+
+...
Index: lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.h
===
--- /dev/null
+++ lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.h
@@ -0,0 +1,44 @@
+//===-- SymbolVendorWasm.h --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D71575#1823252 , @jingham wrote:

> BTW, I had to fix this patch (cd9e5c32302cd3b34b796683eedb072c6a1cfdc1 
> ) to 
> build on macOS.  uint64_t and size_t are differently spelled (though I think 
> otherwise equivalent.)  One is "long long unsigned int", the other "long 
> unsigned int".  I have no idea why that's true, but std::min refuses to 
> compare a size_t and a unit64_t.  Anyway, I fixed this by casting one of the 
> two sides of the comparison.  But this was causing problems because we have 
> an api (ReadImageData) that takes a uint64_t for the offset and a size_t for 
> the size.  That seems a little weird to me, why are these different types?


I am sorry for this problem, thank you for the fix! Evidently size_t can also 
be 32 bit, like in this case
To be more precise ReadImageData should take an lldb::offset_t as first 
argument (which is indeed an uint64_t) and it should use the same type for the 
size; I'll clean up this in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 239784.
paolosev added a comment.

I have verified the logic of the dynamic loader quite carefully, but there are 
a couple of things to clarify.

A Wasm module is loaded at a 64 bit address, where the upper 32 bits are used 
as module identifier. Let’s say that we have a module with Id==4, so it will be 
loaded at address 0x0004`.  Each section is loaded at its relative 
file offset. Therefore if the code section starts at file offset 0x4d in the 
Wasm module, we call:
Target::SetSectionLoadAddress(section_sp, 0x4004d).

The module can also contain embedded DWARF sections, which will also be loaded 
at their relative file offset in the same way. And since there cannot be 
duplicated sections in a module, there is no overlapping, we can always convert 
a load address back into a section.

However, there are two complications.

The first is that we need to call `Target::SetSectionLoadAddress()` twice, from 
two different places. First we need to call `Target::SetSectionLoadAddress()` 
in `ObjectFileWasm::SetLoadAddress()`, and then again in 
`DynamicLoaderWasmDYLD::DidAttach()`. The reason for this seems to originate in 
the sequence of function calls:

In `DynamicLoaderWasmDYLD::DidAttach()` we call 
`ProcessGDBRemote::LoadModules()` to get list of loaded modules from the remote 
(Wasm engine).
`ProcessGDBRemote::LoadModules()` calls, first:

- `DynamicLoaderWasmDYLD::LoadModuleAtAddress()` and from there:
  1. `DynamicLoader::UpdateLoadedSections() -> ObjectFileWasm::SetLoadAddress()`
  2. `Target::GetImages()::AppendIfNeeded(module) -> 
ProcessGDBRemote::ModulesDidLoad() -> JITLoaderList::ModulesDidLoad() -> 
Module::GetSymbolFile() -> SymbolFileDWARF::CalculateAbilities()`. Here we 
initialize the symbols for the module, and set `m_did_load_symfile`, but for 
this to work we need to have already set the load address for each section, in 
the previous `ObjectFileWasm::SetLoadAddress()`.

then:

- `Target::SetExecutableModule() -> Target::ClearModules() -> 
SectionLoadList::Clear()`

So, at the end of `LoadModules()` in `DynamicLoaderWasmDYLD::DidAttach()` the 
SectionLoadList is empty, and we need to set it again by calling 
`Target::.SetSectionLoadAddress()` again. 
This works but the duplication is ugly; is there a way to improve this?
_

The second problem is that the Code Section needs to be initialized (in 
`ObjectFileWasm::CreateSections()`) with `m_file_addr = m_file_offset = 0`, and 
not with the actual file offset of the Code section in the Wasm file. If we set 
`Section::m_file_addr` and `Section::m_file_offset` to the actual code offset, 
the DWARF info does not work correctly.

I have some doubts regarding the DWARF data generated by Clang for a Wasm 
target. Looking at an example, for a Wasm module that has the Code section at 
offset 0x57, I see this DWARF data:

  0x000b: DW_TAG_compile_unit
[…]
DW_AT_low_pc (0x)
DW_AT_ranges (0x
   [0x0002, 0x000e)
   [0x000f, 0x001a)
   [0x001b, 0x0099)
   [0x009b, 0x011c))

The documentation  says 
that //“Wherever a code address is used in DWARF for WebAssembly, it must be 
the offset of an instruction relative within the Code section of the 
WebAssembly file.”// 
But is this correct? Shouldn't maybe code addresses be offset-ed by the file 
address of the Code section?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm.yaml
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -37,6 +37,7 @@
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
 #include "Plugins/Instruction/PPC64/EmulateInstructionPPC64.h"
 #include 

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 240077.
paolosev added a comment.

Thanks for the explanation! I wasn't quite clear on "executable module" here, 
but after your comments I realized that `Target::SetExecutableModule()` should 
not probably be called also for Wasm modules.
The point is that `ObjectFileWasm::CalculateType()` should return 
`eTypeSharedLibrary`, not `eTypeExecutable`.
With this change the first issue is easily solved: we just need to call 
`Target::SetSectionLoadAddress()` once, in `ObjectFileWasm::SetLoadAddress()` 
because `Target::SetExecutableModule() -> Target::ClearModules() -> 
SectionLoadList::Clear()` is not called, and 
`DynamicLoaderWasmDYLD::DidAttach()` can be simplified to just call 
`ProcessGDBRemote::LoadModules()`.
Does this solution work for you? If so, we should look at the second point, the 
need to initialize `m_file_addr = m_file_offset = 0` for the "code" Section in 
order to make the DWARF symbols work...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm.yaml
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -37,6 +37,7 @@
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
 #include "Plugins/Instruction/PPC64/EmulateInstructionPPC64.h"
 #include "Plugins/InstrumentationRuntime/ASan/ASanRuntime.h"
@@ -246,6 +247,7 @@
   DynamicLoaderMacOSXDYLD::Initialize();
   DynamicLoaderMacOS::Initialize();
   DynamicLoaderPOSIXDYLD::Initialize();
+  wasm::DynamicLoaderWasmDYLD::Initialize();
   DynamicLoaderStatic::Initialize();
   DynamicLoaderWindowsDYLD::Initialize();
 
@@ -329,6 +331,7 @@
   DynamicLoaderMacOSXDYLD::Terminate();
   DynamicLoaderMacOS::Terminate();
   DynamicLoaderPOSIXDYLD::Terminate();
+  wasm::DynamicLoaderWasmDYLD::Terminate();
   DynamicLoaderStatic::Terminate();
   DynamicLoaderWindowsDYLD::Terminate();
 
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -1,153 +1,154 @@
-//===-- ObjectFileWasm.h *- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLDB_PLUGINS_OBJECTFILE_WASM_OBJECTFILEWASM_H
-#define LLDB_PLUGINS_OBJECTFILE_WASM_OBJECTFILEWASM_H
-
-#include "lldb/Symbol/ObjectFile.h"
-#include "lldb/Utility/ArchSpec.h"
-
-namespace lldb_private {
-namespace wasm {
-
-/// Generic Wasm object file reader.
-///
-/// This class provides a generic wasm32 reader plugin implementing the
-/// ObjectFile protocol.
-class ObjectFileWasm : public ObjectFile {
-public:
-  static void Initialize();
-  static void Terminate();
-
-  static ConstString GetPluginNameStatic();
-  static const char *GetPluginDescriptionStatic() {
-return "WebAssembly object file reader.";
-  }
-
-  static ObjectFile *
-  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
- lldb::offset_t data_offset, const FileSpec *file,
- lldb::offset_t file_offset, lldb::offset_t length);
-
-  static ObjectFile *CreateMemoryInstance(const lldb::ModuleSP &module_sp,
-  lldb::DataBufferSP &data_sp,
-  const lldb::ProcessSP &process_sp,
-  lldb::addr_t header_addr);
-
-  static size_t GetModuleSpecifications(const FileSpec &file,
-lldb::DataBufferSP &data_sp,
-lldb::offset_t data_off

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 240262.
paolosev added a comment.

Modified to set `m_file_offset` to be the correct offset of the Code section. 
This also simplifies the code in ObjectFileWasm to avoid a special case for the 
code section in `ObjectFileWasm::SetLoadAddress`.
The DWARF code seems to only use GetFileAddress(), which still needs to return 
zero for the Code section.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm.yaml
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -37,6 +37,7 @@
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
 #include "Plugins/Instruction/PPC64/EmulateInstructionPPC64.h"
 #include "Plugins/InstrumentationRuntime/ASan/ASanRuntime.h"
@@ -246,6 +247,7 @@
   DynamicLoaderMacOSXDYLD::Initialize();
   DynamicLoaderMacOS::Initialize();
   DynamicLoaderPOSIXDYLD::Initialize();
+  wasm::DynamicLoaderWasmDYLD::Initialize();
   DynamicLoaderStatic::Initialize();
   DynamicLoaderWindowsDYLD::Initialize();
 
@@ -329,6 +331,7 @@
   DynamicLoaderMacOSXDYLD::Terminate();
   DynamicLoaderMacOS::Terminate();
   DynamicLoaderPOSIXDYLD::Terminate();
+  wasm::DynamicLoaderWasmDYLD::Terminate();
   DynamicLoaderStatic::Terminate();
   DynamicLoaderWindowsDYLD::Terminate();
 
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -1,153 +1,151 @@
-//===-- ObjectFileWasm.h *- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLDB_PLUGINS_OBJECTFILE_WASM_OBJECTFILEWASM_H
-#define LLDB_PLUGINS_OBJECTFILE_WASM_OBJECTFILEWASM_H
-
-#include "lldb/Symbol/ObjectFile.h"
-#include "lldb/Utility/ArchSpec.h"
-
-namespace lldb_private {
-namespace wasm {
-
-/// Generic Wasm object file reader.
-///
-/// This class provides a generic wasm32 reader plugin implementing the
-/// ObjectFile protocol.
-class ObjectFileWasm : public ObjectFile {
-public:
-  static void Initialize();
-  static void Terminate();
-
-  static ConstString GetPluginNameStatic();
-  static const char *GetPluginDescriptionStatic() {
-return "WebAssembly object file reader.";
-  }
-
-  static ObjectFile *
-  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
- lldb::offset_t data_offset, const FileSpec *file,
- lldb::offset_t file_offset, lldb::offset_t length);
-
-  static ObjectFile *CreateMemoryInstance(const lldb::ModuleSP &module_sp,
-  lldb::DataBufferSP &data_sp,
-  const lldb::ProcessSP &process_sp,
-  lldb::addr_t header_addr);
-
-  static size_t GetModuleSpecifications(const FileSpec &file,
-lldb::DataBufferSP &data_sp,
-lldb::offset_t data_offset,
-lldb::offset_t file_offset,
-lldb::offset_t length,
-ModuleSpecList &specs);
-
-  /// PluginInterface protocol.
-  /// \{
-  ConstString GetPluginName() override { return GetPluginNameStatic(); }
-  uint32_t GetPluginVersion() override { return 1; }
-  /// \}
-
-  /// LLVM RTTI support
-  /// \{
-  static char ID;
-  bool isA(const void *ClassID) const override {
-return ClassID == &ID || ObjectFile::isA(ClassID);
-  }
-  static bool clas

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 240711.
paolosev added a comment.

In D72751#1841498 , @labath wrote:

> Thanks. I am glad that we were able to sort that out.
>
> Now that there's nothing wasm-specific in 
> `DynamicLoaderWasmDYLD::LoadModuleAtAddress`, I have another question. :)
>
> The code in that function is a subset of the base 
> `DynamicLoader::LoadModuleAtAddress` method you are overriding.  Is there a 
> reason for that?
>  It doesn't seem like the additional stuff in the base method should hurt 
> here. What the additional code does is that it tries to search for the object 
> file on the local filesystem, and if it finds it, it will use that instead of 
> copying the file over from the remote. In fact, that sounds like it could be 
> useful here too, as copying that much data over gdb-remote isn't particularly 
> fast..
>
> What do you think about that?


You are right! At this point `DynamicLoaderWasmDYLD::LoadModuleAtAddress` does 
not do anything specific to Wasm and there is no reason to override 
`DynamicLoader::LoadModuleAtAddress`. We can just call that base function, 
which should also work when the Wasm module is in the local filesystem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm.yaml
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -37,6 +37,7 @@
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
 #include "Plugins/Instruction/PPC64/EmulateInstructionPPC64.h"
 #include "Plugins/InstrumentationRuntime/ASan/ASanRuntime.h"
@@ -246,6 +247,7 @@
   DynamicLoaderMacOSXDYLD::Initialize();
   DynamicLoaderMacOS::Initialize();
   DynamicLoaderPOSIXDYLD::Initialize();
+  wasm::DynamicLoaderWasmDYLD::Initialize();
   DynamicLoaderStatic::Initialize();
   DynamicLoaderWindowsDYLD::Initialize();
 
@@ -329,6 +331,7 @@
   DynamicLoaderMacOSXDYLD::Terminate();
   DynamicLoaderMacOS::Terminate();
   DynamicLoaderPOSIXDYLD::Terminate();
+  wasm::DynamicLoaderWasmDYLD::Terminate();
   DynamicLoaderStatic::Terminate();
   DynamicLoaderWindowsDYLD::Terminate();
 
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -1,153 +1,151 @@
-//===-- ObjectFileWasm.h *- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLDB_PLUGINS_OBJECTFILE_WASM_OBJECTFILEWASM_H
-#define LLDB_PLUGINS_OBJECTFILE_WASM_OBJECTFILEWASM_H
-
-#include "lldb/Symbol/ObjectFile.h"
-#include "lldb/Utility/ArchSpec.h"
-
-namespace lldb_private {
-namespace wasm {
-
-/// Generic Wasm object file reader.
-///
-/// This class provides a generic wasm32 reader plugin implementing the
-/// ObjectFile protocol.
-class ObjectFileWasm : public ObjectFile {
-public:
-  static void Initialize();
-  static void Terminate();
-
-  static ConstString GetPluginNameStatic();
-  static const char *GetPluginDescriptionStatic() {
-return "WebAssembly object file reader.";
-  }
-
-  static ObjectFile *
-  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
- lldb::offset_t data_offset, const FileSpec *file,
- lldb::offset_t file_offset, lldb::offset_t length);
-
-  static ObjectFile *CreateMemoryInstance(const lldb::ModuleSP &module_sp,
-  lldb::DataBufferSP &data_sp,
-  const lldb::ProcessSP &process_sp,
-  

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

Comments inline...

>> In D72751#1835502 , @paolosev wrote:
>> 
>>> The first is that we need to call `Target::SetSectionLoadAddress()` twice, 
>>> from two different places. First we need to call 
>>> `Target::SetSectionLoadAddress()` in `ObjectFileWasm::SetLoadAddress()`, 
>>> and then again in `DynamicLoaderWasmDYLD::DidAttach()`. The reason for this 
>>> seems to originate in the sequence of function calls:
>>>
>>> In `DynamicLoaderWasmDYLD::DidAttach()` we call 
>>> `ProcessGDBRemote::LoadModules()` to get list of loaded modules from the 
>>> remote (Wasm engine).
>>>  `ProcessGDBRemote::LoadModules()` calls, first:
>>>
>>> - `DynamicLoaderWasmDYLD::LoadModuleAtAddress()` and from there: ...
>>>
>>>   then:
>>> - `Target::SetExecutableModule() -> Target::ClearModules() -> 
>>> SectionLoadList::Clear()`
>>>
>>>   So, at the end of `LoadModules()` in `DynamicLoaderWasmDYLD::DidAttach()` 
>>> the SectionLoadList is empty, and we need to set it again by calling 
>>> `Target::.SetSectionLoadAddress()` again. This works but the duplication is 
>>> ugly; is there a way to improve this?
>> 
>> 
>> I hope so. :) This seems like a bug in `ProcessGDBRemote::LoadModules`. It 
>> seems wrong/wasteful/etc to do all this work to compute the section load 
>> addresses only to have them be thrown away by `SetExecutableModule`. Maybe 
>> all it would take is to reverse the order of these two actions, so that the 
>> load addresses persist? Can you try something like that?
> 
> I would be interested to see if this helps as well.

The problem went away with the realization that `ObjectFileWasm` type should be 
`eTypeSharedLibrary`, not `eTypeExecutable`.
But more generically for `ProcessGDBRemote::LoadModules` I don't know if it 
would be easily possible to reverse the order of the calls to 
`DynamicLoader::LoadModuleAtAddress` and `Target::SetExecutableModule` given 
that the first creates the Module which is passed to the second. Maybe 
refactoring `DynamicLoader::LoadModuleAtAddress` so that it calls 
`Target::SetExecutableModule` just after `target.GetOrCreateModule` but before 
`UpdateLoadedSections`, but other DynamicLoader plugins could override this 
method...

>> I am also worried about the fact that `SymbolFileDWARF::CalculateAbilities` 
>> requires the module to be "loaded". That shouldn't be normally required. 
>> That function does a very basic check on some sections, and this should work 
>> fine without those sections having a "load address", even if they are 
>> actually being loaded from target memory. I think this means there are some 
>> additional places where ObjectFileWasm should use `m_memory_addr` instead of 
>> something else...
> 
> So a lot of these problems might go away if the modify the ObjectFileWASM to 
> "do the right thing" when the ObjectFile is from a live process. Object file 
> instances know if there are from a live process because the ObjectFile 
> members:
> 
>   lldb::ProcessWP m_process_wp;
>   const lldb::addr_t m_memory_addr;
>
> 
> Will be filled in. So the object file doesn't need to have its sections 
> loaded in a target in order to read section contents right? The object file 
> can be asked to read its section data via ObjectFile::ReadSectionData(...) (2 
> variants).
> 
> So Pavel is correct, the sections don't need to be loaded for anything in 
> SymbolFileDWARF::CalculateAbilities(...). The section list does need to be 
> created and available during SymbolFileDWARF::CalculateAbilities(...). We 
> just need to be able to get the section contents and poke around at the DWARF 
> bits.

Yes, this seems to be the case with the current implementation of 
ObjectFileWASM. It creates the section list in `ObjectFileWasm::SetLoadAddress` 
which calls `Target::SetSectionLoadAddress` but the sections don't need to be 
fully loaded, and during `SymbolFileDWARF::CalculateAbilities(...)` 
`ObjectFile::ReadSectionData` is called to load the necessary data.

>> 
>> 
>>> The second problem is that the Code Section needs to be initialized (in 
>>> `ObjectFileWasm::CreateSections()`) with `m_file_addr = m_file_offset = 0`, 
>>> and not with the actual file offset of the Code section in the Wasm file. 
>>> If we set `Section::m_file_addr` and `Section::m_file_offset` to the actual 
>>> code offset, the DWARF info does not work correctly.
>>> 
>>> I have some doubts regarding the DWARF data generated by Clang for a Wasm 
>>> target. Looking at an example, for a Wasm module that has the Code section 
>>> at offset 0x57, I see this DWARF data:
>>> 
>>>   0x000b: DW_TAG_compile_unit
>>> […]
>>> DW_AT_low_pc (0x)
>>> DW_AT_ranges (0x
>>>[0x0002, 0x000e)
>>>[0x000f, 0x001a)
>>>[0x001b, 0x0099)
>>>[0x009b, 0x011c))
>>> 
>>> 
>>> The documen

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

Regarding:

>> - make the base DynamicLoader class instantiatable, and use it whenever we 
>> fail to find a specialized plugin
>> - same as above, but only do that for ProcessGDBRemote instances
>> - make ProcessGDBRemote call LoadModules() itself if no dynamic loader 
>> instance is available WDYT?
> 
> I am fine with 1 as long as we document the DynamicLoader class to say that 
> it will call Process::LoadModules() and will be used if no specialized loader 
> is needed for your platform. I would like to a see a solution that will work 
> for any process plug-in and not just ProcessGDBRemote. If we change solution 
> 3 above to say "Make lldb_private::Process call LoadModules() itself if no 
> dynamic loader instance is available" then solution 3 is also fine.

there is a problem: if I remove DynamicLoaderWasmDYLD what happens is that 
`DynamicLoaderStatic` is found as a valid loader for a triple like 
**"wasm32-unknown-unknown-wasm"** because the Triple::OS is 
llvm::Triple::UnknownOS (I found out the hard way when I was registering 
DynamicLoaderWasmDYLD after DynamicLoaderStatic :-)).
There is an explicit check for UnknownOS:

  DynamicLoader *DynamicLoaderStatic::CreateInstance(Process *process,
 bool force) {
bool create = force;
if (!create) {
  const llvm::Triple &triple_ref =
  process->GetTarget().GetArchitecture().GetTriple();
  const llvm::Triple::OSType os_type = triple_ref.getOS();
  if ((os_type == llvm::Triple::UnknownOS))
create = true;
}
...
  
  call stack:
  DynamicLoaderStatic::CreateInstance(lldb_private::Process * process, bool 
force) Line 29
  DynamicLoader::FindPlugin(lldb_private::Process * process, const char * 
plugin_name) Line 52
  lldb_private::process_gdb_remote::ProcessGDBRemote::GetDynamicLoader() Line 
3993
  lldb_private::Process::CompleteAttach() Line 2931
  lldb_private::Process::ConnectRemote(lldb_private::Stream * strm, 
llvm::StringRef remote_url) Line 3022

Could ProcessGDBRemote::GetDynamicLoader behave differently just when the 
architecture is wasm32, maybe? But then it is probably cleaner to add this 
plugin class, what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 241824.
paolosev marked 2 inline comments as done.
paolosev added a comment.

Fixed the tests as suggested, and also added a couple more tests, to cover both 
the case where the Wasm module is loaded from memory and the case where it is 
loaded from a file.

> However, when I though about that idea further, I realized that any default 
> plugin we could implement this way could not be complete, as we would be 
> missing the part which (un)loads modules when a new shared library 
> (dis)appears. This is something that cannot be done in a generic way, as that 
> usually requires setting breakpoint on some special symbol, or getting 
> notifications about shared library events in some other way. I don't know 
> whether this is something that you will also need/plan to implement for wasm, 
> or if one can assume that all modules are loaded from the get-go, but this is 
> what convinced me that putting this in a separate plugin is fine.

Yes, we'll need to support the case where a Wasm module is loaded at runtime, I 
think ProcessGDBRemote already does most of the work when it receives a stop 
event that contains the "library" flag and calls LoadModules() again. But I 
need to test this carefully.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_sym.yaml
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm_embedded_debug_sections.yaml
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm_external_debug_sections.yaml
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -37,6 +37,7 @@
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
 #include "Plugins/Instruction/PPC64/EmulateInstructionPPC64.h"
 #include "Plugins/InstrumentationRuntime/ASan/ASanRuntime.h"
@@ -246,6 +247,7 @@
   DynamicLoaderMacOSXDYLD::Initialize();
   DynamicLoaderMacOS::Initialize();
   DynamicLoaderPOSIXDYLD::Initialize();
+  wasm::DynamicLoaderWasmDYLD::Initialize(); // before DynamicLoaderStatic.
   DynamicLoaderStatic::Initialize();
   DynamicLoaderWindowsDYLD::Initialize();
 
@@ -329,6 +331,7 @@
   DynamicLoaderMacOSXDYLD::Terminate();
   DynamicLoaderMacOS::Terminate();
   DynamicLoaderPOSIXDYLD::Terminate();
+  wasm::DynamicLoaderWasmDYLD::Terminate();
   DynamicLoaderStatic::Terminate();
   DynamicLoaderWindowsDYLD::Terminate();
 
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -1,153 +1,151 @@
-//===-- ObjectFileWasm.h *- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLDB_PLUGINS_OBJECTFILE_WASM_OBJECTFILEWASM_H
-#define LLDB_PLUGINS_OBJECTFILE_WASM_OBJECTFILEWASM_H
-
-#include "lldb/Symbol/ObjectFile.h"
-#include "lldb/Utility/ArchSpec.h"
-
-namespace lldb_private {
-namespace wasm {
-
-/// Generic Wasm object file reader.
-///
-/// This class provides a generic wasm32 reader plugin implementing the
-/// ObjectFile protocol.
-class ObjectFileWasm : public ObjectFile {
-public:
-  static void Initialize();
-  static void Terminate();
-
-  static ConstString GetPluginNameStatic();
-  static const char *GetPluginDescriptionStatic() {
-return "WebAssembly object file reader.";
-  }
-
-  static ObjectFile *
-  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
- lldb::offset_t data_offset, const FileSpec *file,
- lldb::o

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 242494.
paolosev added a comment.

Rebasing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_sym.yaml
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm_embedded_debug_sections.yaml
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm_external_debug_sections.yaml
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -38,6 +38,7 @@
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h"
 #include "Plugins/Instruction/ARM/EmulateInstructionARM.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
 #include "Plugins/Instruction/MIPS/EmulateInstructionMIPS.h"
@@ -244,6 +245,7 @@
   DynamicLoaderMacOSXDYLD::Initialize();
   DynamicLoaderMacOS::Initialize();
   DynamicLoaderPOSIXDYLD::Initialize();
+  wasm::DynamicLoaderWasmDYLD::Initialize(); // before DynamicLoaderStatic.
   DynamicLoaderStatic::Initialize();
   DynamicLoaderWindowsDYLD::Initialize();
 
@@ -332,6 +334,7 @@
   DynamicLoaderMacOSXDYLD::Terminate();
   DynamicLoaderMacOS::Terminate();
   DynamicLoaderPOSIXDYLD::Terminate();
+  wasm::DynamicLoaderWasmDYLD::Terminate();
   DynamicLoaderStatic::Terminate();
   DynamicLoaderWindowsDYLD::Terminate();
 
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -69,7 +69,7 @@
 return m_arch.GetByteOrder();
   }
 
-  bool IsExecutable() const override { return true; }
+  bool IsExecutable() const override { return false; }
 
   uint32_t GetAddressByteSize() const override {
 return m_arch.GetAddressByteSize();
@@ -81,7 +81,7 @@
 
   Symtab *GetSymtab() override;
 
-  bool IsStripped() override { return true; }
+  bool IsStripped() override { return !!GetExternalDebugInfoFileSpec(); }
 
   void CreateSections(SectionList &unified_section_list) override;
 
@@ -93,7 +93,7 @@
 
   uint32_t GetDependentModules(FileSpecList &files) override { return 0; }
 
-  Type CalculateType() override { return eTypeExecutable; }
+  Type CalculateType() override { return eTypeSharedLibrary; }
 
   Strata CalculateStrata() override { return eStrataUser; }
 
@@ -101,8 +101,7 @@
   bool value_is_offset) override;
 
   lldb_private::Address GetBaseAddress() override {
-return IsInMemory() ? Address(m_memory_addr + m_code_section_offset)
-: Address(m_code_section_offset);
+return IsInMemory() ? Address(m_memory_addr) : Address(0);
   }
   /// \}
 
@@ -127,7 +126,7 @@
   /// \}
 
   /// Read a range of bytes from the Wasm module.
-  DataExtractor ReadImageData(uint64_t offset, size_t size);
+  DataExtractor ReadImageData(lldb::offset_t offset, uint32_t size);
 
   typedef struct section_info {
 lldb::offset_t offset;
@@ -145,7 +144,6 @@
   std::vector m_sect_infos;
   ArchSpec m_arch;
   UUID m_uuid;
-  uint32_t m_code_section_offset;
 };
 
 } // namespace wasm
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
@@ -233,7 +233,7 @@
offset_t data_offset, const FileSpec *file,
offset_t offset, offset_t length)
 : ObjectFile(module_sp, file, offset, length, data_sp, data_offset),
-  m_arch("wasm32-unknown-unknown-wasm"), m_code_section_offset(0) {
+  m_arch("wasm32-unknown-unknown-wasm") {
   m_data.SetAddressByteSize(4);
 }
 
@@ -242,7 +242,7 @@
const lldb::ProcessSP &process_sp,
lldb::addr_t header_addr)
 : ObjectFile(module_sp, process_sp, header

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 242733.
paolosev added a comment.

Fixing ObjectFile/wasm tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_sym.yaml
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm_embedded_debug_sections.yaml
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm_external_debug_sections.yaml
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/test/Shell/ObjectFile/wasm/basic.yaml
  lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -38,6 +38,7 @@
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h"
 #include "Plugins/Instruction/ARM/EmulateInstructionARM.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
 #include "Plugins/Instruction/MIPS/EmulateInstructionMIPS.h"
@@ -244,6 +245,7 @@
   DynamicLoaderMacOSXDYLD::Initialize();
   DynamicLoaderMacOS::Initialize();
   DynamicLoaderPOSIXDYLD::Initialize();
+  wasm::DynamicLoaderWasmDYLD::Initialize(); // before DynamicLoaderStatic.
   DynamicLoaderStatic::Initialize();
   DynamicLoaderWindowsDYLD::Initialize();
 
@@ -332,6 +334,7 @@
   DynamicLoaderMacOSXDYLD::Terminate();
   DynamicLoaderMacOS::Terminate();
   DynamicLoaderPOSIXDYLD::Terminate();
+  wasm::DynamicLoaderWasmDYLD::Terminate();
   DynamicLoaderStatic::Terminate();
   DynamicLoaderWindowsDYLD::Terminate();
 
Index: lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
@@ -13,11 +13,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
+# CHECK: Executable: false
 # CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Type: shared library
 # CHECK: Strata: user
-# CHECK: Base VM address: 0xa
+# CHECK: Base VM address: 0x0
 
 # CHECK: Name: code
 # CHECK: Type: code
Index: lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
@@ -4,9 +4,9 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Type: shared library
 # CHECK: Strata: user
 # CHECK: Base VM address: 0x0
 
Index: lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
@@ -4,11 +4,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Type: shared library
 # CHECK: Strata: user
-# CHECK: Base VM address: 0xa
+# CHECK: Base VM address: 0x0
 
 # CHECK: Name: code
 # CHECK: Type: code
Index: lldb/test/Shell/ObjectFile/wasm/basic.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/basic.yaml
+++ lldb/test/Shell/ObjectFile/wasm/basic.yaml
@@ -4,11 +4,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Typ

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

Thank you Derek, Jonas; I am sorry for all the trouble...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.

In D72751#1860950 , @max-kudr wrote:

> There is Windows Build Bot failure 
> http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/13427. Can 
> you please fix or revert it?
>
>   Cannot open include file: 
> 'Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h': No such file or 
> directory
>


This should have been fixed by 
https://reviews.llvm.org/rGf5f70d1c8fbf12249b4b9598f10a10f12d4db029.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 243045.
paolosev added a comment.

Modified tests to be compatible with Python3.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_sym.yaml
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm_embedded_debug_sections.yaml
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/test_wasm_external_debug_sections.yaml
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/test/Shell/ObjectFile/wasm/basic.yaml
  lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -38,6 +38,7 @@
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h"
 #include "Plugins/Instruction/ARM/EmulateInstructionARM.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
 #include "Plugins/Instruction/MIPS/EmulateInstructionMIPS.h"
@@ -244,6 +245,7 @@
   DynamicLoaderMacOSXDYLD::Initialize();
   DynamicLoaderMacOS::Initialize();
   DynamicLoaderPOSIXDYLD::Initialize();
+  wasm::DynamicLoaderWasmDYLD::Initialize(); // before DynamicLoaderStatic.
   DynamicLoaderStatic::Initialize();
   DynamicLoaderWindowsDYLD::Initialize();
 
@@ -332,6 +334,7 @@
   DynamicLoaderMacOSXDYLD::Terminate();
   DynamicLoaderMacOS::Terminate();
   DynamicLoaderPOSIXDYLD::Terminate();
+  wasm::DynamicLoaderWasmDYLD::Terminate();
   DynamicLoaderStatic::Terminate();
   DynamicLoaderWindowsDYLD::Terminate();
 
Index: lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
@@ -13,11 +13,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
+# CHECK: Executable: false
 # CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Type: shared library
 # CHECK: Strata: user
-# CHECK: Base VM address: 0xa
+# CHECK: Base VM address: 0x0
 
 # CHECK: Name: code
 # CHECK: Type: code
Index: lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
@@ -4,9 +4,9 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Type: shared library
 # CHECK: Strata: user
 # CHECK: Base VM address: 0x0
 
Index: lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
@@ -4,11 +4,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Type: shared library
 # CHECK: Strata: user
-# CHECK: Base VM address: 0xa
+# CHECK: Base VM address: 0x0
 
 # CHECK: Name: code
 # CHECK: Type: code
Index: lldb/test/Shell/ObjectFile/wasm/basic.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/basic.yaml
+++ lldb/test/Shell/ObjectFile/wasm/basic.yaml
@@ -4,11 +4,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: fal

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

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

In D72751#1862140 , @labath wrote:

> As promised, here are the comments on the new tests. I think that most of the 
> py3 incompatibilities will go away once we get rid of the yaml preprocessing 
> step, but it would be good to verify this with python 3 nonetheless...


Thank you! Removing the yaml preprocessing indeed simplify everything, now 
tests should work both with Python 2 and 3.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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