[Lldb-commits] [PATCH] D137003: [lldb-vscode] Send Statistics Dump in terminated event

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D137003#3908918 <https://reviews.llvm.org/D137003#3908918>, @shafik wrote:

> It looks like this change broke `TestVSCode_terminatedEvent.py` see Green Dragon build bot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/48111/
>
> Please fix or revert.

So this issue is a memory ownership issue with the way that llvm::json::Object stores its keys. It uses a llvm::json::ObjectKey which either creates a copy of the string if it is constructed with a std::string or if the string passed in isn't valid UTF8. If you construct a key with a "const char *" or a "llvm::StringRef" then the ObjectKey doesn't own the key string. The issue that happened here was that "void addStatistic(llvm::json::Object &event)" was used to get the statistics from LLDB as a lldb::SBStructuredData object. Then we asked for all top level keys using:

  lldb::SBStringList keys;
  if (!statistics.GetKeys(keys))
    return;

This is a class that owns a std::vector<std::string> inside of LLDB. Then we called "void FilterAndGetValueForKey(...) and passed it each key:

  for (size_t i = 0; i < keys.GetSize(); i++) {
    const char *key = keys.GetStringAtIndex(i);
    FilterAndGetValueForKey(statistics, key, stats_body);
  }

Now in FilterAndGetValueForKey(...), for most emplacing of key/value pairs, it was using this:

  std::string key_utf8 = llvm::json::fixUTF8(key);

As the key, but some places used the "key" which was a "const char *", which is owned by the "lldb::SBStringList keys" from "addStatistic(...)". These keys are not owned since they were added using the "const char *key", which means as soon as we return from "addStatistic(...)" and then from "CreateTerminatedEventObject()" these string values can change since they are on the heap. It all depends if anyone mallocs memory in the same place that these strings used to live. So depends on the OS and how the allocator works and how busy the allocator is in the current process.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137003