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

[Pavel Labath via Phabricator via lldb-commits]

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

lgtm, per the previous comment. A couple of additional inline comments in the test.



================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py:78
+                    file_content = bytearray(file.read())                    
+                    addr_from = addr - self.load_address
+                    addr_to = addr_from + min(length, len(file_content) - addr_from)
----------------
maybe also check that `addr >= self.load_address`. I wouldn't be surprised if lldb (now or in the future) decides it wants to try reading some other parts of memory too... (and we should return an error instead of falling over in that case).


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py:107-123
+        self.assertEquals(0x40000000a, code_section.GetLoadAddress(target))
+
+        debug_info_section = module.GetSectionAtIndex(1)
+        self.assertEquals(".debug_info", debug_info_section.GetName())
+        self.assertEquals(0x400000050, debug_info_section.GetLoadAddress(target))
+
+        debug_abbrev_section = module.GetSectionAtIndex(2)
----------------
I'm not sure how much we can rely on yaml2obj offsets not changing. Hard-coding the order of sections is fine, but maybe if would be better to check something like `section.GetFileOffset() == 0x400...0 | section.GetFileOffset()`. Since section parsing is checked elsewhere, the main thing we want to ensure here is that the `0x400...0` thingy is plumbed through correctly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751