[clang] [clang-tools-extra] [llvm] [Support][JSON] Add safeUTF8() helper to sanitize UTF-8 for JSON values (PR #212949)

Cyrus Ding via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 30 01:15:30 PDT 2026


https://github.com/dingcyrus updated https://github.com/llvm/llvm-project/pull/212949

>From 28c33369c39677cfa65f953bdbcf80620c440961 Mon Sep 17 00:00:00 2001
From: Chenguang Ding <dingchenguang at kylinos.cn>
Date: Thu, 30 Jul 2026 15:50:34 +0800
Subject: [PATCH]   [Support][JSON] Add safeUTF8() helper to sanitize UTF-8 for
 JSON values
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

  The isUTF8() → fixUTF8() pattern is repeated across the codebase because
  json::Value(StringRef) and json::ObjectKey(StringRef) assert on invalid
  input in debug builds.  This makes callers working with untrusted strings
  (e.g. source comments) duplicate the same check-and-fix logic.

  Add json::safeUTF8(StringRef) which returns a zero-copy Value for valid
  UTF-8 and a fixed (U+FFFD-substituted) Value for invalid input.  The
  utility consolidates what was previously open-coded in several places:

  - clang-doc had a local safeJSONString() helper (removed in this patch)
  - SymbolGraphSerializer used an inline isUTF8 ? s : fixUTF8(s) ternary
  - lldb duplicated the pattern in several EmplaceSafeString wrappers

  This is an NFC follow-up to PR #210886.
---
 clang-tools-extra/clang-doc/JSONGenerator.cpp |  14 +-
 .../Serialization/SymbolGraphSerializer.cpp   |   4 +-
 llvm/include/llvm/Support/JSON.h              |  20 ++-
 llvm/lib/Support/JSON.cpp                     |   7 +-
 llvm/unittests/Support/JSONTest.cpp           | 130 ++++++++++++------
 5 files changed, 115 insertions(+), 60 deletions(-)

diff --git a/clang-tools-extra/clang-doc/JSONGenerator.cpp b/clang-tools-extra/clang-doc/JSONGenerator.cpp
index b4284f8eb4d14..842b17944f937 100644
--- a/clang-tools-extra/clang-doc/JSONGenerator.cpp
+++ b/clang-tools-extra/clang-doc/JSONGenerator.cpp
@@ -106,12 +106,6 @@ static void insertNonEmpty(StringRef Key, StringRef Value, Object &Obj) {
     Obj[Key] = Value;
 }
 
-static json::Value safeJSONString(StringRef S) {
-  if (LLVM_LIKELY(json::isUTF8(S)))
-    return S;
-  return json::fixUTF8(S);
-}
-
 static std::string infoTypeToString(InfoType IT) {
   switch (IT) {
   case InfoType::IT_default:
@@ -239,7 +233,7 @@ static Object serializeComment(const CommentInfo &I, Object &Description) {
   switch (I.Kind) {
   case CommentKind::CK_TextComment: {
     if (!I.Text.empty())
-      Obj.insert({commentKindToString(I.Kind), safeJSONString(I.Text)});
+      Obj.insert({commentKindToString(I.Kind), json::safeUTF8(I.Text)});
     return Obj;
   }
 
@@ -264,7 +258,7 @@ static Object serializeComment(const CommentInfo &I, Object &Description) {
     auto &ARef = *ArgsArr.getAsArray();
     ARef.reserve(I.Args.size());
     for (const auto &Arg : I.Args)
-      ARef.emplace_back(safeJSONString(Arg));
+      ARef.emplace_back(json::safeUTF8(Arg));
     Child.insert({"Command", I.Name});
     Child.insert({"Args", ArgsArr});
     Child.insert({"Children", ChildArr});
@@ -298,7 +292,7 @@ static Object serializeComment(const CommentInfo &I, Object &Description) {
 
   case CommentKind::CK_VerbatimBlockLineComment:
   case CommentKind::CK_VerbatimLineComment: {
-    Child.insert({"Text", safeJSONString(I.Text)});
+    Child.insert({"Text", json::safeUTF8(I.Text)});
     Child.insert({"Children", ChildArr});
     Obj.insert({commentKindToString(I.Kind), ChildVal});
     return Obj;
@@ -339,7 +333,7 @@ static Object serializeComment(const CommentInfo &I, Object &Description) {
   }
 
   case CommentKind::CK_Unknown: {
-    Obj.insert({commentKindToString(I.Kind), safeJSONString(I.Text)});
+    Obj.insert({commentKindToString(I.Kind), json::safeUTF8(I.Text)});
     return Obj;
   }
   }
diff --git a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
index 6228f2e687be2..2e44235963cfc 100644
--- a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -264,9 +264,7 @@ std::optional<Object> serializeDocComment(const DocComment &Comment) {
     Object Line;
     // Comments in source files may contain invalid UTF-8. JSON values must be
     // valid UTF-8, so replace any invalid sequences before serializing.
-    Line["text"] = json::isUTF8(CommentLine.Text)
-                       ? CommentLine.Text
-                       : json::fixUTF8(CommentLine.Text);
+    Line["text"] = json::safeUTF8(CommentLine.Text);
     serializeObject(Line, "range",
                     serializeSourceRange(CommentLine.Begin, CommentLine.End));
     LinesArray.emplace_back(std::move(Line));
diff --git a/llvm/include/llvm/Support/JSON.h b/llvm/include/llvm/Support/JSON.h
index 3b145a69c74cb..74a7040db2579 100644
--- a/llvm/include/llvm/Support/JSON.h
+++ b/llvm/include/llvm/Support/JSON.h
@@ -123,11 +123,11 @@ class Object {
   void clear() { M.clear(); }
   std::pair<iterator, bool> insert(KV E);
   template <typename... Ts>
-  std::pair<iterator, bool> try_emplace(const ObjectKey &K, Ts &&... Args) {
+  std::pair<iterator, bool> try_emplace(const ObjectKey &K, Ts &&...Args) {
     return M.try_emplace(K, std::forward<Ts>(Args)...);
   }
   template <typename... Ts>
-  std::pair<iterator, bool> try_emplace(ObjectKey &&K, Ts &&... Args) {
+  std::pair<iterator, bool> try_emplace(ObjectKey &&K, Ts &&...Args) {
     return M.try_emplace(std::move(K), std::forward<Ts>(Args)...);
   }
   bool erase(StringRef K);
@@ -640,9 +640,7 @@ inline Object::Object(std::initializer_list<KV> Properties) {
 inline std::pair<Object::iterator, bool> Object::insert(KV E) {
   return try_emplace(std::move(E.K), std::move(E.V));
 }
-inline bool Object::erase(StringRef K) {
-  return M.erase(ObjectKey(K));
-}
+inline bool Object::erase(StringRef K) { return M.erase(ObjectKey(K)); }
 
 LLVM_ABI std::vector<const Object::value_type *>
 sortedElements(const Object &O);
@@ -981,7 +979,7 @@ Expected<T> parse(const llvm::StringRef &JSON, const char *RootName = "") {
 /// an array, and so on.
 /// With asserts disabled, this is undefined behavior.
 class OStream {
- public:
+public:
   using Block = llvm::function_ref<void()>;
   // If IndentSize is nonzero, output is pretty-printed.
   explicit OStream(llvm::raw_ostream &OS, unsigned IndentSize = 0)
@@ -1035,7 +1033,7 @@ class OStream {
   // Valid only within an object (any number of times).
 
   /// Emit an attribute whose value is self-contained (number, vector<int> etc).
-  void attribute(llvm::StringRef Key, const Value& Contents) {
+  void attribute(llvm::StringRef Key, const Value &Contents) {
     attributeImpl(Key, [&] { value(Contents); });
   }
   /// Emit an attribute whose value is an array with elements from the Block.
@@ -1094,6 +1092,14 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Value &V) {
   OStream(OS).value(V);
   return OS;
 }
+
+/// If \p S is valid UTF-8, returns a Value that references S (zero-copy).
+/// Otherwise, replaces invalid sequences with U+FFFD and returns an owned
+/// string Value (calls fixUTF8). This is safe to use with untrusted strings
+/// without triggering assertions in debug builds.
+/// The returned Value may reference S's data, so S must outlive it.
+LLVM_ABI Value safeUTF8(StringRef S);
+
 } // namespace json
 
 /// Allow printing json::Value with formatv().
diff --git a/llvm/lib/Support/JSON.cpp b/llvm/lib/Support/JSON.cpp
index 23c2542d75810..ae2de692e9223 100644
--- a/llvm/lib/Support/JSON.cpp
+++ b/llvm/lib/Support/JSON.cpp
@@ -726,6 +726,12 @@ std::string fixUTF8(llvm::StringRef S) {
   return Res;
 }
 
+Value safeUTF8(StringRef S) {
+  if (LLVM_LIKELY(isUTF8(S)))
+    return S;
+  return fixUTF8(S);
+}
+
 static void quote(llvm::raw_ostream &OS, llvm::StringRef S) {
   OS << '\"';
   for (unsigned char C : S) {
@@ -930,4 +936,3 @@ void llvm::format_provider<llvm::json::Value>::format(
     llvm_unreachable("json::Value format options should be an integer");
   json::OStream(OS, IndentAmount).value(E);
 }
-
diff --git a/llvm/unittests/Support/JSONTest.cpp b/llvm/unittests/Support/JSONTest.cpp
index 348f5cb04f9db..a1c4113934caf 100644
--- a/llvm/unittests/Support/JSONTest.cpp
+++ b/llvm/unittests/Support/JSONTest.cpp
@@ -139,7 +139,7 @@ TEST(JSONTest, Array) {
   A.emplace_back(3);
   A.emplace(++A.begin(), 0);
   A.push_back(4);
-  A.insert(++++A.begin(), 99);
+  A.insert(++ ++A.begin(), 99);
 
   EXPECT_EQ(A.size(), 6u);
   EXPECT_EQ(R"([1,0,99,2,3,4])", s(std::move(A)));
@@ -263,6 +263,58 @@ TEST(JSONTest, UTF8) {
   }
 }
 
+TEST(JSONTest, SafeUTF8) {
+  // Valid UTF-8: safeUTF8 returns a Value with the same string content.
+  for (StringRef Valid : {
+           "this is ASCII text",
+           "thïs tëxt häs BMP chäräctërs",
+           "𐌶𐌰L𐌾𐍈 C𐍈𐌼𐌴𐍃",
+           "",
+       }) {
+    Value V = safeUTF8(Valid);
+    ASSERT_EQ(V.kind(), Value::String);
+    EXPECT_EQ(*V.getAsString(), Valid);
+  }
+
+  // Invalid UTF-8: safeUTF8 returns a Value with U+FFFD substitutions.
+  for (auto [Invalid, Expected] :
+       std::vector<std::pair<const char *, const char *>>{
+           {"lone trailing \x81\x82 bytes",
+            "lone trailing \xEF\xBF\xBD\xEF\xBF\xBD bytes"},
+           {"missing trailing \xD0 bytes",
+            "missing trailing \xEF\xBF\xBD bytes"},
+           {"truncated character \xD0", "truncated character \xEF\xBF\xBD"},
+           {"not \xC1\x80 the \xE0\x9f\xBF shortest \xF0\x83\x83\x83 encoding",
+            "not \xEF\xBF\xBD\xEF\xBF\xBD the "
+            "\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD shortest "
+            "\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD encoding"},
+           {"surrogate \xED\xA0\x80 invalid \xF4\x90\x80\x80",
+            "surrogate \xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD invalid "
+            "\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD"}}) {
+    Value V = safeUTF8(Invalid);
+    ASSERT_EQ(V.kind(), Value::String);
+    EXPECT_EQ(*V.getAsString(), Expected);
+  }
+
+  // safeUTF8 must not trigger assertions in debug mode — this is its
+  // primary reason for existing.  (The test above already covers this,
+  // but we add an explicit regression check for the clang-doc crash case.)
+  // Byte 0x97 is invalid UTF-8 and triggered:
+  //   assert(false && "Invalid UTF-8 in value used as JSON")
+  Value V = safeUTF8("\x97");
+  ASSERT_EQ(V.kind(), Value::String);
+
+  // Verify safeUTF8 integrates correctly into Object construction.
+  Object O;
+  O["valid"] = safeUTF8("no issue");
+  O["fixed"] = safeUTF8("bad \xD0 byte");
+  std::string S;
+  llvm::raw_string_ostream OS(S);
+  OS << Value(std::move(O));
+  EXPECT_NE(S.find("no issue"), std::string::npos);
+  EXPECT_NE(S.find("bad \xEF\xBF\xBD byte"), std::string::npos);
+}
+
 TEST(JSONTest, Inspection) {
   llvm::Expected<Value> Doc = parse(R"(
     {
@@ -318,49 +370,49 @@ TEST(JSONTest, Integers) {
     std::optional<int64_t> AsInt;
     std::optional<double> AsNumber;
   } TestCases[] = {
-    {
-        "Non-integer. Stored as double, not convertible.",
-        double{1.5},
-        "1.5",
-        std::nullopt,
-        1.5,
-    },
+      {
+          "Non-integer. Stored as double, not convertible.",
+          double{1.5},
+          "1.5",
+          std::nullopt,
+          1.5,
+      },
 
-    {
-        "Integer, not exact double. Stored as int64, convertible.",
-        int64_t{0x4000000000000001},
-        "4611686018427387905",
-        int64_t{0x4000000000000001},
-        double{0x4000000000000000},
-    },
+      {
+          "Integer, not exact double. Stored as int64, convertible.",
+          int64_t{0x4000000000000001},
+          "4611686018427387905",
+          int64_t{0x4000000000000001},
+          double{0x4000000000000000},
+      },
 
-    {
-        "Negative integer, not exact double. Stored as int64, convertible.",
-        int64_t{-0x4000000000000001},
-        "-4611686018427387905",
-        int64_t{-0x4000000000000001},
-        double{-0x4000000000000000},
-    },
-
-      // PR46470,
-      // https://developercommunity.visualstudio.com/content/problem/1093399/incorrect-result-when-printing-6917529027641081856.html
+      {
+          "Negative integer, not exact double. Stored as int64, convertible.",
+          int64_t{-0x4000000000000001},
+          "-4611686018427387905",
+          int64_t{-0x4000000000000001},
+          double{-0x4000000000000000},
+      },
+
+  // PR46470,
+  // https://developercommunity.visualstudio.com/content/problem/1093399/incorrect-result-when-printing-6917529027641081856.html
 #if !defined(_MSC_VER) || _MSC_VER < 1926
-    {
-        "Dynamically exact integer. Stored as double, convertible.",
-        double{0x6000000000000000},
-        "6.9175290276410819e+18",
-        int64_t{0x6000000000000000},
-        double{0x6000000000000000},
-    },
+      {
+          "Dynamically exact integer. Stored as double, convertible.",
+          double{0x6000000000000000},
+          "6.9175290276410819e+18",
+          int64_t{0x6000000000000000},
+          double{0x6000000000000000},
+      },
 #endif
 
-    {
-        "Dynamically integer, >64 bits. Stored as double, not convertible.",
-        1.5 * double{0x8000000000000000},
-        "1.3835058055282164e+19",
-        std::nullopt,
-        1.5 * double{0x8000000000000000},
-    },
+      {
+          "Dynamically integer, >64 bits. Stored as double, not convertible.",
+          1.5 * double{0x8000000000000000},
+          "1.3835058055282164e+19",
+          std::nullopt,
+          1.5 * double{0x8000000000000000},
+      },
   };
   for (const auto &T : TestCases) {
     EXPECT_EQ(T.Str, s(T.Val)) << T.Desc;



More information about the cfe-commits mailing list