Author: Aviral Goel Date: 2026-02-25T05:02:50Z New Revision: be7aa6a2b7a487a4c9d6942abe3c78735025fac1
URL: https://github.com/llvm/llvm-project/commit/be7aa6a2b7a487a4c9d6942abe3c78735025fac1 DIFF: https://github.com/llvm/llvm-project/commit/be7aa6a2b7a487a4c9d6942abe3c78735025fac1.diff LOG: [clang][ssaf] Add checks for missing and mismatched `EntitySummary` data and improve code coverage This PR adds new checks and tests for null `EntitySummary`, and `SummaryName` match against `EntitySummary::getSummaryName()`. It also adds tests for `SummaryName` with no registered `FormatInfo`. As part of this change, the `JSONFormatTest` fixture has been made to inherit from `ssaf::TestFixture` to provide its subclasses direct access to the private-field accessors that TestFixture exposes. This brings JSONFormat.cpp to almost 100% coverage. The remaining uncovered lines are either in untestable paths or coverage instrumentation artifacts. Added: Modified: clang/include/clang/Analysis/Scalable/Serialization/JSONFormat.h clang/lib/Analysis/Scalable/Serialization/JSONFormat.cpp clang/unittests/Analysis/Scalable/Serialization/JSONFormatTest/JSONFormatTest.h clang/unittests/Analysis/Scalable/Serialization/JSONFormatTest/TUSummaryTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Analysis/Scalable/Serialization/JSONFormat.h b/clang/include/clang/Analysis/Scalable/Serialization/JSONFormat.h index 706ace7e8feb8..8d46bbf29863a 100644 --- a/clang/include/clang/Analysis/Scalable/Serialization/JSONFormat.h +++ b/clang/include/clang/Analysis/Scalable/Serialization/JSONFormat.h @@ -116,6 +116,10 @@ class JSONFormat final : public SerializationFormat { entityDataMapEntryFromJSON(const Object &EntityDataMapEntryObject, const SummaryName &SN, EntityIdTable &IdTable) const; + llvm::Expected<Object> + entityDataMapEntryToJSON(const EntityId EI, + const std::unique_ptr<EntitySummary> &EntitySummary, + const SummaryName &SN) const; llvm::Expected<std::map<EntityId, std::unique_ptr<EntitySummary>>> entityDataMapFromJSON(const SummaryName &SN, const Array &EntityDataArray, EntityIdTable &IdTable) const; diff --git a/clang/lib/Analysis/Scalable/Serialization/JSONFormat.cpp b/clang/lib/Analysis/Scalable/Serialization/JSONFormat.cpp index a72eb29a4039b..e2eda695a4eee 100644 --- a/clang/lib/Analysis/Scalable/Serialization/JSONFormat.cpp +++ b/clang/lib/Analysis/Scalable/Serialization/JSONFormat.cpp @@ -65,13 +65,26 @@ constexpr const char *FailedToReadObjectAtField = constexpr const char *FailedToReadObjectAtIndex = "failed to read {0} from index '{1}': expected JSON {2}"; -constexpr const char *FailedToDeserializeEntitySummary = +constexpr const char *FailedToDeserializeEntitySummaryNoFormatInfo = "failed to deserialize EntitySummary: no FormatInfo registered for summary " "'{0}'"; -constexpr const char *FailedToSerializeEntitySummary = +constexpr const char *FailedToSerializeEntitySummaryNoFormatInfo = "failed to serialize EntitySummary: no FormatInfo registered for summary " "'{0}'"; +constexpr const char *FailedToDeserializeEntitySummaryMissingData = + "failed to deserialize EntitySummary: null EntitySummary data for summary " + "'{0}'"; +constexpr const char *FailedToSerializeEntitySummaryMissingData = + "JSONFormat - null EntitySummary data for summary '{0}'"; + +constexpr const char *FailedToDeserializeEntitySummaryMismatchedSummaryName = + "failed to deserialize EntitySummary: EntitySummary data for summary '{0}' " + "reports mismatched summary '{1}'"; +constexpr const char *FailedToSerializeEntitySummaryMismatchedSummaryName = + "JSONFormat - EntitySummary data for summary '{0}' reports mismatched " + "summary '{1}'"; + constexpr const char *InvalidBuildNamespaceKind = "invalid 'kind' BuildNamespaceKind value '{0}'"; @@ -698,9 +711,10 @@ JSONFormat::entitySummaryFromJSON(const SummaryName &SN, EntityIdTable &IdTable) const { auto InfoIt = FormatInfos.find(SN); if (InfoIt == FormatInfos.end()) { - return ErrorBuilder::create(std::errc::invalid_argument, - ErrorMessages::FailedToDeserializeEntitySummary, - SN.str()) + return ErrorBuilder::create( + std::errc::invalid_argument, + ErrorMessages::FailedToDeserializeEntitySummaryNoFormatInfo, + SN.str()) .build(); } @@ -716,9 +730,10 @@ JSONFormat::entitySummaryToJSON(const SummaryName &SN, const EntitySummary &ES) const { auto InfoIt = FormatInfos.find(SN); if (InfoIt == FormatInfos.end()) { - return ErrorBuilder::create(std::errc::invalid_argument, - ErrorMessages::FailedToSerializeEntitySummary, - SN.str()) + return ErrorBuilder::create( + std::errc::invalid_argument, + ErrorMessages::FailedToSerializeEntitySummaryNoFormatInfo, + SN.str()) .build(); } @@ -776,9 +791,59 @@ JSONFormat::entityDataMapEntryFromJSON(const Object &EntityDataMapEntryObject, .build(); } + if (*ExpectedEntitySummary == nullptr) { + return ErrorBuilder::create( + std::errc::invalid_argument, + ErrorMessages::FailedToDeserializeEntitySummaryMissingData, + SN.str()) + .build(); + } + + auto ActualSN = (*ExpectedEntitySummary)->getSummaryName(); + if (SN != ActualSN) { + return ErrorBuilder::create( + std::errc::invalid_argument, + ErrorMessages:: + FailedToDeserializeEntitySummaryMismatchedSummaryName, + SN.str(), ActualSN.str()) + .build(); + } + return std::make_pair(std::move(EI), std::move(*ExpectedEntitySummary)); } +llvm::Expected<Object> JSONFormat::entityDataMapEntryToJSON( + const EntityId EI, const std::unique_ptr<EntitySummary> &EntitySummary, + const SummaryName &SN) const { + Object Entry; + + Entry["entity_id"] = entityIdToJSON(EI); + + if (!EntitySummary) { + ErrorBuilder::fatal( + ErrorMessages::FailedToSerializeEntitySummaryMissingData, SN.str()); + } + + const auto ActualSN = EntitySummary->getSummaryName(); + if (SN != ActualSN) { + ErrorBuilder::fatal( + ErrorMessages::FailedToSerializeEntitySummaryMismatchedSummaryName, + SN.str(), ActualSN.str()); + } + + auto ExpectedEntitySummaryObject = entitySummaryToJSON(SN, *EntitySummary); + if (!ExpectedEntitySummaryObject) { + return ErrorBuilder::wrap(ExpectedEntitySummaryObject.takeError()) + .context(ErrorMessages::WritingToField, "EntitySummary", + "entity_summary") + .build(); + } + + Entry["entity_summary"] = std::move(*ExpectedEntitySummaryObject); + + return Entry; +} + //---------------------------------------------------------------------------- // EntityDataMap //---------------------------------------------------------------------------- @@ -803,11 +868,12 @@ JSONFormat::entityDataMapFromJSON(const SummaryName &SN, auto ExpectedEntityDataMapEntry = entityDataMapEntryFromJSON(*OptEntityDataMapEntryObject, SN, IdTable); - if (!ExpectedEntityDataMapEntry) + if (!ExpectedEntityDataMapEntry) { return ErrorBuilder::wrap(ExpectedEntityDataMapEntry.takeError()) .context(ErrorMessages::ReadingFromIndex, "EntitySummary entry", Index) .build(); + } auto [DataIt, DataInserted] = EntityDataMap.insert(std::move(*ExpectedEntityDataMapEntry)); @@ -834,20 +900,16 @@ llvm::Expected<Array> JSONFormat::entityDataMapToJSON( llvm::enumerate(EntityDataMap)) { const auto &[EntityId, EntitySummary] = EntityDataMapEntry; - Object Entry; + auto ExpectedEntityDataMapEntryObject = + entityDataMapEntryToJSON(EntityId, EntitySummary, SN); - Entry["entity_id"] = entityIdToJSON(EntityId); - - auto ExpectedEntitySummaryObject = entitySummaryToJSON(SN, *EntitySummary); - if (!ExpectedEntitySummaryObject) { - return ErrorBuilder::wrap(ExpectedEntitySummaryObject.takeError()) + if (!ExpectedEntityDataMapEntryObject) { + return ErrorBuilder::wrap(ExpectedEntityDataMapEntryObject.takeError()) .context(ErrorMessages::WritingToIndex, "EntitySummary entry", Index) .build(); } - Entry["entity_summary"] = std::move(*ExpectedEntitySummaryObject); - - Result.push_back(std::move(Entry)); + Result.push_back(std::move(*ExpectedEntityDataMapEntryObject)); } return Result; diff --git a/clang/unittests/Analysis/Scalable/Serialization/JSONFormatTest/JSONFormatTest.h b/clang/unittests/Analysis/Scalable/Serialization/JSONFormatTest/JSONFormatTest.h index 22261e706f131..a317e8f23a1b9 100644 --- a/clang/unittests/Analysis/Scalable/Serialization/JSONFormatTest/JSONFormatTest.h +++ b/clang/unittests/Analysis/Scalable/Serialization/JSONFormatTest/JSONFormatTest.h @@ -13,6 +13,7 @@ #ifndef LLVM_CLANG_UNITTESTS_ANALYSIS_SCALABLE_SERIALIZATION_JSONFORMATTEST_JSONFORMATTEST_H #define LLVM_CLANG_UNITTESTS_ANALYSIS_SCALABLE_SERIALIZATION_JSONFORMATTEST_JSONFORMATTEST_H +#include "TestFixture.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" @@ -20,7 +21,6 @@ #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" -#include "gtest/gtest.h" #ifndef _WIN32 #include <unistd.h> #endif @@ -29,7 +29,7 @@ // Test Fixture // ============================================================================ -class JSONFormatTest : public ::testing::Test { +class JSONFormatTest : public clang::ssaf::TestFixture { public: using PathString = llvm::SmallString<128>; diff --git a/clang/unittests/Analysis/Scalable/Serialization/JSONFormatTest/TUSummaryTest.cpp b/clang/unittests/Analysis/Scalable/Serialization/JSONFormatTest/TUSummaryTest.cpp index 54742bd8d9892..287de203cc80c 100644 --- a/clang/unittests/Analysis/Scalable/Serialization/JSONFormatTest/TUSummaryTest.cpp +++ b/clang/unittests/Analysis/Scalable/Serialization/JSONFormatTest/TUSummaryTest.cpp @@ -27,7 +27,7 @@ using ::testing::HasSubstr; namespace { // ============================================================================ -// Test Analysis - Simple analysis for testing JSON serialization. +// PairsEntitySummaryForJSONFormatTest - Simple analysis for testing JSONFormat // ============================================================================ struct PairsEntitySummaryForJSONFormatTest final : EntitySummary { @@ -100,6 +100,81 @@ static llvm::Registry<JSONFormat::FormatInfo>::Add< "PairsEntitySummaryForJSONFormatTest", "Format info for PairsArrayEntitySummary"); +// ============================================================================ +// NullEntitySummaryForJSONFormatTest - For null data checks +// ============================================================================ + +struct NullEntitySummaryForJSONFormatTest final : EntitySummary { + SummaryName getSummaryName() const override { + return SummaryName("NullEntitySummaryForJSONFormatTest"); + } +}; + +struct NullEntitySummaryForJSONFormatTestFormatInfo final + : JSONFormat::FormatInfo { + NullEntitySummaryForJSONFormatTestFormatInfo() + : JSONFormat::FormatInfo( + SummaryName("NullEntitySummaryForJSONFormatTest"), + [](const EntitySummary &, const JSONFormat::EntityIdConverter &) + -> json::Object { return json::Object{}; }, + [](const json::Object &, EntityIdTable &, + const JSONFormat::EntityIdConverter &) + -> llvm::Expected<std::unique_ptr<EntitySummary>> { + return nullptr; + }) {} +}; + +static llvm::Registry<JSONFormat::FormatInfo>::Add< + NullEntitySummaryForJSONFormatTestFormatInfo> + RegisterNullEntitySummaryForJSONFormatTest( + "NullEntitySummaryForJSONFormatTest", + "Format info for NullEntitySummary"); + +// ============================================================================ +// UnregisteredEntitySummaryForJSONFormatTest - For missing FormatInfo checks +// ============================================================================ + +struct UnregisteredEntitySummaryForJSONFormatTest final : EntitySummary { + SummaryName getSummaryName() const override { + return SummaryName("UnregisteredEntitySummaryForJSONFormatTest"); + } +}; + +// ============================================================================ +// MismatchedEntitySummaryForJSONFormatTest - For mismatched SummaryName checks +// ============================================================================ + +struct MismatchedEntitySummaryForJSONFormatTest final : EntitySummary { + SummaryName getSummaryName() const override { + return SummaryName("MismatchedEntitySummaryForJSONFormatTest_WrongName"); + } +}; + +struct MismatchedEntitySummaryForJSONFormatTestFormatInfo final + : JSONFormat::FormatInfo { + MismatchedEntitySummaryForJSONFormatTestFormatInfo() + : JSONFormat::FormatInfo( + SummaryName("MismatchedEntitySummaryForJSONFormatTest"), + [](const EntitySummary &, const JSONFormat::EntityIdConverter &) + -> json::Object { return json::Object{}; }, + [](const json::Object &, EntityIdTable &, + const JSONFormat::EntityIdConverter &) + -> llvm::Expected<std::unique_ptr<EntitySummary>> { + return std::make_unique< + MismatchedEntitySummaryForJSONFormatTest>(); + }) {} +}; + +static llvm::Registry<JSONFormat::FormatInfo>::Add< + MismatchedEntitySummaryForJSONFormatTestFormatInfo> + RegisterMismatchedEntitySummaryForJSONFormatTest( + "MismatchedEntitySummaryForJSONFormatTest", + "Format info for MismatchedEntitySummary"); + +// ============================================================================ +// JSONFormatTUSummaryTest Test Fixture +// ============================================================================ + class JSONFormatTUSummaryTest : public JSONFormatTest { protected: llvm::Expected<TUSummary> readTUSummaryFromFile(StringRef FileName) const { @@ -1168,7 +1243,7 @@ TEST_F(JSONFormatTUSummaryTest, DuplicateEntity) { // JSONFormat::entitySummaryFromJSON() Error Tests // ============================================================================ -TEST_F(JSONFormatTUSummaryTest, EntitySummaryNoFormatInfo) { +TEST_F(JSONFormatTUSummaryTest, ReadEntitySummaryNoFormatInfo) { auto Result = readTUSummaryFromString(R"({ "tu_namespace": { "kind": "compilation_unit", @@ -1178,7 +1253,7 @@ TEST_F(JSONFormatTUSummaryTest, EntitySummaryNoFormatInfo) { "linkage_table": [], "data": [ { - "summary_name": "unknown_summary_type", + "summary_name": "UnregisteredEntitySummaryForJSONFormatTest", "summary_data": [ { "entity_id": 0, @@ -1199,8 +1274,8 @@ TEST_F(JSONFormatTUSummaryTest, EntitySummaryNoFormatInfo) { HasSubstr("reading EntitySummary entry from index '0'"), HasSubstr("reading EntitySummary from field 'entity_summary'"), HasSubstr("failed to deserialize EntitySummary"), - HasSubstr( - "no FormatInfo registered for summary 'unknown_summary_type'")))); + HasSubstr("no FormatInfo registered for summary " + "'UnregisteredEntitySummaryForJSONFormatTest'")))); } // ============================================================================ @@ -1576,6 +1651,119 @@ TEST_F(JSONFormatTUSummaryTest, EntityIDNotUInt64) { HasSubstr("expected JSON integer")))); } +TEST_F(JSONFormatTUSummaryTest, ReadEntitySummaryMissingData) { + auto Result = readTUSummaryFromString(R"({ + "tu_namespace": { + "kind": "compilation_unit", + "name": "test.cpp" + }, + "id_table": [], + "linkage_table": [], + "data": [ + { + "summary_name": "NullEntitySummaryForJSONFormatTest", + "summary_data": [ + { + "entity_id": 0, + "entity_summary": {} + } + ] + } + ] + })"); + + EXPECT_THAT_EXPECTED( + Result, + FailedWithMessage(AllOf( + HasSubstr("reading TUSummary from file"), + HasSubstr("reading SummaryData entries from field 'data'"), + HasSubstr("reading SummaryData entry from index '0'"), + HasSubstr("reading EntitySummary entries from field 'summary_data'"), + HasSubstr("reading EntitySummary entry from index '0'"), + HasSubstr("failed to deserialize EntitySummary"), + HasSubstr("null EntitySummary data for summary " + "'NullEntitySummaryForJSONFormatTest'")))); +} + +TEST_F(JSONFormatTUSummaryTest, ReadEntitySummaryMismatchedSummaryName) { + auto Result = readTUSummaryFromString(R"({ + "tu_namespace": { + "kind": "compilation_unit", + "name": "test.cpp" + }, + "id_table": [], + "linkage_table": [], + "data": [ + { + "summary_name": "MismatchedEntitySummaryForJSONFormatTest", + "summary_data": [ + { + "entity_id": 0, + "entity_summary": {} + } + ] + } + ] + })"); + + EXPECT_THAT_EXPECTED( + Result, + FailedWithMessage(AllOf( + HasSubstr("reading TUSummary from file"), + HasSubstr("reading SummaryData entries from field 'data'"), + HasSubstr("reading SummaryData entry from index '0'"), + HasSubstr("reading EntitySummary entries from field 'summary_data'"), + HasSubstr("reading EntitySummary entry from index '0'"), + HasSubstr("failed to deserialize EntitySummary"), + HasSubstr("EntitySummary data for summary " + "'MismatchedEntitySummaryForJSONFormatTest' reports " + "mismatched summary " + "'MismatchedEntitySummaryForJSONFormatTest_WrongName'")))); +} + +// ============================================================================ +// JSONFormat::entityDataMapEntryToJSON() Fatal Tests +// ============================================================================ + +TEST_F(JSONFormatTUSummaryTest, WriteEntitySummaryMissingData) { + TUSummary Summary( + BuildNamespace(BuildNamespaceKind::CompilationUnit, "test.cpp")); + + NestedBuildNamespace Namespace = + NestedBuildNamespace::makeCompilationUnit("test.cpp"); + EntityId EI = getIdTable(Summary).getId( + EntityName{"c:@F@foo", "", std::move(Namespace)}); + + SummaryName SN("NullEntitySummaryForJSONFormatTest"); + getData(Summary)[SN][EI] = nullptr; + + EXPECT_DEATH( + { (void)writeTUSummary(Summary, "output.json"); }, + "JSONFormat - null EntitySummary data for summary " + "'NullEntitySummaryForJSONFormatTest'"); +} + +TEST_F(JSONFormatTUSummaryTest, WriteEntitySummaryMismatchedSummaryName) { + TUSummary Summary( + BuildNamespace(BuildNamespaceKind::CompilationUnit, "test.cpp")); + + NestedBuildNamespace Namespace = + NestedBuildNamespace::makeCompilationUnit("test.cpp"); + EntityId EI = getIdTable(Summary).getId( + EntityName{"c:@F@foo", "", std::move(Namespace)}); + + SummaryName SN("MismatchedEntitySummaryForJSONFormatTest"); + getData(Summary)[SN][EI] = + std::make_unique<MismatchedEntitySummaryForJSONFormatTest>(); + + EXPECT_DEATH( + { (void)writeTUSummary(Summary, "output.json"); }, + "JSONFormat - EntitySummary data for summary " + "'MismatchedEntitySummaryForJSONFormatTest' reports " + "mismatched summary " + "'MismatchedEntitySummaryForJSONFormatTest_WrongName'"); +} + // ============================================================================ // JSONFormat::entityDataMapFromJSON() Error Tests // ============================================================================ @@ -1944,6 +2132,38 @@ TEST_F(JSONFormatTUSummaryTest, WriteStreamOpenFailure) { EXPECT_THAT_ERROR(std::move(RestoreError), Succeeded()); } +// ============================================================================ +// JSONFormat::writeTUSummary() Error Tests +// ============================================================================ + +TEST_F(JSONFormatTUSummaryTest, WriteEntitySummaryNoFormatInfo) { + TUSummary Summary( + BuildNamespace(BuildNamespaceKind::CompilationUnit, "test.cpp")); + + NestedBuildNamespace Namespace = + NestedBuildNamespace::makeCompilationUnit("test.cpp"); + EntityId EI = getIdTable(Summary).getId( + EntityName{"c:@F@foo", "", std::move(Namespace)}); + + SummaryName UnknownSN("UnregisteredEntitySummaryForJSONFormatTest"); + getData(Summary)[UnknownSN][EI] = + std::make_unique<UnregisteredEntitySummaryForJSONFormatTest>(); + + auto Result = writeTUSummary(Summary, "output.json"); + + EXPECT_THAT_ERROR( + std::move(Result), + FailedWithMessage( + AllOf(HasSubstr("writing TUSummary to file"), + HasSubstr("writing SummaryData entry to index '0'"), + HasSubstr("writing EntitySummary entries to field " + "'summary_data'"), + HasSubstr("writing EntitySummary entry to index '0'"), + HasSubstr("failed to serialize EntitySummary"), + HasSubstr("no FormatInfo registered for summary " + "'UnregisteredEntitySummaryForJSONFormatTest'")))); +} + // ============================================================================ // Round-Trip Tests - Serialization Verification // ============================================================================ _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
