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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp:90-126
+  ModuleList loaded_module_list;
+  const ModuleList &module_list = m_process->GetTarget().GetImages();
+  const size_t num_modules = module_list.GetSize();
+  for (uint32_t idx = 0; idx < num_modules; ++idx) {
+    ModuleSP module_sp(module_list.GetModuleAtIndexUnlocked(idx));
+    if (!module_sp)
+      continue;
----------------
Right, so, given that (IIUC) you use the `qXfer:libraries` packet, I believe this code should not be needed. In this case `ProcessGDBRemote::LoadModules` should do all the work (by calling into `DynamicLoaderWasmDYLD::LoadModuleAtAddress`, which will then call into `ObjectFileWasm::SetLoadAddress`).

The fact that you need fix up section load addresses after these functions are done makes me believe that those functions are not doing their job properly. That wouldn't be too bad if there is a reason for that, but right now I don't see any indication that this is the case.

Can you explain what is the purpose of this code (specifically, what would happen without it, if we only had m_process->LoadModules() here), so we can figure out what to do about this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751