aelitashen marked 3 inline comments as done.
aelitashen added inline comments.
================
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());
----------------
clayborg wrote:
> 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));
> ```
>
This piece of code will make path unreadable in the log, and cause the modules
view cannot be displayed properly.
================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:344-346
+ std::string symbol_path =
+ std::string(module.GetSymbolFileSpec().GetDirectory()) + "/" +
+ std::string(module.GetSymbolFileSpec().GetFilename());
----------------
clayborg wrote:
> 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));
> ```
>
Same reason as above.
================
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);
----------------
clayborg wrote:
> 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);
> }
> ```
Decimal to Hex conversion is done in the IDE, not sure if we need to move it
here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82477/new/
https://reviews.llvm.org/D82477
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
