Author: Ziqing Luo Date: 2026-06-11T16:46:23-07:00 New Revision: 9f1e08fa8ed7bcf4b7cfaf9eaaa7c23a2d3ed347
URL: https://github.com/llvm/llvm-project/commit/9f1e08fa8ed7bcf4b7cfaf9eaaa7c23a2d3ed347 DIFF: https://github.com/llvm/llvm-project/commit/9f1e08fa8ed7bcf4b7cfaf9eaaa7c23a2d3ed347.diff LOG: [SSAF][Extractor] Make hard errors in PointerFlow and UnsafeBufferUsage Extractors quiet (#201953) 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. In addition, translating unsupported language constructs now silently results in an empty EntityPointerLevelSet. The PointerFlowExtractor will skip empty sets for either the source or the destination when building edges to avoid an ill-formed edge set data structure. rdar://178747892 --------- Co-authored-by: Balázs Benics <[email protected]> Added: Modified: clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp clang/lib/ScalableStaticAnalysisFramework/Analyses/SSAFAnalysesCommon.h clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp Removed: ################################################################################ diff --git a/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp b/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp index 6fb2f1dd16ef8..6d725790d3cc3 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> @@ -107,9 +108,10 @@ PointerFlowMatcher::addEdges(Expected<EntityPointerLevelSet> &&LHS, return LHS.takeError(); if (!RHS) return RHS.takeError(); - if (!RHS->empty()) - for (auto L : *LHS) - Results[L].insert(RHS->begin(), RHS->end()); + if (RHS->empty()) + return llvm::Error::success(); + for (auto L : *LHS) + Results[L].insert(RHS->begin(), RHS->end()); return llvm::Error::success(); } @@ -311,7 +313,8 @@ class PointerFlowTUSummaryExtractor : public TUSummaryExtractor { PointerFlowTUSummaryExtractor(TUSummaryBuilder &Builder) : TUSummaryExtractor(Builder) {} - Expected<std::unique_ptr<PointerFlowEntitySummary>> + /// \return a non-null unique pointer to a PointerFlowEntitySummary + std::unique_ptr<PointerFlowEntitySummary> extractEntitySummary(const NamedDecl *Contributor, ASTContext &Ctx, TUSummaryExtractor &Extractor) { PointerFlowMatcher Matcher(Ctx, Extractor); @@ -319,7 +322,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); @@ -334,18 +337,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 f0d43c5968d05..be5e1f58fc019 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,14 @@ class UnsafeBufferUsageTUSummaryExtractor : public TUSummaryExtractor { UnsafeBufferUsageTUSummaryExtractor(TUSummaryBuilder &Builder) : TUSummaryExtractor(Builder) {} - Expected<std::unique_ptr<UnsafeBufferUsageEntitySummary>> + /// \return a non-null unique pointer to a 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 +61,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 +76,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 bff9dc10bfc1f..29864f2c22fd8 100644 --- a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp +++ b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp @@ -13,14 +13,14 @@ #include "clang/AST/DynamicRecursiveASTVisitor.h" #include "clang/AST/ExprCXX.h" #include "clang/Frontend/ASTUnit.h" -#include "clang/ScalableStaticAnalysisFramework/Core/ASTEntityMapping.h" #include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityId.h" -#include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityName.h" #include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/ExtractorRegistry.h" #include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummary.h" #include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryBuilder.h" #include "clang/Tooling/Tooling.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/Debug.h" +#include "llvm/Support/SaveAndRestore.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include <memory> @@ -1193,4 +1193,45 @@ TEST_F(PointerFlowTest, CXXConstructExprArrayInit) { ASSERT_NE(Sum, nullptr); EXPECT_EQ(*Sum, makeEdges(__LINE__, {{{"q", 1U}, {"arr", 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::SaveAndRestore<bool> DebugFlag(llvm::DebugFlag, true); + llvm::setCurrentDebugType("ssaf-analyses"); + testing::internal::CaptureStderr(); + + ASSERT_TRUE(setUpTest(Code)); + // Verify the warning was logged + ASSERT_TRUE(StringRef(testing::internal::GetCapturedStderr()) + .contains("failed to create EntityId for Decomposition")); +} + +TEST_F(PointerFlowTest, RHSResultsInNoEntityPointerLevel) { + ASSERT_TRUE(setUpTest(R"cpp( + void f() { + int *p = new int[10]; + const char *q = "hello"; + } + struct S { + int *p; + void g() { p = (int *)this; } + }; + )cpp")); + ASSERT_FALSE(getEntitySummary<FunctionDecl>("f")); + ASSERT_FALSE(getEntitySummary<CXXMethodDecl>("g")); +} + } // namespace diff --git a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp index 1ef2c5e7337bf..284ec3a8471eb 100644 --- a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp +++ b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp @@ -20,6 +20,8 @@ #include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryExtractor.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/Support/Debug.h" +#include "llvm/Support/SaveAndRestore.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include <initializer_list> @@ -684,4 +686,28 @@ TEST_F(UnsafeBufferUsageTest, CXXScalarValueInitExpr) { ASSERT_NE(Sum, nullptr); EXPECT_EQ(*Sum, makeSet(__LINE__, {{"q", 1U}})); } + +// Robustness test: unsupported constructs will not cause crash +TEST_F(UnsafeBufferUsageTest, StmtExprArrayAccess) { + // GNU statement expressions are not supported, but should not crash and + // should log a warning. + llvm::SaveAndRestore<bool> DebugFlag(llvm::DebugFlag, true); + + llvm::setCurrentDebugType("ssaf-analyses"); + testing::internal::CaptureStderr(); + + ASSERT_TRUE(setUpTest(R"cpp( + void foo(int i) { + ({ int *p = 0; p; })[i]; + } + )cpp")); + + // The only unsafe pointer is unsupported, so no summary should be produced. + ASSERT_FALSE(getEntitySummary("foo")); + // Verify the warning was logged + EXPECT_TRUE( + StringRef(testing::internal::GetCapturedStderr()) + .contains("attempt to translate StmtExpr to EntityPointerLevels")); +} + } // namespace _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
