Add a check whether or not a str is utf8 prior to emplacing

[lanza]

Summary

Highlighing junk data on VSCode can send a query for evaluate which
fails. In particular cases on Windows, this the error message can end
up as a c-string of [-35,-35,-35,-35,...]. Attempting to emplace this
as the error message causes an assert failure.

Prior to emplacing the error message, confirm that it is valid UTF8 to
eliminate errors such as mentione above.

[clayborg]

So any string in the debugger can contain invalid UTF8. This can easily happen for C string summaries or char array summaries from lldb::SBValue objects when a variable isn't yet initialized. It would rather us sanitize any strings that can contain invalid UTF8 and just change any invalid UTF8 sequences into either "\\xXX" where XX is the byte. Notice the extra backslash then the string would show up as \xXX when it gets displayed. Or we can use \uXXXX (notice just a single backslash) which would send the character over as is, just not as invalid UTF8. Best option would be to convert valid UTF8 sequences over to use \uXXXX format (single backslash to correctly encode the character as unicode), and invalid UTF8 sequences to send each invalid byte as \\xXX (double backslash to just display the non printable character correctly. One vote for using the first \\xXX format is it will display correctly in a variable view. If we encode invalid UTF8 sequences over as the actual characters themselves, then the GUI might show something unexpected, or have issues displaying the invalid UTF8.

I would vote to fix this in LLVM's JSON parser when it is encoding any string value. If we can't do this, then we need to make a function that sanitizes any string values and always use that to encode a string correctly.

[lanza]

You're certainly right. But the goal I'm working on now is simply to get lldb-vscode running on Windows. I'll take note of this task to fix it into the future.

[lanza]

Okay, so after looking into this it turns out that llvm::json does handle this well:

Value(std::string V) : Type(T_String) {
  if (LLVM_UNLIKELY(!isUTF8(V))) {
    assert(false && "Invalid UTF-8 in value used as JSON");
    V = fixUTF8(std::move(V));
  }
  create<std::string>(std::move(V));
}

it asserts that the string is valid utf8 but will attempt to fix it if noassert is enabled. So we should just call fixUTF8(thestring) within lldb-vscode at the necessary points where we can end up with non-utf8.

[lanza]

fix if possible

[clayborg]

So we need to do this everywhere for any string value that gets put into an object. Otherwise we are just waiting for a crash from any string value. Values of variables, summaries from variables, so many other places where we get the string value from the API in LLDB can fail. I am fine with just making a static helper function that does this for us:

void emplace_safe_string(llvm::json::Object &obj, llvm::StringRef key, llvm::StringRef str) {
  if (llvm::json::isUTF8(str))
    obj.try_emplace(key, str.str());
  else
    obj.try_emplace(key, llvm::json::fixUTF8(str));
}

Then we need to find all places where the string value is coming from an API, and use this function.

[lanza]

add more fixes

[clayborg]

Just reverse if statement as noted in inlined comments and this is good to go.

tools/lldb-vscode/JSONUtils.cpp

28–31 ↗

(On Diff #174134)

Reverse the if for clarity:

if (LLVM_LIKELY(llvm::json::isUTF8(str)))
    obj.try_emplace(key, str.str());
  else
    obj.try_emplace(key, llvm::json::fixUTF8(str));
}

[lanza]

👍🏼

tools/lldb-vscode/lldb-vscode.cpp

if (!expression.empty() && expression[0] == '`') {
     // many cases and it is faster.
     lldb::SBValue value = frame.GetValueForVariablePath(
         expression.data(), lldb::eDynamicDontRunTarget);
     if (value.GetError().Fail())
       value = frame.EvaluateExpression(expression.data());
     if (value.GetError().Fail()) {
       response.try_emplace("success", false);
       const char *error_cstr = value.GetError().GetCString();
-      if (error_cstr && error_cstr[0])
-        response.try_emplace("message", std::string(error_cstr));
-      else
+      if (error_cstr && error_cstr[0]) {
+        std::string error_string(error_cstr);
+        if (!llvm::json::isUTF8(error_string))
+          error_string = llvm::json::fixUTF8(std::move(error_string));
+        response.try_emplace("message", error_string);
+      } else {
         response.try_emplace("message", "evaluate failed");
+      }
     } else {
       SetValueForKey(value, body, "result");
       auto value_typename = value.GetType().GetDisplayTypeName();
       body.try_emplace("type", value_typename ? value_typename : NO_TYPENAME);
       if (value.MightHaveChildren()) {
         auto variablesReference = VARIDX_TO_VARREF(g_vsc.variables.GetSize());
         g_vsc.variables.Append(value);
         body.try_emplace("variablesReference", variablesReference);