[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

We need to add a test for the symbols added target notification. See my previous comment on stripping a.out to a.out.stripped and then using "a.out.stripped" as the main executable.



================
Comment at: lldb/test/API/tools/lldb-vscode/module/main.cpp:2-9
+int multiply(int x, int y) {
+  return x * y; // breakpoint 1
+}
+
+int main(int argc, char const *argv[]) {
+  int result = multiply(argc, 20);
+  return result < 0;
----------------
We can probably simplify this to just be:

```
int main(int argc, char const *argv[]) {
  return 0; // breakpoint 1
}
```


================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:337-340
+    std::string(module.GetFileSpec().GetFilename()));
+  std::string module_path = std::string(module.GetFileSpec().GetDirectory()) +
+                            "/" +
+                            std::string(module.GetFileSpec().GetFilename());
----------------
Let LLDB take care of the directory delimiter. Otherwise this will be bad on windows:
```
char module_path[PATH_MAX];
module.GetFileSpec().GetPath(module_path, sizeof(module_path));
```



================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:344-346
+    std::string symbol_path =
+        std::string(module.GetSymbolFileSpec().GetDirectory()) + "/" +
+        std::string(module.GetSymbolFileSpec().GetFilename());
----------------
Let LLDB take care of the directory delimiter. Otherwise this will be bad on windows:

```
char symbol_path[PATH_MAX];
module.GetSymbolFileSpec().GetPath(module_path, sizeof(symbol_path));
```



================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:349-350
+  }
+  std::string loaded_addr = std::to_string(
+      module.GetObjectFileHeaderAddress().GetLoadAddress(g_vsc.target));
+  object.try_emplace("addressRange", loaded_addr);
----------------
We need to make sure the address returned is valid and we want the string value to be in hex. Are we sure std::to_string() will create a hex number? If not we can use sprintf:

```
uint64_t load_addr = module.GetObjectFileHeaderAddress().GetLoadAddress(g_vsc.target);
if (load_addr != LLDB_INVALID_ADDRESS) {
  char load_addr_str[64];
  snprintf(load_addr_str, sizeof(load_addr_str), "0x%016" PRIx64, load_addr);
  object.try_emplace("addressRange", load_addr_str);
}
```


================
Comment at: lldb/tools/lldb-vscode/JSONUtils.h:241
 
+/// Converts Module Event to a Visual Studio Code "Module"
+///
----------------
Might be better with text like:

```
/// Converts a LLDB module to a VS Code DAP module for use in "modules" events.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477