https://github.com/ziqingluo-90 updated https://github.com/llvm/llvm-project/pull/201953
>From 34555beef6657856fc284d8b7feb3205888cdb08 Mon Sep 17 00:00:00 2001 From: Ziqing Luo <[email protected]> Date: Thu, 4 Jun 2026 14:25:47 -0700 Subject: [PATCH 1/2] [SSAF][Extractor] Make hard errors in Extractors quiet Hard errors were used in extractors during development to quickly identify unsupported language constructs. This commit converts them to DEBUG_WITH_TYPE so that these errors are silenced in release builds. rdar://178747892 --- .../PointerFlow/PointerFlowExtractor.cpp | 19 ++++++------ .../Analyses/SSAFAnalysesCommon.h | 9 ++++++ .../UnsafeBufferUsageExtractor.cpp | 21 ++++++------- .../Analyses/PointerFlow/PointerFlowTest.cpp | 31 +++++++++++++++++++ .../UnsafeBufferUsageTest.cpp | 25 +++++++++++++++ 5 files changed, 85 insertions(+), 20 deletions(-) diff --git a/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp b/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp index e1130a2c52e4c..e79a4d4c6db4c 100644 --- a/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp +++ b/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp @@ -25,6 +25,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/Sequence.h" +#include "llvm/Support/Debug.h" #include "llvm/Support/Error.h" #include <memory> #include <optional> @@ -308,7 +309,7 @@ class PointerFlowTUSummaryExtractor : public TUSummaryExtractor { PointerFlowTUSummaryExtractor(TUSummaryBuilder &Builder) : TUSummaryExtractor(Builder) {} - Expected<std::unique_ptr<PointerFlowEntitySummary>> + std::unique_ptr<PointerFlowEntitySummary> extractEntitySummary(const NamedDecl *Contributor, ASTContext &Ctx, TUSummaryExtractor &Extractor) { PointerFlowMatcher Matcher(Ctx, Extractor); @@ -316,7 +317,7 @@ class PointerFlowTUSummaryExtractor : public TUSummaryExtractor { auto Err = Matcher.matches(Node, Contributor); if (Err) - llvm::report_fatal_error(std::move(Err)); + logWarningFromError(std::move(Err)); }; findMatchesIn(Contributor, MatchAction); @@ -331,18 +332,18 @@ class PointerFlowTUSummaryExtractor : public TUSummaryExtractor { for (auto *CD : Contributors) { auto EntitySummary = extractEntitySummary(CD, Ctx, *this); - if (!EntitySummary) - llvm::reportFatalInternalError(EntitySummary.takeError()); - assert(*EntitySummary); - if ((*EntitySummary)->empty()) + assert(EntitySummary); + if (EntitySummary->empty()) continue; std::optional<EntityId> ContributorId = addEntity(CD); - if (!ContributorId) - llvm::reportFatalInternalError(makeEntityNameErr(Ctx, CD)); + if (!ContributorId) { + logWarningFromError(makeEntityNameErr(Ctx, CD)); + continue; + } [[maybe_unused]] auto [_, InsertionSucceeded] = - SummaryBuilder.addSummary(*ContributorId, std::move(*EntitySummary)); + SummaryBuilder.addSummary(*ContributorId, std::move(EntitySummary)); assert(InsertionSucceeded && "duplicated contributor extraction"); } diff --git a/clang/lib/ScalableStaticAnalysisFramework/Analyses/SSAFAnalysesCommon.h b/clang/lib/ScalableStaticAnalysisFramework/Analyses/SSAFAnalysesCommon.h index 63755304ea3c9..f3aa30120ffe6 100644 --- a/clang/lib/ScalableStaticAnalysisFramework/Analyses/SSAFAnalysesCommon.h +++ b/clang/lib/ScalableStaticAnalysisFramework/Analyses/SSAFAnalysesCommon.h @@ -15,7 +15,10 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/ASTTypeTraits.h" #include "clang/AST/Decl.h" +#include "llvm/Support/Debug.h" +#include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" +#include "llvm/Support/raw_ostream.h" namespace clang::ssaf { ///\return a short descriptions of a json::Value @@ -50,6 +53,12 @@ template <typename DeclOrExpr> bool hasPtrOrArrType(const DeclOrExpr *E) { llvm::Error makeEntityNameErr(clang::ASTContext &Ctx, const clang::NamedDecl *D); +/// Log a warning from an llvm::Error +inline void logWarningFromError(llvm::Error Err) { + DEBUG_WITH_TYPE("ssaf-analyses", llvm::errs() << Err); + llvm::consumeError(std::move(Err)); +} + /// Find all contributors in an AST. void findContributors(ASTContext &Ctx, std::vector<const NamedDecl *> &Contributors); diff --git a/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp b/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp index 9103778d585fb..e3185f775031d 100644 --- a/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp +++ b/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp @@ -17,7 +17,6 @@ #include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryBuilder.h" #include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryExtractor.h" #include "llvm/ADT/STLExtras.h" -#include "llvm/Support/Error.h" #include "llvm/Support/ErrorHandling.h" using namespace clang; @@ -29,13 +28,13 @@ class UnsafeBufferUsageTUSummaryExtractor : public TUSummaryExtractor { UnsafeBufferUsageTUSummaryExtractor(TUSummaryBuilder &Builder) : TUSummaryExtractor(Builder) {} - Expected<std::unique_ptr<UnsafeBufferUsageEntitySummary>> + std::unique_ptr<UnsafeBufferUsageEntitySummary> extractEntitySummary(const NamedDecl *Contributor, ASTContext &Ctx); void HandleTranslationUnit(ASTContext &Ctx) override; }; } // namespace clang::ssaf -Expected<std::unique_ptr<UnsafeBufferUsageEntitySummary>> +std::unique_ptr<UnsafeBufferUsageEntitySummary> clang::ssaf::UnsafeBufferUsageTUSummaryExtractor::extractEntitySummary( const NamedDecl *Contributor, ASTContext &Ctx) { std::set<const Expr *> UnsafePointers; @@ -61,7 +60,7 @@ clang::ssaf::UnsafeBufferUsageTUSummaryExtractor::extractEntitySummary( Results.insert(FilteredTranslation.begin(), FilteredTranslation.end()); continue; } - return Translation.takeError(); + logWarningFromError(Translation.takeError()); } return std::make_unique<UnsafeBufferUsageEntitySummary>( @@ -76,19 +75,19 @@ void clang::ssaf::UnsafeBufferUsageTUSummaryExtractor::HandleTranslationUnit( for (auto *CD : Contributors) { auto EntitySummary = extractEntitySummary(CD, Ctx); - if (!EntitySummary) - llvm::reportFatalInternalError(EntitySummary.takeError()); - assert(*EntitySummary); - if ((*EntitySummary)->empty()) + assert(EntitySummary); + if (EntitySummary->empty()) continue; auto ContributorId = addEntity(CD); - if (!ContributorId) - llvm::reportFatalInternalError(makeEntityNameErr(Ctx, CD)); + if (!ContributorId) { + logWarningFromError(makeEntityNameErr(Ctx, CD)); + continue; + } [[maybe_unused]] auto [Ignored, InsertionSucceeded] = - SummaryBuilder.addSummary(*ContributorId, std::move(*EntitySummary)); + SummaryBuilder.addSummary(*ContributorId, std::move(EntitySummary)); assert(InsertionSucceeded && "duplicated contributor extraction"); } diff --git a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp index 9f7a508ea35fd..019f552664655 100644 --- a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp +++ b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp @@ -21,6 +21,7 @@ #include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryBuilder.h" #include "clang/Tooling/Tooling.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/Debug.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include <memory> @@ -1126,4 +1127,34 @@ TEST_F(PointerFlowTest, NestedLambdaAssign) { ASSERT_NE(Sum, nullptr); EXPECT_EQ(*Sum, makeEdges(__LINE__, {{{"y", 1U}, {"x", 1U}}})); } + +////////////////////////////////////////////////////////////// +// Robustness Tests (No Crash Tests) // +////////////////////////////////////////////////////////////// + +TEST_F(PointerFlowTest, StructuredBindingWithPointers) { + StringRef Code = R"cpp( + void foo() { + int *a[2]; + auto [ptr1, ptr2] = a; + ptr1[5]; + ptr2[3]; + } + )cpp"; + + // BindingDecl may not be fully supported, but should not crash + llvm::DebugFlag = true; + llvm::setCurrentDebugType("ssaf-analyses"); + testing::internal::CaptureStderr(); + + ASSERT_EQ(setUpTest(Code), true); + + std::string Output = testing::internal::GetCapturedStderr(); + llvm::DebugFlag = false; + + // Verify the warning was logged + EXPECT_NE(Output.find("failed to create EntityId for Decomposition"), + std::string::npos); +} + } // namespace diff --git a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp index 6b75ed3f1a1fe..14d70f35bd8ce 100644 --- a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp +++ b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp @@ -20,6 +20,7 @@ #include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryExtractor.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/Support/Debug.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include <initializer_list> @@ -570,4 +571,28 @@ TEST_F(UnsafeBufferUsageTest, NestedDefinitions2) { EXPECT_EQ(Sum, nullptr); } +// Robustness test: unsupported constructs will not cause crash +TEST_F(UnsafeBufferUsageTest, DirectNewExpressionArrayAccess) { + // 'new' not yet supported, but should not crash and should log warning + llvm::DebugFlag = true; + llvm::setCurrentDebugType("ssaf-analyses"); + testing::internal::CaptureStderr(); + + ASSERT_TRUE(setUpTest(R"cpp( + void foo(int i) { + (new int[2])[i]; + } + )cpp")); + + std::string Output = testing::internal::GetCapturedStderr(); + llvm::DebugFlag = false; + + // The only unsafe pointer is unsupported, so no summary should be produced. + EXPECT_EQ(getEntitySummary("foo"), nullptr); + // Verify the warning was logged + EXPECT_NE( + Output.find("attempt to translate CXXNewExpr to EntityPointerLevels"), + std::string::npos); +} + } // namespace >From 1a893407a0b5d6cfa30e6fdba15195c38a7d6cca Mon Sep 17 00:00:00 2001 From: Ziqing Luo <[email protected]> Date: Fri, 5 Jun 2026 17:47:17 -0700 Subject: [PATCH 2/2] Enforce an EdgeSet (implemented as a map) invariant: each map entry represents at least one edge. That is, a map key should not exist if the corresponding value is empty. --- .../PointerFlow/PointerFlowExtractor.cpp | 2 ++ .../Analyses/PointerFlow/PointerFlowTest.cpp | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp b/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp index e79a4d4c6db4c..b4b957a9e49b4 100644 --- a/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp +++ b/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp @@ -108,6 +108,8 @@ PointerFlowMatcher::addEdges(Expected<EntityPointerLevelSet> &&LHS, return LHS.takeError(); if (!RHS) return RHS.takeError(); + if (RHS->empty()) + return llvm::Error::success(); for (auto L : *LHS) Results[L].insert(RHS->begin(), RHS->end()); return llvm::Error::success(); diff --git a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp index 019f552664655..0270a0bd5bd64 100644 --- a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp +++ b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp @@ -1157,4 +1157,21 @@ TEST_F(PointerFlowTest, StructuredBindingWithPointers) { std::string::npos); } +TEST_F(PointerFlowTest, RHSResultsInNoEntityPointerLevel) { + ASSERT_EQ(setUpTest(R"cpp( + void f() { + int *p = new int[10]; + const char *q = "hello"; + } + struct S { + int *p; + void g() { p = (int *)this; } + }; + )cpp"), + true); + auto *Sum = getEntitySummary<FunctionDecl>("f"); + ASSERT_EQ(Sum, nullptr); + Sum = getEntitySummary<CXXMethodDecl>("g"); + ASSERT_EQ(Sum, nullptr); +} } // namespace _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
