[Lldb-commits] [PATCH] D126014: Show error message for optimized variables

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D126014#3886996 <https://reviews.llvm.org/D126014#3886996>, @jgorbe wrote:

> Hi! I've just debugged an issue that brought me to this commit. I'll start preparing a patch tomorrow (not sure how to test it yet) but since it's a recent-ish change I figured I'd ping the commit thread to give you a heads up.
>
> `v.GetSummary()` returns a char* that is backed by a member `std::string m_summary_str` in SBValue, and we put the result in a StringRef that just points there. I've found that in some cases the call to `v.GetError()` invalidates the cached summary and causes the `m_summary_str` member to be cleared.
>
> So, if we have a summary provider that returns "Summary string", it ends up being displayed as "<NUL byte>ummary string", because clearing `m_summary_str` sets the first byte to 0 but we are still holding on to a StringRef with the old length.
>
> Calling `v.GetError()` first seems to avoid the issue. Another option to fix it would be to keep the summary in a local `std::string` instead. Any preference?

I would vote that we modify our lldb::SBValue to not clear the cached m_summary_str when it is called. Doesn't make any sense for it to do this. Since we are returning "const char *" from the SBValue object methods, those strings need to live for the lifetime of the object and any methods on SBValue shouldn't be causing others to be cleared, unless someone holds onto a SBValue object and it actually gets updated due to a process resuming and then stopping.

See inlined code suggestion as a way to fix this a bit more safely.

If you want to use std::string objects you have to watch our for NULL being returned for any "const char *". If NULL is returned the std::string constructor will crash lldb-vscode.



================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:136-156
   llvm::StringRef value = v.GetValue();
   llvm::StringRef summary = v.GetSummary();
   llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
+  lldb::SBError error = v.GetError();
 
   std::string result;
   llvm::raw_string_ostream strm(result);
----------------
Another way to fix this is to re-org the code a bit. We don't need the value, summary or typename if there is an error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126014