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 <[email protected]> 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; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
