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

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:40
+
+  const uint32_t *version = reinterpret_cast<const uint32_t *>(
+      data_sp->GetBytes() + sizeof(llvm::wasm::WasmMagic));
----------------
This looks like it will cause problems on big endian hosts..


================
Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:252
+
+  ModuleSpec spec(file, ArchSpec("wasm32-unknown-unknown-wasm"));
+  specs.Append(spec);
----------------
I take it that wasm files don't have anything like a build id, uuid or something similar?


================
Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:334-351
+      if (section_type != eSectionTypeOther) {
+        SectionSP section_sp(
+            new Section(GetModule(), // Module to which this section belongs.
+                        this, // ObjectFile to which this section belongs and
+                              // should read section data from.
+                        section_type,                  // Section ID.
+                        ConstString(sect_info.name),   // Section name.
----------------
It would be nice to merge these two section-creating blocks..


================
Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h:116
+
+  typedef struct section_info {
+    lldb::offset_t offset;
----------------
We don't use this typedef style (except possibly in some old code which we shouldn't emulate).


================
Comment at: lldb/unittests/ObjectFile/wasm/TestObjectFileWasm.cpp:1
+//===-- TestObjectFileWasm.cpp --------------------------------------------===//
+//
----------------
Overall, these tests would be better off as "lit" tests. Something along the lines of:
```
yaml2obj %s >%t
lldb-test object-file %t | FileCheck %t
```
You can look at existing tests in `test/Shell/ObjectFile` for inspiration. Is there anything you test here that "lldb-test object-file" does not print out?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575