Author: Ziqing Luo Date: 2026-06-12T12:16:45-07:00 New Revision: 10508afd0b14ffcf6819665e31d64d77ccd45d3f
URL: https://github.com/llvm/llvm-project/commit/10508afd0b14ffcf6819665e31d64d77ccd45d3f DIFF: https://github.com/llvm/llvm-project/commit/10508afd0b14ffcf6819665e31d64d77ccd45d3f.diff LOG: Reland "[SSAF][Extractor] Make hard errors in PointerFlow and UnsafeBufferUsage Extractors quiet (#201953)" (#203602) Reverted 7dcd1d2ad104c3f9748370a42dc775cd6e7e34dc and added '#ifndef NDEBUG' guards for tests using 'llvm::setCurrentDebugType'. Original message: 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 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 f039cebebdf0a..39cee76440de4 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(); } @@ -312,7 +314,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); @@ -320,7 +323,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); @@ -335,18 +338,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 c0622a0fa0998..e5512288491c5 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> @@ -1263,4 +1263,47 @@ TEST_F(PointerFlowTest, CXXConstructExprArrayInit) { ASSERT_NE(Sum, nullptr); EXPECT_EQ(*Sum, makeEdges(__LINE__, {{{"q", 1U}, {"arr", 1U}}})); } + +////////////////////////////////////////////////////////////// +// Robustness Tests (No Crash Tests) // +////////////////////////////////////////////////////////////// + +#ifndef NDEBUG +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")); +} +#endif + +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..4a8fe5a74b58c 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,30 @@ TEST_F(UnsafeBufferUsageTest, CXXScalarValueInitExpr) { ASSERT_NE(Sum, nullptr); EXPECT_EQ(*Sum, makeSet(__LINE__, {{"q", 1U}})); } + +// Robustness test: unsupported constructs will not cause crash +#ifndef NDEBUG +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")); +} +#endif + } // namespace _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
