https://github.com/ziqingluo-90 updated https://github.com/llvm/llvm-project/pull/184899
>From cb8c65a3e04198b27b9706d52b4dd4a99478a240 Mon Sep 17 00:00:00 2001 From: Ziqing Luo <[email protected]> Date: Thu, 12 Mar 2026 14:10:19 -0700 Subject: [PATCH 1/4] Reapply "[clang][ssaf] Add UnsafeBufferUsage summary extractor for functions (#182941)" This reverts commit 53739c75a8720aaef8032628267ed4fd050af038. Reapply after module dependency issues are resolved. (rdar://169191570) --- .../UnsafeBufferUsage/UnsafeBufferUsage.h | 56 +-- .../UnsafeBufferUsageExtractor.h | 40 ++ .../UnsafeBufferUsageBuilder.h | 32 -- .../Analyses/CMakeLists.txt | 15 + .../UnsafeBufferUsageExtractor.cpp | 281 ++++++++++++ .../CMakeLists.txt | 2 + .../UnsafeBufferUsageTest.cpp | 423 ++++++++++++++++-- .../CMakeLists.txt | 1 + 8 files changed, 743 insertions(+), 107 deletions(-) rename clang/include/clang/ScalableStaticAnalysisFramework/{Core => }/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h (62%) create mode 100644 clang/include/clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.h delete mode 100644 clang/include/clang/ScalableStaticAnalysisFramework/Core/Analyses/UnsafeBufferUsage/UnsafeBufferUsageBuilder.h create mode 100644 clang/lib/ScalableStaticAnalysisFramework/Analyses/CMakeLists.txt create mode 100644 clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp diff --git a/clang/include/clang/ScalableStaticAnalysisFramework/Core/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h b/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h similarity index 62% rename from clang/include/clang/ScalableStaticAnalysisFramework/Core/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h rename to clang/include/clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h index 5b58ed0684333..5f418000723b4 100644 --- a/clang/include/clang/ScalableStaticAnalysisFramework/Core/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h +++ b/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h @@ -6,13 +6,12 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_CORE_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGE_H -#define LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_CORE_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGE_H +#ifndef LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGE_H +#define LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGE_H #include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityId.h" #include "clang/ScalableStaticAnalysisFramework/Core/Model/SummaryName.h" #include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/EntitySummary.h" -#include "llvm/ADT/iterator_range.h" #include <set> namespace clang::ssaf { @@ -26,23 +25,19 @@ namespace clang::ssaf { /// *' of 'p'. /// /// An EntityPointerLevel can be identified by an EntityId and an unsigned -/// integer indicating the pointer level: '(EntityId, PointerLevel)'. An -/// EntityPointerLevel 'P' is valid iff -/// - 'P.EntityId' has a pointer type with at least 'P.PointerLevel' levels -/// (This implies 'P.PointerLevel > 0'); -/// - 'P.EntityId' identifies an lvalue object and 'P.PointerLevel == 0'. -/// The latter case represents address-of expressions. +/// integer indicating the pointer level: '(EntityId, PointerLevel)'. +/// An EntityPointerLevel 'P' is valid iff 'P.EntityId' has a pointer type with +/// at least 'P.PointerLevel' levels (This implies 'P.PointerLevel > 0'). /// /// For the same example 'int *p[10];', the EntityPointerLevels below are valid: -/// '(p, 1)' is associated with 'int *[10]' of 'p'; -/// '(p, 2)' is associated with 'int *' of 'p'; -/// '(p, 0)' represents '&p'. +/// - '(p, 2)' is associated with the 'int *' part of the declared type of 'p'; +/// - '(p, 1)' is associated with the 'int *[10]' part of the declared type of +/// 'p'. class EntityPointerLevel { EntityId Entity; unsigned PointerLevel; - friend class UnsafeBufferUsageTUSummaryBuilder; - friend class UnsafeBufferUsageEntitySummary; + friend class UnsafeBufferUsageTUSummaryExtractor; EntityPointerLevel(EntityId Entity, unsigned PointerLevel) : Entity(Entity), PointerLevel(PointerLevel) {} @@ -52,7 +47,8 @@ class EntityPointerLevel { unsigned getPointerLevel() const { return PointerLevel; } bool operator==(const EntityPointerLevel &Other) const { - return Entity == Other.Entity && PointerLevel == Other.PointerLevel; + return std::tie(Entity, PointerLevel) == + std::tie(Other.Entity, Other.PointerLevel); } bool operator!=(const EntityPointerLevel &Other) const { @@ -64,7 +60,8 @@ class EntityPointerLevel { std::tie(Other.Entity, Other.PointerLevel); } - // Comparator supporting partial comparison against EntityId: + /// Compares `EntityPointerLevel`s; additionally, partially compares + /// `EntityPointerLevel` with `EntityId`. struct Comparator { using is_transparent = void; bool operator()(const EntityPointerLevel &L, @@ -88,33 +85,20 @@ using EntityPointerLevelSet = class UnsafeBufferUsageEntitySummary final : public EntitySummary { const EntityPointerLevelSet UnsafeBuffers; - friend class UnsafeBufferUsageTUSummaryBuilder; + friend class UnsafeBufferUsageTUSummaryExtractor; - UnsafeBufferUsageEntitySummary(EntityPointerLevelSet &&UnsafeBuffers) + UnsafeBufferUsageEntitySummary(EntityPointerLevelSet UnsafeBuffers) : EntitySummary(), UnsafeBuffers(std::move(UnsafeBuffers)) {} public: - using const_iterator = EntityPointerLevelSet::const_iterator; - - const_iterator begin() const { return UnsafeBuffers.begin(); } - const_iterator end() const { return UnsafeBuffers.end(); } - - const_iterator find(const EntityPointerLevel &V) const { - return UnsafeBuffers.find(V); - } - - llvm::iterator_range<const_iterator> getSubsetOf(EntityId Entity) const { - return llvm::make_range(UnsafeBuffers.equal_range(Entity)); - } - - /// \return the size of the set of EntityLevelPointers, which represents the - /// set of unsafe buffers - size_t getNumUnsafeBuffers() { return UnsafeBuffers.size(); } - SummaryName getSummaryName() const override { return SummaryName{"UnsafeBufferUsage"}; }; + + bool operator==(const EntityPointerLevelSet &Other) const { + return UnsafeBuffers == Other; + } }; } // namespace clang::ssaf -#endif // LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_CORE_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGE_H +#endif // LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGE_H diff --git a/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.h b/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.h new file mode 100644 index 0000000000000..765b2c37562ce --- /dev/null +++ b/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.h @@ -0,0 +1,40 @@ +//===- UnsafeBufferUsageExtractor.h -----------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +#ifndef LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGEBUILDER_H +#define LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGEBUILDER_H + +#include "clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h" +#include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityId.h" +#include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityName.h" +#include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryBuilder.h" +#include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryExtractor.h" +#include "llvm/Support/Error.h" +#include <memory> + +namespace clang::ssaf { +class UnsafeBufferUsageTUSummaryExtractor : public TUSummaryExtractor { +public: + UnsafeBufferUsageTUSummaryExtractor(TUSummaryBuilder &Builder) + : TUSummaryExtractor(Builder) {} + + static EntityPointerLevel buildEntityPointerLevel(EntityId Entity, + unsigned PointerLevel) { + return {Entity, PointerLevel}; + } + + EntityId addEntity(EntityName EN) { return SummaryBuilder.addEntity(EN); } + + std::unique_ptr<UnsafeBufferUsageEntitySummary> + extractEntitySummary(const Decl *Contributor, ASTContext &Ctx, + llvm::Error &Error); + + void HandleTranslationUnit(ASTContext &Ctx) override; +}; +} // namespace clang::ssaf + +#endif // LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGEBUILDER_H diff --git a/clang/include/clang/ScalableStaticAnalysisFramework/Core/Analyses/UnsafeBufferUsage/UnsafeBufferUsageBuilder.h b/clang/include/clang/ScalableStaticAnalysisFramework/Core/Analyses/UnsafeBufferUsage/UnsafeBufferUsageBuilder.h deleted file mode 100644 index db6c097510a57..0000000000000 --- a/clang/include/clang/ScalableStaticAnalysisFramework/Core/Analyses/UnsafeBufferUsage/UnsafeBufferUsageBuilder.h +++ /dev/null @@ -1,32 +0,0 @@ -//===- UnsafeBufferUsageBuilder.h -------------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_CORE_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGEBUILDER_H -#define LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_CORE_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGEBUILDER_H - -#include "clang/ScalableStaticAnalysisFramework/Core/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h" -#include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryBuilder.h" -#include <memory> - -namespace clang::ssaf { -class UnsafeBufferUsageTUSummaryBuilder : public TUSummaryBuilder { -public: - static EntityPointerLevel buildEntityPointerLevel(EntityId Entity, - unsigned PointerLevel) { - return {Entity, PointerLevel}; - } - - static std::unique_ptr<UnsafeBufferUsageEntitySummary> - buildUnsafeBufferUsageEntitySummary(EntityPointerLevelSet &&UnsafeBuffers) { - return std::make_unique<UnsafeBufferUsageEntitySummary>( - UnsafeBufferUsageEntitySummary(std::move(UnsafeBuffers))); - } -}; -} // namespace clang::ssaf - -#endif // LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_CORE_ANALYSES_UNSAFEBUFFERUSAGE_UNSAFEBUFFERUSAGEBUILDER_H diff --git a/clang/lib/ScalableStaticAnalysisFramework/Analyses/CMakeLists.txt b/clang/lib/ScalableStaticAnalysisFramework/Analyses/CMakeLists.txt new file mode 100644 index 0000000000000..34c6dd9b61203 --- /dev/null +++ b/clang/lib/ScalableStaticAnalysisFramework/Analyses/CMakeLists.txt @@ -0,0 +1,15 @@ +set(LLVM_LINK_COMPONENTS + Support + ) + +add_clang_library(clangScalableStaticAnalysisFrameworkAnalyses + UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp + + LINK_LIBS + clangAST + clangAnalysis + clangBasic + clangScalableStaticAnalysisFrameworkCore + + DEPENDS + ) diff --git a/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp b/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp new file mode 100644 index 0000000000000..c150b2918db59 --- /dev/null +++ b/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp @@ -0,0 +1,281 @@ +//===- UnsafeBufferUsageExtractor.cpp -------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.h" +#include "clang/AST/ASTConsumer.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DynamicRecursiveASTVisitor.h" +#include "clang/AST/StmtVisitor.h" +#include "clang/Analysis/Analyses/UnsafeBufferUsage.h" +#include "clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h" +#include "clang/ScalableStaticAnalysisFramework/Core/ASTEntityMapping.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/iterator_range.h" +#include "llvm/Support/Error.h" +#include <memory> + +namespace { +using namespace clang; +using namespace ssaf; + +static bool hasPointerType(const Expr *E) { + auto Ty = E->getType(); + return !Ty.isNull() && !Ty->isFunctionPointerType() && + (Ty->isPointerType() || Ty->isArrayType()); +} + +constexpr inline auto buildEntityPointerLevel = + UnsafeBufferUsageTUSummaryExtractor::buildEntityPointerLevel; + +// Translate a pointer type expression 'E' to a (set of) EntityPointerLevel(s) +// associated with the declared type of the base address of `E`. If the base +// address of `E` is not associated with an entity, the translation result is an +// empty set. +// +// The translation is a process of traversing into the pointer 'E' until its +// base address can be represented by an entity, with the number of dereferences +// tracked by incrementing the pointer level. Naturally, taking address of, as +// the inverse operation of dereference, is tracked by decrementing the pointer +// level. +// +// For example, suppose there are pointers and arrays declared as +// int *ptr, **p1, **p2; +// int arr[10][10]; +// , the translation of expressions involving these base addresses will be: +// Translate(ptr + 5) -> {(ptr, 1)} +// Translate(arr[5]) -> {(arr, 2)} +// Translate(cond ? p1[5] : p2) -> {(p1, 2), (p2, 1)} +// Translate(&arr[5]) -> {(arr, 1)} +class EntityPointerLevelTranslator + : ConstStmtVisitor<EntityPointerLevelTranslator, + Expected<EntityPointerLevelSet>> { + friend class StmtVisitorBase; + + // Fallback method for all unsupported expression kind: + llvm::Error fallback(const Stmt *E) { + return llvm::createStringError( + "unsupported expression kind for translation to " + "EntityPointerLevel: %s", + E->getStmtClassName()); + } + + static EntityPointerLevel incrementPointerLevel(const EntityPointerLevel &E) { + return buildEntityPointerLevel(E.getEntity(), E.getPointerLevel() + 1); + } + + static EntityPointerLevel decrementPointerLevel(const EntityPointerLevel &E) { + assert(E.getPointerLevel() > 0); + return buildEntityPointerLevel(E.getEntity(), E.getPointerLevel() - 1); + } + + EntityPointerLevel createEntityPointerLevelFor(const EntityName &Name) { + return buildEntityPointerLevel(Extractor.addEntity(Name), 1); + } + + // The common helper function for Translate(*base): + // Translate(*base) -> Translate(base) with .pointerLevel + 1 + Expected<EntityPointerLevelSet> translateDereferencePointer(const Expr *Ptr) { + assert(hasPointerType(Ptr)); + + Expected<EntityPointerLevelSet> SubResult = Visit(Ptr); + if (!SubResult) + return SubResult.takeError(); + + auto Incremented = llvm::map_range(*SubResult, incrementPointerLevel); + return EntityPointerLevelSet{Incremented.begin(), Incremented.end()}; + } + + UnsafeBufferUsageTUSummaryExtractor &Extractor; + +public: + EntityPointerLevelTranslator(UnsafeBufferUsageTUSummaryExtractor &Extractor) + : Extractor(Extractor) {} + + Expected<EntityPointerLevelSet> translate(const Expr *E) { return Visit(E); } + +private: + Expected<EntityPointerLevelSet> VisitStmt(const Stmt *E) { + return fallback(E); + } + + // Translate(base + x) -> Translate(base) + // Translate(x + base) -> Translate(base) + // Translate(base - x) -> Translate(base) + // Translate(base {+=, -=, =} x) -> Translate(base) + // Translate(x, base) -> Translate(base) + Expected<EntityPointerLevelSet> VisitBinaryOperator(const BinaryOperator *E) { + switch (E->getOpcode()) { + case clang::BO_Add: + if (hasPointerType(E->getLHS())) + return Visit(E->getLHS()); + return Visit(E->getRHS()); + case clang::BO_Sub: + case clang::BO_AddAssign: + case clang::BO_SubAssign: + case clang::BO_Assign: + return Visit(E->getLHS()); + case clang::BO_Comma: + return Visit(E->getRHS()); + default: + return fallback(E); + } + } + + // Translate({++, --}base) -> Translate(base) + // Translate(base{++, --}) -> Translate(base) + // Translate(*base) -> Translate(base) with .pointerLevel += 1 + // Translate(&base) -> {}, if Translate(base) is {} + // -> Translate(base) with .pointerLevel -= 1 + Expected<EntityPointerLevelSet> VisitUnaryOperator(const UnaryOperator *E) { + switch (E->getOpcode()) { + case clang::UO_PostInc: + case clang::UO_PostDec: + case clang::UO_PreInc: + case clang::UO_PreDec: + return Visit(E->getSubExpr()); + case clang::UO_AddrOf: { + Expected<EntityPointerLevelSet> SubResult = Visit(E->getSubExpr()); + if (!SubResult) + return SubResult.takeError(); + + auto Decremented = llvm::map_range(*SubResult, decrementPointerLevel); + return EntityPointerLevelSet{Decremented.begin(), Decremented.end()}; + } + case clang::UO_Deref: + return translateDereferencePointer(E->getSubExpr()); + default: + return fallback(E); + } + } + + // Translate((T*)base) -> Translate(p) if p has pointer type + // -> {} otherwise + Expected<EntityPointerLevelSet> VisitCastExpr(const CastExpr *E) { + if (hasPointerType(E->getSubExpr())) + return Visit(E->getSubExpr()); + return EntityPointerLevelSet{}; + } + + // Translate(f(...)) -> {} if it is an indirect call + // -> {(f_return, 1)}, otherwise + Expected<EntityPointerLevelSet> VisitCallExpr(const CallExpr *E) { + if (auto *FD = E->getDirectCallee()) + if (auto FDEntityName = getEntityNameForReturn(FD)) + return EntityPointerLevelSet{ + createEntityPointerLevelFor(*FDEntityName)}; + return EntityPointerLevelSet{}; + } + + // Translate(base[x]) -> Translate(*base) + Expected<EntityPointerLevelSet> + VisitArraySubscriptExpr(const ArraySubscriptExpr *E) { + return translateDereferencePointer(E->getBase()); + } + + // Translate(cond ? base1 : base2) := Translate(base1) U Translate(base2) + Expected<EntityPointerLevelSet> + VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) { + Expected<EntityPointerLevelSet> ReT = Visit(E->getTrueExpr()); + Expected<EntityPointerLevelSet> ReF = Visit(E->getFalseExpr()); + + if (ReT && ReF) { + ReT->insert(ReF->begin(), ReF->end()); + return ReT; + } + if (!ReF && !ReT) + return llvm::joinErrors(ReT.takeError(), ReF.takeError()); + if (!ReF) + return ReF.takeError(); + return ReT.takeError(); + } + + Expected<EntityPointerLevelSet> VisitParenExpr(const ParenExpr *E) { + return Visit(E->getSubExpr()); + } + + // Translate("string-literal") -> {} + // Buffer accesses on string literals are unsafe, but string literals are not + // entities so there is no EntityPointerLevel associated with it. + Expected<EntityPointerLevelSet> VisitStringLiteral(const StringLiteral *E) { + return EntityPointerLevelSet{}; + } + + // Translate(DRE) -> {(Decl, 1)} + Expected<EntityPointerLevelSet> VisitDeclRefExpr(const DeclRefExpr *E) { + if (auto EntityName = getEntityName(E->getDecl())) + return EntityPointerLevelSet{createEntityPointerLevelFor(*EntityName)}; + return llvm::createStringError( + "failed to create entity name from the Decl of " + + E->getNameInfo().getAsString()); + } + + // Translate({., ->}f) -> {(MemberDecl, 1)} + Expected<EntityPointerLevelSet> VisitMemberExpr(const MemberExpr *E) { + if (auto EntityName = getEntityName(E->getMemberDecl())) + return EntityPointerLevelSet{createEntityPointerLevelFor(*EntityName)}; + return llvm::createStringError( + "failed to create entity name from the MemberDecl of " + + E->getMemberDecl()->getNameAsString()); + } + + Expected<EntityPointerLevelSet> + VisitOpaqueValueExpr(const OpaqueValueExpr *S) { + return Visit(S->getSourceExpr()); + } +}; + +Expected<EntityPointerLevelSet> +buildEntityPointerLevels(std::set<const Expr *> &&UnsafePointers, + UnsafeBufferUsageTUSummaryExtractor &Extractor) { + EntityPointerLevelSet Result{}; + EntityPointerLevelTranslator Translator{Extractor}; + llvm::Error AllErrors = llvm::ErrorSuccess(); + + for (const Expr *Ptr : UnsafePointers) { + Expected<EntityPointerLevelSet> Translation = Translator.translate(Ptr); + + if (Translation) { + // Filter out those temporary invalid EntityPointerLevels associated with + // `&E` pointers: + auto FilteredTranslation = llvm::make_filter_range( + *Translation, [](const EntityPointerLevel &E) -> bool { + return E.getPointerLevel() > 0; + }); + Result.insert(FilteredTranslation.begin(), FilteredTranslation.end()); + continue; + } + AllErrors = llvm::joinErrors(std::move(AllErrors), Translation.takeError()); + } + if (AllErrors) + return AllErrors; + return Result; +} +} // namespace + +std::unique_ptr<UnsafeBufferUsageEntitySummary> +UnsafeBufferUsageTUSummaryExtractor::extractEntitySummary( + const Decl *Contributor, ASTContext &Ctx, llvm::Error &Error) { + // FIXME: findUnsafePointers should accept more kinds of `Decl`s than just + // `FunctionDecl`: + if (const auto *FD = dyn_cast<FunctionDecl>(Contributor)) { + Expected<EntityPointerLevelSet> EPLs = + buildEntityPointerLevels(findUnsafePointers(FD), *this); + + if (EPLs) + return std::make_unique<UnsafeBufferUsageEntitySummary>( + UnsafeBufferUsageEntitySummary(std::move(*EPLs))); + Error = EPLs.takeError(); + } + return nullptr; +} + +void UnsafeBufferUsageTUSummaryExtractor::HandleTranslationUnit( + ASTContext &Ctx) { + // FIXME: impl me! +} diff --git a/clang/lib/ScalableStaticAnalysisFramework/CMakeLists.txt b/clang/lib/ScalableStaticAnalysisFramework/CMakeLists.txt index d3d75430233fe..6bcb1a0038127 100644 --- a/clang/lib/ScalableStaticAnalysisFramework/CMakeLists.txt +++ b/clang/lib/ScalableStaticAnalysisFramework/CMakeLists.txt @@ -1,2 +1,4 @@ +add_subdirectory(Analyses) add_subdirectory(Core) add_subdirectory(Frontend) + diff --git a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp index 082ff7f8ac175..81c742dc30811 100644 --- a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp +++ b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp @@ -6,37 +6,144 @@ // //===----------------------------------------------------------------------===// -#include "clang/ScalableStaticAnalysisFramework/Core/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h" -#include "clang/ScalableStaticAnalysisFramework/Core/Analyses/UnsafeBufferUsage/UnsafeBufferUsageBuilder.h" +#include "clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h" +#include "clang/AST/DynamicRecursiveASTVisitor.h" +#include "clang/Frontend/ASTUnit.h" +#include "clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.h" +#include "clang/ScalableStaticAnalysisFramework/Core/ASTEntityMapping.h" #include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityId.h" -#include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityIdTable.h" +#include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityName.h" +#include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummary.h" +#include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryBuilder.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" using namespace clang; using namespace ssaf; +using testing::UnorderedElementsAre; namespace { +template <typename SomeDecl = NamedDecl> +const SomeDecl *findDeclByName(StringRef Name, ASTContext &Ctx) { + class NamedDeclFinder : public DynamicRecursiveASTVisitor { + public: + StringRef SearchingName; + const NamedDecl *FoundDecl = nullptr; + + NamedDeclFinder(StringRef SearchingName) : SearchingName(SearchingName) {} + + bool VisitDecl(Decl *D) override { + if (const auto *ND = dyn_cast<NamedDecl>(D)) { + if (ND->getNameAsString() == SearchingName) { + FoundDecl = ND; + return false; + } + } + return true; + } + }; + + NamedDeclFinder Finder(Name); + + Finder.TraverseDecl(Ctx.getTranslationUnitDecl()); + return dyn_cast_or_null<SomeDecl>(Finder.FoundDecl); +} + +const FunctionDecl *findFnByName(StringRef Name, ASTContext &Ctx) { + return findDeclByName<FunctionDecl>(Name, Ctx); +} + constexpr inline auto buildEntityPointerLevel = - UnsafeBufferUsageTUSummaryBuilder::buildEntityPointerLevel; -constexpr inline auto buildUnsafeBufferUsageEntitySummary = - UnsafeBufferUsageTUSummaryBuilder::buildUnsafeBufferUsageEntitySummary; + UnsafeBufferUsageTUSummaryExtractor::buildEntityPointerLevel; class UnsafeBufferUsageTest : public testing::Test { protected: - EntityIdTable Table; + TUSummary TUSum; + TUSummaryBuilder TUSummaryBuilder; + UnsafeBufferUsageTUSummaryExtractor Extractor; + std::unique_ptr<ASTUnit> AST; + + UnsafeBufferUsageTest() + : TUSum(BuildNamespace(BuildNamespaceKind::CompilationUnit, "Mock.cpp")), + TUSummaryBuilder(TUSum), Extractor(TUSummaryBuilder) {} + + std::unique_ptr<UnsafeBufferUsageEntitySummary> + setUpTest(StringRef Code, StringRef ContributorName) { + AST = tooling::buildASTFromCodeWithArgs( + Code, {"-Wno-unused-value -Wno-int-to-pointer-cast"}); + + const auto *ContributorDefn = + findDeclByName(ContributorName, AST->getASTContext()); + std::optional<EntityName> EN = getEntityName(ContributorDefn); + + if (!ContributorDefn || !EN) + return nullptr; + + llvm::Error Error = llvm::ErrorSuccess(); + auto Sum = Extractor.extractEntitySummary(ContributorDefn, + AST->getASTContext(), Error); + + if (Error) { + llvm::consumeError(std::move(Error)); + return nullptr; + } + return Sum; + } + + std::optional<EntityId> getEntityId(StringRef Name) { + if (const auto *D = findDeclByName(Name, AST->getASTContext())) + if (auto EntityName = getEntityName(D)) + return Extractor.addEntity(*EntityName); + return std::nullopt; + } + + std::optional<EntityId> getEntityIdForReturn(StringRef FunName) { + if (const auto *D = findFnByName(FunName, AST->getASTContext())) + if (auto EntityName = getEntityNameForReturn(D)) + return Extractor.addEntity(*EntityName); + return std::nullopt; + } + + // Same as `std::pair<StringName, unsigned>` for a pair of entity declaration + // name and a pointer level with an extra optional flag for whether the entity + // represents a function return value: + struct EPLPair { + EPLPair(StringRef Name, unsigned Lv, bool isFunRet = false) + : Name(Name), Lv(Lv), isFunRet(isFunRet) {} + + StringRef Name; + unsigned Lv; + bool isFunRet; + }; + + EntityPointerLevelSet makeSet(unsigned Line, ArrayRef<EPLPair> Pairs) { + auto EPLs = llvm::map_range( + Pairs, [this, Line](const EPLPair &Pair) -> EntityPointerLevel { + std::optional<EntityId> Entity = Pair.isFunRet + ? getEntityIdForReturn(Pair.Name) + : getEntityId(Pair.Name); + if (!Entity) + ADD_FAILURE_AT(__FILE__, Line) << "Entity not found: " << Pair.Name; + return buildEntityPointerLevel(*Entity, Pair.Lv); + }); + return EntityPointerLevelSet{EPLs.begin(), EPLs.end()}; + } }; ////////////////////////////////////////////////////////////// // Data Structure Tests // ////////////////////////////////////////////////////////////// -#define EXPECT_CONTAINS(Set, Elt) EXPECT_NE((Set).find(Elt), (Set).end()) -#define EXPECT_EXCLUDES(Set, Elt) EXPECT_EQ((Set).find(Elt), (Set).end()) +static llvm::iterator_range<EntityPointerLevelSet::iterator> +getSubsetOf(const EntityPointerLevelSet &Set, EntityId Entity) { + return llvm::make_range(Set.equal_range(Entity)); +} TEST_F(UnsafeBufferUsageTest, EntityPointerLevelComparison) { - EntityId E1 = Table.getId({"c:@F@foo", "", {}}); - EntityId E2 = Table.getId({"c:@F@bar", "", {}}); + EntityId E1 = Extractor.addEntity({"c:@F@foo", "", {}}); + EntityId E2 = Extractor.addEntity({"c:@F@bar", "", {}}); auto P1 = buildEntityPointerLevel(E1, 2); auto P2 = buildEntityPointerLevel(E1, 2); @@ -53,48 +160,286 @@ TEST_F(UnsafeBufferUsageTest, EntityPointerLevelComparison) { EXPECT_FALSE(P2 < P1); } -TEST_F(UnsafeBufferUsageTest, UnsafeBufferUsageEntitySummaryTest) { - EntityId E1 = Table.getId({"c:@F@foo", "", {}}); - EntityId E2 = Table.getId({"c:@F@bar", "", {}}); - EntityId E3 = Table.getId({"c:@F@baz", "", {}}); +TEST_F(UnsafeBufferUsageTest, UnsafeBufferUsageEntityPointerLevelSetTest) { + EntityId E1 = Extractor.addEntity({"c:@F@foo", "", {}}); + EntityId E2 = Extractor.addEntity({"c:@F@bar", "", {}}); + EntityId E3 = Extractor.addEntity({"c:@F@baz", "", {}}); auto P1 = buildEntityPointerLevel(E1, 1); auto P2 = buildEntityPointerLevel(E1, 2); auto P3 = buildEntityPointerLevel(E2, 1); auto P4 = buildEntityPointerLevel(E2, 2); auto P5 = buildEntityPointerLevel(E3, 1); - auto P6 = buildEntityPointerLevel(E3, 2); EntityPointerLevelSet Set{P1, P2, P3, P4, P5}; - auto ES = buildUnsafeBufferUsageEntitySummary(std::move(Set)); - ASSERT_TRUE(ES); - EXPECT_CONTAINS(*ES, P1); - EXPECT_CONTAINS(*ES, P2); - EXPECT_CONTAINS(*ES, P3); - EXPECT_CONTAINS(*ES, P4); - EXPECT_CONTAINS(*ES, P5); - EXPECT_EXCLUDES(*ES, P6); + EXPECT_THAT(Set, UnorderedElementsAre(P1, P2, P3, P4, P5)); + EXPECT_THAT(getSubsetOf(Set, E1), UnorderedElementsAre(P1, P2)); + EXPECT_THAT(getSubsetOf(Set, E2), UnorderedElementsAre(P3, P4)); + EXPECT_THAT(getSubsetOf(Set, E3), UnorderedElementsAre(P5)); +} + +////////////////////////////////////////////////////////////// +// Extractor Tests // +////////////////////////////////////////////////////////////// + +TEST_F(UnsafeBufferUsageTest, SimpleFunctionWithUnsafePointer) { + auto Sum = setUpTest(R"cpp( + void foo(int *p) { + p[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}})); +} - EntityPointerLevelSet Subset1{ES->getSubsetOf(E1).begin(), - ES->getSubsetOf(E1).end()}; +TEST_F(UnsafeBufferUsageTest, PointerArithmetic) { + auto Sum = setUpTest(R"cpp( + void foo(int *p, int *q) { + *(p + 5); + *(q - 3); + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}, {"q", 1U}})); +} + +TEST_F(UnsafeBufferUsageTest, PointerIncrementDecrement) { + auto Sum = setUpTest(R"cpp( + void foo(int *p, int *q, int *r, int *s) { + (++p)[5]; + (q++)[5]; + (--r)[5]; + (s--)[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, + makeSet(__LINE__, {{"p", 1U}, {"q", 1U}, {"r", 1U}, {"s", 1U}})); +} + +TEST_F(UnsafeBufferUsageTest, PointerAssignment) { + auto Sum = setUpTest(R"cpp( + void foo(int *p, int *q) { + (p = q + 5)[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}, {"q", 1U}})); +} + +TEST_F(UnsafeBufferUsageTest, CompoundAssignment) { + auto Sum = setUpTest(R"cpp( + void foo(int *p, int *q) { + (p += 5)[5]; + (q -= 3)[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}, {"q", 1U}})); +} - EXPECT_CONTAINS(Subset1, P1); - EXPECT_CONTAINS(Subset1, P2); - EXPECT_EQ(Subset1.size(), 2U); +TEST_F(UnsafeBufferUsageTest, MultiLevelPointer) { + auto Sum = setUpTest(R"cpp( + void foo(int **p, int **q, int **r) { + (*p)[5]; + *(*q); + *(q[5]); + r[5][5]; + } + )cpp", + "foo"); - EntityPointerLevelSet Subset2{ES->getSubsetOf(E2).begin(), - ES->getSubsetOf(E2).end()}; + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, + makeSet(__LINE__, {{"p", 2U}, {"q", 1U}, {"r", 1U}, {"r", 2U}})); +} + +TEST_F(UnsafeBufferUsageTest, ConditionalOperator) { + auto Sum = setUpTest(R"cpp( + void foo(int **p, int **q, int cond) { + (cond ? *p : *q)[5]; + cond ? p[5] : q[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, + makeSet(__LINE__, {{"p", 1U}, {"q", 1U}, {"p", 2U}, {"q", 2U}})); +} + +TEST_F(UnsafeBufferUsageTest, CastExpression) { + auto Sum = setUpTest(R"cpp( + void foo(void *p, int q) { + ((int*)p)[5]; + ((int*)q)[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}})); +} + +TEST_F(UnsafeBufferUsageTest, CommaOperator) { + auto Sum = setUpTest(R"cpp( + void foo(int *p, int x) { + (x++, p)[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}})); +} + +TEST_F(UnsafeBufferUsageTest, CommaOperator2) { + auto Sum = setUpTest(R"cpp( + void foo(int **p, int **q, int x) { + (p[x] = 0, q[x] = 0)[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}, {"q", 1U}, {"q", 2U}})); +} + +TEST_F(UnsafeBufferUsageTest, ParenthesizedExpression) { + auto Sum = setUpTest(R"cpp( + void foo(int *p) { + (((p)))[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}})); +} + +TEST_F(UnsafeBufferUsageTest, ArrayParameter) { + auto Sum = setUpTest(R"cpp( + void foo(int arr[], int arr2[][10]) { + int n = 5; + arr[100]; + arr2[5][n]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"arr", 1U}, {"arr2", 1U}, {"arr2", 2U}})); +} + +TEST_F(UnsafeBufferUsageTest, FunctionCall) { + auto Sum = setUpTest(R"cpp( + int ** (*fp)(); + int ** foo() { + fp = &foo; + foo()[5]; + (*fp())[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + // No (foo, 2) becasue indirect calls are ignored. + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"foo", 1U, true}})); +} + +TEST_F(UnsafeBufferUsageTest, StructMemberAccess) { + auto Sum = setUpTest(R"cpp( + struct S { + int *ptr; + int (*ptr_to_arr)[10]; + }; + void foo(struct S obj) { + int n = 5; + obj.ptr[5]; + (*obj.ptr_to_arr)[n]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"ptr", 1U}, {"ptr_to_arr", 2U}})); +} + +TEST_F(UnsafeBufferUsageTest, StringLiteralSubscript) { + auto Sum = setUpTest(R"cpp( + void foo() { + "hello"[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + // String literals should not generate pointer kind variables + EXPECT_EQ(*Sum, makeSet(__LINE__, {})); +} + +TEST_F(UnsafeBufferUsageTest, OpaqueValueExpr) { + auto Sum = setUpTest(R"cpp( + void foo(int *p, int *q) { + (p ?: q)[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}, {"q", 1U}})); +} + +TEST_F(UnsafeBufferUsageTest, AddressOfOperator) { + auto Sum = setUpTest(R"cpp( + void foo(int x) { + (&x)[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + // Address-of should not generate pointer kind variables for 'x' + EXPECT_EQ(*Sum, makeSet(__LINE__, {})); +} + +TEST_F(UnsafeBufferUsageTest, AddressOfThenDereference) { + auto Sum = setUpTest(R"cpp( + void foo(int *p, int *q) { + (*(&p))[5]; + (&(*q))[5]; + } + )cpp", + "foo"); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1}, {"q", 1}})); +} - EXPECT_CONTAINS(Subset2, P3); - EXPECT_CONTAINS(Subset2, P4); - EXPECT_EQ(Subset2.size(), 2U); +TEST_F(UnsafeBufferUsageTest, PointerToArrayOfPointers) { + auto Sum = setUpTest(R"cpp( + void foo() { + int * arr[10]; + int * (*p)[10] = arr; - EntityPointerLevelSet Subset3{ES->getSubsetOf(E3).begin(), - ES->getSubsetOf(E3).end()}; + (*p)[5][5]; // '(*p)[5]' is unsafe + // '(*p)' is fine because 5 < 10 + } + )cpp", + "foo"); - EXPECT_CONTAINS(Subset3, P5); - EXPECT_EXCLUDES(Subset3, P6); - EXPECT_EQ(Subset3.size(), 1U); + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 3}})); } } // namespace diff --git a/clang/unittests/ScalableStaticAnalysisFramework/CMakeLists.txt b/clang/unittests/ScalableStaticAnalysisFramework/CMakeLists.txt index 871d9e6b0c02c..f541e81d42789 100644 --- a/clang/unittests/ScalableStaticAnalysisFramework/CMakeLists.txt +++ b/clang/unittests/ScalableStaticAnalysisFramework/CMakeLists.txt @@ -29,6 +29,7 @@ add_distinct_clang_unittest(ClangScalableAnalysisTests clangASTMatchers clangBasic clangFrontend + clangScalableStaticAnalysisFrameworkAnalyses clangScalableStaticAnalysisFrameworkCore clangScalableStaticAnalysisFrameworkFrontend clangSerialization >From 09fcf76d7f0e854bcb2fc3521e4692d0f1974efd Mon Sep 17 00:00:00 2001 From: Ziqing Luo <[email protected]> Date: Thu, 5 Mar 2026 13:00:44 -0800 Subject: [PATCH 2/4] [ssaf][UnsafeBufferUsage] Add support for extracting unsafe pointers from all kinds of contributors - Generalize the -Wunsafe-buffer-usage API for finding unsafe pointers in all kinds of Decls - Add support in SSAF-based UnsafeBufferUsage analysis for extracting from various contributors - Mock implementation of HandleTranslationUnit rdar://171735836 --- .../Analysis/Analyses/UnsafeBufferUsage.h | 9 +- .../UnsafeBufferUsage/UnsafeBufferUsage.h | 2 + clang/lib/Analysis/UnsafeBufferUsage.cpp | 61 +++++--- .../UnsafeBufferUsageExtractor.cpp | 140 ++++++++++++++---- .../UnsafeBufferUsageTest.cpp | 62 +++++++- 5 files changed, 226 insertions(+), 48 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h index 876682ad779d4..e0d583c735e61 100644 --- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h +++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h @@ -201,7 +201,14 @@ bool anyConflict(const llvm::SmallVectorImpl<FixItHint> &FixIts, const SourceManager &SM); } // namespace internal -std::set<const Expr *> findUnsafePointers(const FunctionDecl *FD); +/// Find unsafe pointers in body/initializer of `D`, if `D` is one of the +/// followings: +/// VarDecl +/// FieldDecl +/// FunctionDecl +/// BlockDecl +/// ObjCMethodDecl +std::set<const Expr *> findUnsafePointers(const Decl *D); } // end namespace clang #endif /* LLVM_CLANG_ANALYSIS_ANALYSES_UNSAFEBUFFERUSAGE_H */ diff --git a/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h b/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h index 5f418000723b4..c0a4c2f76ab48 100644 --- a/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h +++ b/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h @@ -98,6 +98,8 @@ class UnsafeBufferUsageEntitySummary final : public EntitySummary { bool operator==(const EntityPointerLevelSet &Other) const { return UnsafeBuffers == Other; } + + bool empty() const { return UnsafeBuffers.empty(); } }; } // namespace clang::ssaf diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 133e39b8fac2b..d6f554246a1b4 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -28,6 +28,7 @@ #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/APInt.h" #include "llvm/ADT/APSInt.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" @@ -36,6 +37,7 @@ #include <queue> #include <set> #include <sstream> +#include <vector> using namespace clang; @@ -2942,7 +2944,37 @@ template <typename NodeTy> struct CompareNode { } }; -std::set<const Expr *> clang::findUnsafePointers(const FunctionDecl *FD) { +// Populate `Stmts` with the body/initializer Stmt of `D`, if `D` is one of the +// followings: +// VarDecl +// FieldDecl +// FunctionDecl +// BlockDecl +// ObjCMethodDecl +static void populateStmtsForFindingGadgets(SmallVector<const Stmt *> &Stmts, + const Decl *D) { + auto AddStmt = [&Stmts](const Stmt *S) { + if (S) + Stmts.push_back(S); + }; + if (const auto *FD = dyn_cast<FunctionDecl>(D)) { + AddStmt(FD->getBody()); + for (const auto *PD : FD->parameters()) + AddStmt(PD->getDefaultArg()); + if (const auto *CtorD = dyn_cast<CXXConstructorDecl>(FD)) + llvm::append_range( + Stmts, llvm::map_range(CtorD->inits(), + std::mem_fn(&CXXCtorInitializer::getInit))); + } else if (isa<BlockDecl>(D) || isa<ObjCMethodDecl>(D)) { + AddStmt(D->getBody()); + } else if (const auto *VD = dyn_cast<VarDecl>(D)) { + AddStmt(VD->getInit()); // FIXME: default arg for ParmVarDecl? + } else if (const auto *FD = dyn_cast<FieldDecl>(D)) { + AddStmt(FD->getInClassInitializer()); + } +} + +std::set<const Expr *> clang::findUnsafePointers(const Decl *D) { class MockReporter : public UnsafeBufferUsageHandler { public: MockReporter() {} @@ -2981,9 +3013,13 @@ std::set<const Expr *> clang::findUnsafePointers(const FunctionDecl *FD) { WarningGadgetList WarningGadgets; DeclUseTracker Tracker; MockReporter IgnoreHandler; + ASTContext &Ctx = D->getASTContext(); + SmallVector<const Stmt *> Stmts; - findGadgets(FD->getBody(), FD->getASTContext(), IgnoreHandler, false, - FixableGadgets, WarningGadgets, Tracker); + populateStmtsForFindingGadgets(Stmts, D); + for (auto *Stmt : Stmts) + findGadgets(Stmt, Ctx, IgnoreHandler, false, FixableGadgets, WarningGadgets, + Tracker); std::set<const Expr *> Result; for (auto &G : WarningGadgets) { @@ -4673,9 +4709,6 @@ void clang::checkUnsafeBufferUsage(const Decl *D, #endif assert(D); - - SmallVector<Stmt *> Stmts; - if (const auto *FD = dyn_cast<FunctionDecl>(D)) { // Consteval functions are free of UB by the spec, so we don't need to // visit them or produce diagnostics. @@ -4697,27 +4730,21 @@ void clang::checkUnsafeBufferUsage(const Decl *D, break; } } + } - Stmts.push_back(FD->getBody()); + SmallVector<const Stmt *> Stmts; - if (const auto *ID = dyn_cast<CXXConstructorDecl>(D)) { - for (const CXXCtorInitializer *CI : ID->inits()) { - Stmts.push_back(CI->getInit()); - } - } - } else if (isa<BlockDecl>(D) || isa<ObjCMethodDecl>(D)) { - Stmts.push_back(D->getBody()); - } + populateStmtsForFindingGadgets(Stmts, D); assert(!Stmts.empty()); FixableGadgetList FixableGadgets; WarningGadgetList WarningGadgets; DeclUseTracker Tracker; - for (Stmt *S : Stmts) { + for (const Stmt *S : Stmts) { findGadgets(S, D->getASTContext(), Handler, EmitSuggestions, FixableGadgets, WarningGadgets, Tracker); } applyGadgets(D, std::move(FixableGadgets), std::move(WarningGadgets), std::move(Tracker), Handler, EmitSuggestions); -} +} \ No newline at end of file diff --git a/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp b/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp index c150b2918db59..c609168e4dc7d 100644 --- a/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp +++ b/clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp @@ -33,6 +33,32 @@ static bool hasPointerType(const Expr *E) { constexpr inline auto buildEntityPointerLevel = UnsafeBufferUsageTUSummaryExtractor::buildEntityPointerLevel; +static llvm::Error makeUnsupportedStmtKindError(const Stmt *Unsupported) { + return llvm::createStringError( + "unsupported expression kind for translation to " + "EntityPointerLevel: %s", + Unsupported->getStmtClassName()); +} + +static llvm::Error makeCreateEntityNameError(const NamedDecl *FailedDecl, + ASTContext &Ctx) { + std::string LocStr = FailedDecl->getSourceRange().getBegin().printToString( + Ctx.getSourceManager()); + return llvm::createStringError( + "failed to create entity name for %s declared at %s", + FailedDecl->getNameAsString().c_str(), LocStr.c_str()); +} + +static llvm::Error makeAddEntitySummaryError(const NamedDecl *FailedContributor, + ASTContext &Ctx) { + std::string LocStr = + FailedContributor->getSourceRange().getBegin().printToString( + Ctx.getSourceManager()); + return llvm::createStringError( + "failed to add entity summary for contributor %s declared at %s", + FailedContributor->getNameAsString().c_str(), LocStr.c_str()); +} + // Translate a pointer type expression 'E' to a (set of) EntityPointerLevel(s) // associated with the declared type of the base address of `E`. If the base // address of `E` is not associated with an entity, the translation result is an @@ -59,10 +85,7 @@ class EntityPointerLevelTranslator // Fallback method for all unsupported expression kind: llvm::Error fallback(const Stmt *E) { - return llvm::createStringError( - "unsupported expression kind for translation to " - "EntityPointerLevel: %s", - E->getStmtClassName()); + return makeUnsupportedStmtKindError(E); } static EntityPointerLevel incrementPointerLevel(const EntityPointerLevel &E) { @@ -92,10 +115,12 @@ class EntityPointerLevelTranslator } UnsafeBufferUsageTUSummaryExtractor &Extractor; + ASTContext &Ctx; public: - EntityPointerLevelTranslator(UnsafeBufferUsageTUSummaryExtractor &Extractor) - : Extractor(Extractor) {} + EntityPointerLevelTranslator(UnsafeBufferUsageTUSummaryExtractor &Extractor, + ASTContext &Ctx) + : Extractor(Extractor), Ctx(Ctx) {} Expected<EntityPointerLevelSet> translate(const Expr *E) { return Visit(E); } @@ -210,18 +235,14 @@ class EntityPointerLevelTranslator Expected<EntityPointerLevelSet> VisitDeclRefExpr(const DeclRefExpr *E) { if (auto EntityName = getEntityName(E->getDecl())) return EntityPointerLevelSet{createEntityPointerLevelFor(*EntityName)}; - return llvm::createStringError( - "failed to create entity name from the Decl of " + - E->getNameInfo().getAsString()); + return makeCreateEntityNameError(E->getDecl(), Ctx); } // Translate({., ->}f) -> {(MemberDecl, 1)} Expected<EntityPointerLevelSet> VisitMemberExpr(const MemberExpr *E) { if (auto EntityName = getEntityName(E->getMemberDecl())) return EntityPointerLevelSet{createEntityPointerLevelFor(*EntityName)}; - return llvm::createStringError( - "failed to create entity name from the MemberDecl of " + - E->getMemberDecl()->getNameAsString()); + return makeCreateEntityNameError(E->getMemberDecl(), Ctx); } Expected<EntityPointerLevelSet> @@ -232,9 +253,10 @@ class EntityPointerLevelTranslator Expected<EntityPointerLevelSet> buildEntityPointerLevels(std::set<const Expr *> &&UnsafePointers, - UnsafeBufferUsageTUSummaryExtractor &Extractor) { + UnsafeBufferUsageTUSummaryExtractor &Extractor, + ASTContext &Ctx) { EntityPointerLevelSet Result{}; - EntityPointerLevelTranslator Translator{Extractor}; + EntityPointerLevelTranslator Translator{Extractor, Ctx}; llvm::Error AllErrors = llvm::ErrorSuccess(); for (const Expr *Ptr : UnsafePointers) { @@ -258,24 +280,90 @@ buildEntityPointerLevels(std::set<const Expr *> &&UnsafePointers, } } // namespace +static std::set<const Expr *> findUnsafePointersInContributor(const Decl *D) { + if (isa<FunctionDecl>(D) || isa<VarDecl>(D)) + return findUnsafePointers(D); + if (auto *RD = dyn_cast<RecordDecl>(D)) { + std::set<const Expr *> Result; + + for (const FieldDecl *FD : RD->fields()) { + Result.merge(findUnsafePointers(FD)); + } + return Result; + } + return {}; +} + std::unique_ptr<UnsafeBufferUsageEntitySummary> UnsafeBufferUsageTUSummaryExtractor::extractEntitySummary( const Decl *Contributor, ASTContext &Ctx, llvm::Error &Error) { - // FIXME: findUnsafePointers should accept more kinds of `Decl`s than just - // `FunctionDecl`: - if (const auto *FD = dyn_cast<FunctionDecl>(Contributor)) { - Expected<EntityPointerLevelSet> EPLs = - buildEntityPointerLevels(findUnsafePointers(FD), *this); - - if (EPLs) - return std::make_unique<UnsafeBufferUsageEntitySummary>( - UnsafeBufferUsageEntitySummary(std::move(*EPLs))); - Error = EPLs.takeError(); - } + Expected<EntityPointerLevelSet> EPLs = buildEntityPointerLevels( + findUnsafePointersInContributor(Contributor), *this, Ctx); + + if (EPLs) + return std::make_unique<UnsafeBufferUsageEntitySummary>( + UnsafeBufferUsageEntitySummary(std::move(*EPLs))); + Error = EPLs.takeError(); return nullptr; } void UnsafeBufferUsageTUSummaryExtractor::HandleTranslationUnit( ASTContext &Ctx) { - // FIXME: impl me! + + // FIXME: I suppose finding contributor Decls is commonly needed by all/many + // extractors + class ContributorFinder : public DynamicRecursiveASTVisitor { + public: + std::vector<const NamedDecl *> Contributors; + + bool VisitFunctionDecl(FunctionDecl *D) override { + Contributors.push_back(D); + return true; + } + + bool VisitRecordDecl(RecordDecl *D) override { + Contributors.push_back(D); + return true; + } + + bool VisitVarDecl(VarDecl *D) override { + DeclContext *DC = D->getDeclContext(); + + if (DC->isFileContext() || DC->isNamespace()) + Contributors.push_back(D); + return false; + } + } ContributorFinder; + + ContributorFinder.VisitTranslationUnitDecl(Ctx.getTranslationUnitDecl()); + + llvm::Error Errors = llvm::ErrorSuccess(); + auto addError = [&Errors](llvm::Error Err) { + Errors = llvm::joinErrors(std::move(Errors), std::move(Err)); + }; + + for (auto *CD : ContributorFinder.Contributors) { + llvm::Error Error = llvm::ErrorSuccess(); + auto EntitySummary = extractEntitySummary(CD, Ctx, Error); + + if (Error) + addError(std::move(Error)); + if (EntitySummary->empty()) + continue; + + auto ContributorName = getEntityName(CD); + + if (!ContributorName) { + addError(makeCreateEntityNameError(CD, Ctx)); + continue; + } + + auto [EntitySummaryPtr, Success] = SummaryBuilder.addSummary( + addEntity(*ContributorName), std::move(EntitySummary)); + + if (!Success) + addError(makeAddEntitySummaryError(CD, Ctx)); + } + // FIXME: handle errors! + llvm::consumeError(std::move(Errors)); } diff --git a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp index 81c742dc30811..67eec3714047b 100644 --- a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp +++ b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp @@ -35,7 +35,7 @@ const SomeDecl *findDeclByName(StringRef Name, ASTContext &Ctx) { NamedDeclFinder(StringRef SearchingName) : SearchingName(SearchingName) {} bool VisitDecl(Decl *D) override { - if (const auto *ND = dyn_cast<NamedDecl>(D)) { + if (const auto *ND = dyn_cast<SomeDecl>(D)) { if (ND->getNameAsString() == SearchingName) { FoundDecl = ND; return false; @@ -69,16 +69,21 @@ class UnsafeBufferUsageTest : public testing::Test { : TUSum(BuildNamespace(BuildNamespaceKind::CompilationUnit, "Mock.cpp")), TUSummaryBuilder(TUSum), Extractor(TUSummaryBuilder) {} + template <typename ContributorDecl = NamedDecl> std::unique_ptr<UnsafeBufferUsageEntitySummary> setUpTest(StringRef Code, StringRef ContributorName) { AST = tooling::buildASTFromCodeWithArgs( - Code, {"-Wno-unused-value -Wno-int-to-pointer-cast"}); + Code, {"-Wno-unused-value", "-Wno-int-to-pointer-cast"}); const auto *ContributorDefn = - findDeclByName(ContributorName, AST->getASTContext()); + findDeclByName<ContributorDecl>(ContributorName, AST->getASTContext()); + + if (!ContributorDefn) + return nullptr; + std::optional<EntityName> EN = getEntityName(ContributorDefn); - if (!ContributorDefn || !EN) + if (!EN) return nullptr; llvm::Error Error = llvm::ErrorSuccess(); @@ -442,4 +447,53 @@ TEST_F(UnsafeBufferUsageTest, PointerToArrayOfPointers) { EXPECT_NE(Sum, nullptr); EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 3}})); } + +TEST_F(UnsafeBufferUsageTest, UnsafePointerInGlobalVariableInitializer) { + auto Sum = setUpTest(R"cpp( + int *gp; + int x = gp[5]; + )cpp", + {"x"}); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"gp", 1U}})); +} + +TEST_F(UnsafeBufferUsageTest, UnsafePointerInFieldInitializer) { + auto Sum = setUpTest<>(R"cpp( + int *gp; + struct Foo { + int field = gp[5]; + }; + )cpp", + {"Foo"}); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"gp", 1U}})); +} + +TEST_F(UnsafeBufferUsageTest, UnsafePointerInCXXCtorInitializer) { + auto Sum = setUpTest<CXXConstructorDecl>(R"cpp( + struct Foo { + int member; + Foo(int *p) : member(p[5]) {} + }; + )cpp", + {"Foo"}); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}})); +} + +TEST_F(UnsafeBufferUsageTest, UnsafePointerInDefaultArg) { + auto Sum = setUpTest(R"cpp( + int * gp; + void foo(int x = gp[5]); + )cpp", + {"foo"}); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"gp", 1U}})); +} + } // namespace >From 613955c1b475383116fc8182aa12a21d87d63ddd Mon Sep 17 00:00:00 2001 From: Ziqing Luo <[email protected]> Date: Thu, 5 Mar 2026 14:09:34 -0800 Subject: [PATCH 3/4] clean up --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 2 +- .../Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index d6f554246a1b4..5a9241acbee36 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -4747,4 +4747,4 @@ void clang::checkUnsafeBufferUsage(const Decl *D, } applyGadgets(D, std::move(FixableGadgets), std::move(WarningGadgets), std::move(Tracker), Handler, EmitSuggestions); -} \ No newline at end of file +} diff --git a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp index 67eec3714047b..db11717a86747 100644 --- a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp +++ b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp @@ -460,7 +460,7 @@ TEST_F(UnsafeBufferUsageTest, UnsafePointerInGlobalVariableInitializer) { } TEST_F(UnsafeBufferUsageTest, UnsafePointerInFieldInitializer) { - auto Sum = setUpTest<>(R"cpp( + auto Sum = setUpTest(R"cpp( int *gp; struct Foo { int field = gp[5]; >From deef1a0a65f6e03f19f6fa74ea232b6149b570be Mon Sep 17 00:00:00 2001 From: Ziqing Luo <[email protected]> Date: Mon, 16 Mar 2026 17:36:39 -0700 Subject: [PATCH 4/4] Address comments --- .../UnsafeBufferUsageTest.cpp | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp index db11717a86747..8108ef8ad77b5 100644 --- a/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp +++ b/clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp @@ -472,6 +472,35 @@ TEST_F(UnsafeBufferUsageTest, UnsafePointerInFieldInitializer) { EXPECT_EQ(*Sum, makeSet(__LINE__, {{"gp", 1U}})); } +TEST_F(UnsafeBufferUsageTest, UnsafePointerInFieldInitializer2) { + auto Sum = setUpTest(R"cpp( + int *gp; + union Foo { + int field = gp[5]; + int x; + }; + )cpp", + {"Foo"}); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"gp", 1U}})); +} + +TEST_F(UnsafeBufferUsageTest, InitializerList) { + auto Sum = setUpTest(R"cpp( + int *gp; + struct Foo { + int field; + int x; + }; + Foo FooObj{gp[5], 0}; + )cpp", + {"FooObj"}); + + EXPECT_NE(Sum, nullptr); + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"gp", 1U}})); +} + TEST_F(UnsafeBufferUsageTest, UnsafePointerInCXXCtorInitializer) { auto Sum = setUpTest<CXXConstructorDecl>(R"cpp( struct Foo { @@ -496,4 +525,55 @@ TEST_F(UnsafeBufferUsageTest, UnsafePointerInDefaultArg) { EXPECT_EQ(*Sum, makeSet(__LINE__, {{"gp", 1U}})); } +TEST_F(UnsafeBufferUsageTest, NestedDefinitions) { + StringRef Code = R"cpp( + int * a = [](){ + struct Foo { + void bar(int * ptr) { ptr[3] = 0; } + }; + return nullptr; + }(); + )cpp"; + auto Sum = setUpTest(Code, {"bar"}); + + EXPECT_NE(Sum, nullptr); + // The closest contributor owns the fact: + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"ptr", 1U}})); + + Sum = setUpTest(Code, {"Foo"}); + + EXPECT_NE(Sum, nullptr); + EXPECT_TRUE(Sum->empty()); + + Sum = setUpTest(Code, {"a"}); + + EXPECT_NE(Sum, nullptr); + EXPECT_TRUE(Sum->empty()); +} + +TEST_F(UnsafeBufferUsageTest, NestedDefinitions2) { + StringRef Code = R"cpp( + int main(void) { + struct Foo { + void bar(int * ptr) { ptr[3] = 0; } + }; + } + )cpp"; + auto Sum = setUpTest(Code, {"bar"}); + + EXPECT_NE(Sum, nullptr); + // The closest contributor owns the fact: + EXPECT_EQ(*Sum, makeSet(__LINE__, {{"ptr", 1U}})); + + Sum = setUpTest(Code, {"Foo"}); + + EXPECT_NE(Sum, nullptr); + EXPECT_TRUE(Sum->empty()); + + Sum = setUpTest(Code, {"main"}); + + EXPECT_NE(Sum, nullptr); + EXPECT_TRUE(Sum->empty()); +} + } // namespace _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
