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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

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...



================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py:8
+
+LLDB_INVALID_ADDRESS = 0xffffffffffffffff
+load_address = 0x400000000
----------------
this should be available as `lldb.LLDB_INVALID_ADDRESS`


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py:173-179
+        with open(yaml_template_path, mode='r') as file:
+            yaml = file.read()
+            file.close()
+            yaml = yaml.replace("###_EXTERNAL_DEBUG_INFO_###", format_bytearray_as_hexstring(encode_wasm_string(sym_obj_path)))
+            with open(yaml_path, mode='w') as file:
+                file.write(yaml)
+                file.close()
----------------
a simpler way to handle this would be to put just the bare file name (no path) into the yaml, and then add the build directory to the `target.debug-file-search-paths` setting.


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py:233
+            def readMemory(self, addr, length):
+                assert False # Should not be called
+
----------------
As I said in the previous comment, this needs to be relaxed a bit. Maybe you could just always return an error. This way we can be sure that the file is not accidentally read from memory but spurious memory reads be lldb will not cause the test to fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751