https://github.com/ziqingluo-90 created https://github.com/llvm/llvm-project/pull/203432
This reverts commit 9f1e08fa8ed7bcf4b7cfaf9eaaa7c23a2d3ed347. It causes build error: https://lab.llvm.org/buildbot/#/builders/2/builds/53597. The use of 'setCurrentDebugType' should be guarded by '#ifndef NDEBUG' >From 6aeb74b59d2abc0d20a51a4e1df6351f7f2bccc4 Mon Sep 17 00:00:00 2001 From: Ziqing Luo <[email protected]> Date: Thu, 11 Jun 2026 18:11:42 -0700 Subject: [PATCH] Revert "[SSAF][Extractor] Make hard errors in PointerFlow and UnsafeBufferUsage Extractors quiet (#201953)" This reverts commit 9f1e08fa8ed7bcf4b7cfaf9eaaa7c23a2d3ed347. It causes build error: https://lab.llvm.org/buildbot/#/builders/2/builds/53597 The use of 'setCurrentDebugType' should be guarded by '#ifndef NDEBUG' --- .../PointerFlow/PointerFlowExtractor.cpp | 27 +++++------ .../Analyses/SSAFAnalysesCommon.h | 9 ---- .../UnsafeBufferUsageExtractor.cpp | 22 ++++----- .../Analyses/PointerFlow/PointerFlowTest.cpp | 45 +------------------ .../UnsafeBufferUsageTest.cpp | 26 ----------- 5 files changed, 25 insertions(+), 104 deletions(-) diff --git a/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp b/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp index 39cee76440de4..f039cebebdf0a 100644 --- a/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp +++ b/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp @@ -25,7 +25,6 @@ #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> @@ -108,10 +107,9 @@ 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()); + if (!RHS->empty()) + for (auto L : *LHS) + Results[L].insert(RHS->begin(), RHS->end()); return llvm::Error::success(); } @@ -314,8 +312,7 @@ class PointerFlowTUSummaryExtractor : public TUSummaryExtractor { PointerFlowTUSummaryExtractor(TUSummaryBuilder &Builder) : TUSummaryExtractor(Builder) {} - /// \return a non-null unique pointer to a PointerFlowEntitySummary - std::unique_ptr<PointerFlowEntitySummary> + Expected<std::unique_ptr<PointerFlowEntitySummary>> extractEntitySummary(const NamedDecl *Contributor, ASTContext &Ctx, TUSummaryExtractor &Extractor) { PointerFlowMatcher Matcher(Ctx, Extractor); @@ -323,7 +320,7 @@ class PointerFlowTUSummaryExtractor : public TUSummaryExtractor { auto Err = Matcher.matches(Node, Contributor); if (Err) - logWarningFromError(std::move(Err)); + llvm::report_fatal_error(std::move(Err)); }; findMatchesIn(Contributor, MatchAction); @@ -338,18 +335,18 @@ class PointerFlowTUSummaryExtractor : public TUSummaryExtractor { for (auto *CD : Contributors) { auto EntitySummary = extractEntitySummary(CD, Ctx, *this); - assert(EntitySummary); - if (EntitySummary->empty()) + if (!EntitySummary) + llvm::reportFatalInternalError(EntitySummary.takeError()); + assert(*EntitySummary); + if ((*EntitySummary)->empty()) continue; std::optional<EntityId> ContributorId = addEntity(CD); - if (!ContributorId) { - logWarningFromError(makeEntityNameErr(Ctx, CD)); - continue; - } + if (!ContributorId) + llvm::reportFatalInternalError(makeEntityNameErr(Ctx, CD)); [[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 f3aa30120ffe6..63755304ea3c9 100644 --- a/clang/lib/ScalableStaticAnalysisFramework/Analyses/SSAFAnalysesCommon.h +++ b/clang/lib/ScalableStaticAnalysisFramework/Analyses/SSAFAnalysesCommon.h @@ -15,10 +15,7 @@ #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 @@ -53,12 +50,6 @@ 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 be5e1f58fc019..f0d43c5968d05 100644 --- a/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp +++ b/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp @@ -17,6 +17,7 @@ #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; @@ -28,14 +29,13 @@ class UnsafeBufferUsageTUSummaryExtractor : public TUSummaryExtractor { UnsafeBufferUsageTUSummaryExtractor(TUSummaryBuilder &Builder) : TUSummaryExtractor(Builder) {} - /// \return a non-null unique pointer to a UnsafeBufferUsageEntitySummary - std::unique_ptr<UnsafeBufferUsageEntitySummary> + Expected<std::unique_ptr<UnsafeBufferUsageEntitySummary>> extractEntitySummary(const NamedDecl *Contributor, ASTContext &Ctx); void HandleTranslationUnit(ASTContext &Ctx) override; }; } // namespace clang::ssaf -std::unique_ptr<UnsafeBufferUsageEntitySummary> +Expected<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; } - logWarningFromError(Translation.takeError()); + return 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); - assert(EntitySummary); - if (EntitySummary->empty()) + if (!EntitySummary) + llvm::reportFatalInternalError(EntitySummary.takeError()); + assert(*EntitySummary); + if ((*EntitySummary)->empty()) continue; auto ContributorId = addEntity(CD); - if (!ContributorId) { - logWarningFromError(makeEntityNameErr(Ctx, CD)); - continue; - } + if (!ContributorId) + llvm::reportFatalInternalError(makeEntityNameErr(Ctx, CD)); [[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 5745cca72b7ff..c0622a0fa0998 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,45 +1263,4 @@ 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 284ec3a8471eb..1ef2c5e7337bf 100644 --- a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp +++ b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp @@ -20,8 +20,6 @@ #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> @@ -686,28 +684,4 @@ 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
