https://github.com/ziqingluo-90 updated https://github.com/llvm/llvm-project/pull/210457
>From 939d94d957220cb029d19b310110feb182c30a46 Mon Sep 17 00:00:00 2001 From: Jan Korous <[email protected]> Date: Fri, 17 Jul 2026 13:50:27 -0700 Subject: [PATCH 1/2] [clang][ssaf] Add cpp-bounded-buffers source transformation Adds the first built-in `Transformation`, `cpp-bounded-buffers`, which rewrites buffers -- raw pointers and arrays -- into bounded types (`bounded_ptr<T>`, `bounded_array<T, N>`) using the reachability computed by `UnsafeBufferReachableAnalysis`. The transformation collects every reachable pointer/array declarator and function return declared in the translation unit, then either rewrites it or records a SARIF note explaining why it did not. Shapes that are not yet handled -- multi-level pointers, pointer to array, references to pointers, multi-dimensional and unbounded arrays, multi-declarator groups, macro-spelled declarators, and trailing return types -- are reported rather than rewritten, and a final pass reports any reachable entity that was neither rewritten nor otherwise accounted for. Edits are validated and committed atomically, so a declarator whose edit cannot be formed (such as a raw array of function pointers) is reported instead of mangled. Anchored via `CppBoundedBuffersAnchorSource` so static builds keep the registration. --- .../BuiltinAnchorSources.def | 1 + .../Transformations/CppBoundedBuffers.h | 77 +++ .../SourceTransformation/CMakeLists.txt | 2 + .../Transformations/CppBoundedBuffers.cpp | 482 ++++++++++++++++++ .../ScalableStaticAnalysis/CMakeLists.txt | 1 + .../CppBoundedBuffersTest.cpp | 397 +++++++++++++++ 6 files changed, 960 insertions(+) create mode 100644 clang/include/clang/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.h create mode 100644 clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp create mode 100644 clang/unittests/ScalableStaticAnalysis/SourceTransformation/CppBoundedBuffersTest.cpp diff --git a/clang/include/clang/ScalableStaticAnalysis/BuiltinAnchorSources.def b/clang/include/clang/ScalableStaticAnalysis/BuiltinAnchorSources.def index a128ced676ed3..4af2184b9d8a6 100644 --- a/clang/include/clang/ScalableStaticAnalysis/BuiltinAnchorSources.def +++ b/clang/include/clang/ScalableStaticAnalysis/BuiltinAnchorSources.def @@ -19,6 +19,7 @@ ANCHOR(AnalysisRegistryAnchorSource) ANCHOR(CallGraphExtractorAnchorSource) ANCHOR(CallGraphJSONFormatAnchorSource) +ANCHOR(CppBoundedBuffersAnchorSource) ANCHOR(EntitySourceLocationExtractorAnchorSource) ANCHOR(JSONFormatAnchorSource) ANCHOR(TypeConstrainedPointersAnchorSource) diff --git a/clang/include/clang/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.h b/clang/include/clang/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.h new file mode 100644 index 0000000000000..5b37282b313cf --- /dev/null +++ b/clang/include/clang/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.h @@ -0,0 +1,77 @@ +//===- CppBoundedBuffers.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 +// +//===----------------------------------------------------------------------===// +// +// The cpp-bounded-buffers transformation rewrites buffers -- raw pointers and +// arrays -- reachable from unsafe buffer usage into bounded types +// (bounded_ptr<T>, bounded_array<T, N>). Reachable declarators that are not +// rewritten are reported instead, so no reachable buffer is silently left raw. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_SCALABLESTATICANALYSIS_SOURCETRANSFORMATION_TRANSFORMATIONS_CPPBOUNDEDBUFFERS_H +#define LLVM_CLANG_SCALABLESTATICANALYSIS_SOURCETRANSFORMATION_TRANSFORMATIONS_CPPBOUNDEDBUFFERS_H + +#include "clang/AST/Type.h" +#include "clang/ScalableStaticAnalysis/SourceTransformation/Transformation.h" +#include "llvm/ADT/SmallSet.h" +#include "llvm/ADT/StringRef.h" +#include <optional> +#include <string> + +namespace clang { +class ASTContext; +} // namespace clang + +namespace clang::ssaf { + +/// The bounded type a raw declarator is rewritten to. +enum class BoundedType { Ptr, Array }; + +/// Why a reachable declarator was reported instead of rewritten. +enum class ReportReason { + MultiLevelPointer, + PointerToArray, + ReferenceToPointer, + MultiDimensionalArray, + IncompleteArray, + UnreproducibleType, + DeclarationGroup, + MacroExpansion, + TrailingReturnType, + EmissionFailed, + NotTransformed, +}; + +/// Returns the report message for \p Reason. +llvm::StringRef messageFor(ReportReason Reason); + +/// The outcome of classifying a declared type against the reachable pointer +/// levels of its entity: a bounded-type rewrite, a report reason, or neither. +struct ClassifyResult { + std::optional<BoundedType> NewType; + // Pointee/element spelling; meaningful only when NewType is set. + std::string InnerSpelling; + std::optional<ReportReason> Skip; +}; + +/// Classifies the declared type \p T of a reachable entity. \p ReachableLevels +/// holds the entity's reachable pointer levels (1-based, outermost is level 1). +ClassifyResult +classifyDeclType(QualType T, const llvm::SmallSet<unsigned, 4> &ReachableLevels, + const ASTContext &Ctx); + +class CppBoundedBuffers : public Transformation { +public: + using Transformation::Transformation; + + void HandleTranslationUnit(clang::ASTContext &Ctx) override; +}; + +} // namespace clang::ssaf + +#endif // LLVM_CLANG_SCALABLESTATICANALYSIS_SOURCETRANSFORMATION_TRANSFORMATIONS_CPPBOUNDEDBUFFERS_H diff --git a/clang/lib/ScalableStaticAnalysis/SourceTransformation/CMakeLists.txt b/clang/lib/ScalableStaticAnalysis/SourceTransformation/CMakeLists.txt index 25db2fb7eca7d..96e85ec3a50a8 100644 --- a/clang/lib/ScalableStaticAnalysis/SourceTransformation/CMakeLists.txt +++ b/clang/lib/ScalableStaticAnalysis/SourceTransformation/CMakeLists.txt @@ -5,11 +5,13 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangScalableStaticAnalysisSourceTransformation SARIFTransformationReportFormat.cpp TransformationRegistry.cpp + Transformations/CppBoundedBuffers.cpp YAMLSourceEditFormat.cpp LINK_LIBS clangAST clangBasic + clangLex clangScalableStaticAnalysisCore clangToolingCore ) diff --git a/clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp b/clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp new file mode 100644 index 0000000000000..25816420e504a --- /dev/null +++ b/clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp @@ -0,0 +1,482 @@ +//===- CppBoundedBuffers.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/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclBase.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DynamicRecursiveASTVisitor.h" +#include "clang/AST/Type.h" +#include "clang/AST/TypeLoc.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Lex/Lexer.h" +#include "clang/ScalableStaticAnalysis/Analyses/EntityPointerLevel/EntityPointerLevel.h" +#include "clang/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsageAnalysis.h" +#include "clang/ScalableStaticAnalysis/Core/ASTEntityMapping.h" +#include "clang/ScalableStaticAnalysis/Core/Model/EntityId.h" +#include "clang/ScalableStaticAnalysis/Core/Model/EntityIdTable.h" +#include "clang/ScalableStaticAnalysis/Core/Model/EntityName.h" +#include "clang/ScalableStaticAnalysis/SourceTransformation/TransformationRegistry.h" +#include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallVector.h" +#include <cassert> +#include <map> +#include <string> + +using namespace clang; +using namespace clang::ssaf; + +static constexpr llvm::StringLiteral SkippedRuleId = + "cpp-bounded-buffers-skipped"; + +namespace { + +/// A declarator whose type can carry pointer levels. +bool isCandidateType(QualType T) { + QualType U = T.getNonReferenceType(); + return U->isPointerType() || U->isArrayType(); +} + +std::string spell(QualType T, const ASTContext &Ctx) { + return T.getAsString(Ctx.getPrintingPolicy()); +} + +/// Whether \p T can be re-emitted as written. Anonymous records and lambdas +/// have no usable spelling. +bool isReproducible(QualType T) { + const auto *RT = T->getAs<RecordType>(); + if (!RT) + return true; + const RecordDecl *RD = RT->getDecl(); + if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) + if (CXXRD->isLambda()) + return false; + return RD->getIdentifier() || RD->getTypedefNameForAnonDecl(); +} + +std::string cvPrefix(QualType T) { + std::string Prefix; + if (T.isLocalConstQualified()) + Prefix += "const "; + if (T.isLocalVolatileQualified()) + Prefix += "volatile "; + return Prefix; +} + +std::string renderNewType(const ClassifyResult &R, QualType T, + const ASTContext &Ctx) { + if (*R.NewType == BoundedType::Ptr) + return cvPrefix(T) + "bounded_ptr<" + R.InnerSpelling + "> "; + const auto *CAT = Ctx.getAsConstantArrayType(T); + std::string N = std::to_string(CAT->getSize().getZExtValue()); + return "bounded_array<" + R.InnerSpelling + ", " + N + "> "; +} + +/// Whether another declarator in \p D's lexical context shares its type +/// specifier, i.e. \p D is one declarator of a multi-declarator group. +bool sharesTypeSpecifier(const DeclaratorDecl *D) { + const TypeSourceInfo *TSI = D->getTypeSourceInfo(); + const DeclContext *DC = D->getLexicalDeclContext(); + if (!TSI || !DC) + return false; + SourceLocation Begin = TSI->getTypeLoc().getBeginLoc(); + for (const Decl *Sibling : DC->decls()) { + if (Sibling == D) + continue; + const auto *Other = dyn_cast<DeclaratorDecl>(Sibling); + if (Other && Other->getTypeSourceInfo() && + Other->getTypeSourceInfo()->getTypeLoc().getBeginLoc() == Begin) + return true; + } + return false; +} + +bool hasTrailingReturnType(const FunctionDecl *FD) { + const auto *FPT = FD->getType()->getAs<FunctionProtoType>(); + return FPT && FPT->hasTrailingReturn(); +} + +CharSourceRange declTypeRange(const DeclaratorDecl *D) { + if (const TypeSourceInfo *TSI = D->getTypeSourceInfo()) + return CharSourceRange::getTokenRange(TSI->getTypeLoc().getSourceRange()); + return CharSourceRange::getTokenRange(D->getSourceRange()); +} + +/// A leading cv-qualifier keyword (e.g. the `const` in `const char *`) is not +/// covered by the type-loc's begin location; extend \p TypeBegin left over it. +SourceLocation extendOverLeadingQualifiers(SourceLocation TypeBegin, + const ASTContext &Ctx) { + const SourceManager &SM = Ctx.getSourceManager(); + const LangOptions &LangOpts = Ctx.getLangOpts(); + while (std::optional<Token> Prev = Lexer::findPreviousToken( + TypeBegin, SM, LangOpts, /*IncludeComments=*/false)) { + // findPreviousToken lexes raw tokens, so keywords arrive as identifiers. + if (!Prev->is(tok::raw_identifier)) + break; + StringRef Text = Prev->getRawIdentifier(); + if (Text != "const" && Text != "volatile") + break; + TypeBegin = Prev->getLocation(); + } + return TypeBegin; +} + +/// Reverse index from the whole-program reachability result onto entity names, +/// so a declaration in this TU can look up its reachable pointer levels. +class ReachabilityMap { + const std::map<EntityId, EntityPointerLevelSet> &Reachables; + std::map<EntityName, EntityId> NameToId; + +public: + ReachabilityMap(const WPASuite &Suite, + const std::map<EntityId, EntityPointerLevelSet> &Reachables) + : Reachables(Reachables) { + Suite.getIdTable().forEach([this](const EntityName &Name, EntityId Id) { + NameToId.emplace(Name, Id); + }); + } + + llvm::SmallSet<unsigned, 4> levelsFor(std::optional<EntityName> Name) const { + llvm::SmallSet<unsigned, 4> Levels; + if (!Name) + return Levels; + auto NameIt = NameToId.find(*Name); + if (NameIt == NameToId.end()) + return Levels; + auto ReachIt = Reachables.find(NameIt->second); + if (ReachIt == Reachables.end()) + return Levels; + for (const EntityPointerLevel &EPL : ReachIt->second) + Levels.insert(EPL.getPointerLevel()); + return Levels; + } +}; + +struct Candidate { + llvm::SmallSet<unsigned, 4> Levels; + bool AccountedFor = false; +}; + +using DeclLevels = std::map<const Decl *, Candidate>; +using ReturnLevels = std::map<const FunctionDecl *, Candidate>; + +/// Collects the reachable pointer/array declarators and function returns +/// declared in this TU. +class CollectVisitor : public DynamicRecursiveASTVisitor { +public: + CollectVisitor(const ReachabilityMap &Reach, DeclLevels &Decls, + ReturnLevels &Returns) + : Reach(Reach), Decls(Decls), Returns(Returns) {} + + bool VisitVarDecl(VarDecl *D) override { + collect(D, D->getType(), getEntityName(D)); + return true; + } + + bool VisitFieldDecl(FieldDecl *D) override { + collect(D, D->getType(), getEntityName(D)); + return true; + } + + bool VisitFunctionDecl(FunctionDecl *FD) override { + if (!FD->isTemplated() && isCandidateType(FD->getReturnType())) { + llvm::SmallSet<unsigned, 4> Levels = + Reach.levelsFor(getEntityNameForReturn(FD)); + if (!Levels.empty()) + Returns[FD].Levels = std::move(Levels); + } + return true; + } + +private: + void collect(const Decl *D, QualType T, std::optional<EntityName> Name) { + if (D->isTemplated() || !isCandidateType(T)) + return; + llvm::SmallSet<unsigned, 4> Levels = Reach.levelsFor(Name); + if (!Levels.empty()) + Decls[D].Levels = std::move(Levels); + } + + const ReachabilityMap &Reach; + DeclLevels &Decls; + ReturnLevels &Returns; +}; + +/// Rewrites or reports every collected declarator and function return. +class RewriteVisitor : public DynamicRecursiveASTVisitor { +public: + RewriteVisitor(ASTContext &Ctx, DeclLevels &Decls, ReturnLevels &Returns, + SourceEditEmitter &Edits, TransformationReportEmitter &Report) + : Ctx(Ctx), Decls(Decls), Returns(Returns), Edits(Edits), Report(Report) { + } + + bool VisitVarDecl(VarDecl *D) override { + processDecl(D, D->getType()); + return true; + } + + bool VisitFieldDecl(FieldDecl *D) override { + processDecl(D, D->getType()); + return true; + } + + bool VisitFunctionDecl(FunctionDecl *FD) override { + auto It = Returns.find(FD); + if (It == Returns.end()) + return true; + Candidate &Cand = It->second; + if (hasTrailingReturnType(FD)) + return account(Cand, FD, ReportReason::TrailingReturnType); + + SourceLocation TypeBegin = FD->getReturnTypeSourceRange().getBegin(); + SourceLocation NameLoc = FD->getLocation(); + if (TypeBegin.isMacroID() || NameLoc.isMacroID()) + return account(Cand, FD, ReportReason::MacroExpansion); + + ClassifyResult R = classifyDeclType(FD->getReturnType(), Cand.Levels, Ctx); + if (R.Skip) + return account(Cand, FD, *R.Skip); + if (R.NewType) { + bool Ok = emit(TypeBegin, NameLoc, FD->getReturnType(), R, + /*ArrayTypeLoc=*/std::nullopt); + return account(Cand, FD, + Ok ? std::nullopt + : std::optional(ReportReason::EmissionFailed)); + } + return true; + } + +private: + void processDecl(DeclaratorDecl *D, QualType T) { + auto It = Decls.find(D); + if (It == Decls.end()) + return; + Candidate &Cand = It->second; + if (sharesTypeSpecifier(D)) + return (void)account(Cand, D, ReportReason::DeclarationGroup); + + const TypeSourceInfo *TSI = D->getTypeSourceInfo(); + SourceLocation TypeBegin = + TSI ? TSI->getTypeLoc().getBeginLoc() : SourceLocation(); + SourceLocation NameLoc = D->getLocation(); + if (TypeBegin.isMacroID() || NameLoc.isMacroID()) + return (void)account(Cand, D, ReportReason::MacroExpansion); + + ClassifyResult R = classifyDeclType(T, Cand.Levels, Ctx); + if (R.Skip) + return (void)account(Cand, D, *R.Skip); + if (R.NewType) { + std::optional<TypeLoc> ArrayTypeLoc; + if (*R.NewType == BoundedType::Array && TSI) + ArrayTypeLoc = TSI->getTypeLoc(); + bool Ok = emit(TypeBegin, NameLoc, T, R, ArrayTypeLoc); + account(Cand, D, + Ok ? std::nullopt : std::optional(ReportReason::EmissionFailed)); + } + } + + /// Emits the type-token replacement (and, for arrays, deletes the trailing + /// extent). Returns false without emitting anything if a valid, + /// self-contained edit cannot be formed. + bool emit(SourceLocation TypeBegin, SourceLocation NameLoc, QualType T, + const ClassifyResult &R, std::optional<TypeLoc> ForArray) { + const SourceManager &SM = Ctx.getSourceManager(); + if (TypeBegin.isValid() && !TypeBegin.isMacroID()) + TypeBegin = extendOverLeadingQualifiers(TypeBegin, Ctx); + if (TypeBegin.isInvalid() || NameLoc.isInvalid() || TypeBegin.isMacroID() || + NameLoc.isMacroID() || + SM.getFileID(TypeBegin) != SM.getFileID(NameLoc) || + SM.getFileOffset(NameLoc) <= SM.getFileOffset(TypeBegin)) + return false; + + llvm::SmallVector<tooling::Replacement, 2> Edited; + Edited.emplace_back(SM, CharSourceRange::getCharRange(TypeBegin, NameLoc), + renderNewType(R, T, Ctx), Ctx.getLangOpts()); + + if (ForArray) { + ArrayTypeLoc ATL = ForArray->getAs<ArrayTypeLoc>(); + if (!ATL) + return false; + SourceLocation LBracket = ATL.getLBracketLoc(); + SourceLocation RBracket = ATL.getRBracketLoc(); + // A clean array declarator ends at its closing bracket; otherwise the + // element spelling wraps the name (e.g. an array of function pointers) + // and cannot be rewritten by stripping a trailing extent. + if (LBracket.isInvalid() || RBracket.isInvalid() || + ForArray->getEndLoc() != RBracket) + return false; + Edited.emplace_back(SM, + CharSourceRange::getTokenRange(LBracket, RBracket), + "", Ctx.getLangOpts()); + } + + for (const tooling::Replacement &Repl : Edited) + if (!Repl.isApplicable()) + return false; + for (tooling::Replacement &Repl : Edited) + Edits.addReplacement(std::move(Repl)); + return true; + } + + /// Marks \p Cand accounted for, reporting \p Reason if one is given. + bool account(Candidate &Cand, const DeclaratorDecl *D, + std::optional<ReportReason> Reason) { + Cand.AccountedFor = true; + if (Reason) + Report.addResult(SkippedRuleId, SarifResultLevel::Note, declTypeRange(D), + messageFor(*Reason)); + return true; + } + + ASTContext &Ctx; + DeclLevels &Decls; + ReturnLevels &Returns; + SourceEditEmitter &Edits; + TransformationReportEmitter &Report; +}; + +} // namespace + +namespace clang::ssaf { + +llvm::StringRef messageFor(ReportReason Reason) { + switch (Reason) { + case ReportReason::MultiLevelPointer: + return "multi-level pointer indirection is not yet rewritten"; + case ReportReason::PointerToArray: + return "pointer to array is not yet rewritten"; + case ReportReason::ReferenceToPointer: + return "reference to pointer is not yet rewritten"; + case ReportReason::MultiDimensionalArray: + return "multi-dimensional array is not yet rewritten"; + case ReportReason::IncompleteArray: + return "array of unknown bound is not yet rewritten"; + case ReportReason::UnreproducibleType: + return "type spelling cannot be reproduced"; + case ReportReason::DeclarationGroup: + return "declarator of a multi-declarator group is not yet rewritten"; + case ReportReason::MacroExpansion: + return "declarator spelled through a macro is not yet rewritten"; + case ReportReason::TrailingReturnType: + return "trailing return type is not yet rewritten"; + case ReportReason::EmissionFailed: + return "no source edit could be formed for this declarator"; + case ReportReason::NotTransformed: + return "reachable buffer was not transformed"; + } + llvm_unreachable("unhandled ReportReason"); +} + +ClassifyResult +classifyDeclType(QualType T, const llvm::SmallSet<unsigned, 4> &ReachableLevels, + const ASTContext &Ctx) { + ClassifyResult R; + if (!ReachableLevels.count(1)) + return R; + + // A deeper indirection level is reachable too; that is a multi-level rewrite, + // which is not yet supported. + if (llvm::any_of(ReachableLevels, [](unsigned L) { return L > 1; })) { + R.Skip = ReportReason::MultiLevelPointer; + return R; + } + + if (T->isReferenceType()) { + QualType Pointee = T.getNonReferenceType(); + if (Pointee->isPointerType() || Pointee->isArrayType()) + R.Skip = ReportReason::ReferenceToPointer; + return R; + } + + if (const auto *PT = T->getAs<PointerType>()) { + QualType Pointee = PT->getPointeeType(); + if (Pointee->isFunctionType()) { + assert(false && + "function pointer entities are not expected to be reachable"); + return R; + } + if (Pointee->isPointerType()) { + R.Skip = ReportReason::MultiLevelPointer; + return R; + } + if (Pointee->isArrayType()) { + R.Skip = ReportReason::PointerToArray; + return R; + } + if (!isReproducible(Pointee)) { + R.Skip = ReportReason::UnreproducibleType; + return R; + } + R.NewType = BoundedType::Ptr; + R.InnerSpelling = Pointee->isVoidType() ? "char" : spell(Pointee, Ctx); + return R; + } + + if (const auto *CAT = Ctx.getAsConstantArrayType(T)) { + QualType Element = CAT->getElementType(); + if (Element->isArrayType()) { + R.Skip = ReportReason::MultiDimensionalArray; + return R; + } + if (!isReproducible(Element)) { + R.Skip = ReportReason::UnreproducibleType; + return R; + } + R.NewType = BoundedType::Array; + R.InnerSpelling = spell(Element, Ctx); + return R; + } + + if (T->isArrayType()) + R.Skip = ReportReason::IncompleteArray; + return R; +} + +void CppBoundedBuffers::HandleTranslationUnit(ASTContext &Ctx) { + auto Reachable = Suite.get<UnsafeBufferReachableAnalysisResult>(); + if (!Reachable) { + llvm::consumeError(Reachable.takeError()); + return; + } + + ReachabilityMap Reach(Suite, Reachable->Reachables); + DeclLevels Decls; + ReturnLevels Returns; + + Decl *TU = Ctx.getTranslationUnitDecl(); + CollectVisitor(Reach, Decls, Returns).TraverseDecl(TU); + RewriteVisitor(Ctx, Decls, Returns, Edits, Report).TraverseDecl(TU); + + // Every reachable buffer in this TU is either rewritten or reported; a + // leftover means it was neither, which must still be surfaced. + for (const auto &[D, Cand] : Decls) + if (!Cand.AccountedFor) + Report.addResult(SkippedRuleId, SarifResultLevel::Note, + declTypeRange(cast<DeclaratorDecl>(D)), + messageFor(ReportReason::NotTransformed)); + for (const auto &[FD, Cand] : Returns) + if (!Cand.AccountedFor) + Report.addResult( + SkippedRuleId, SarifResultLevel::Note, + CharSourceRange::getTokenRange(FD->getReturnTypeSourceRange()), + messageFor(ReportReason::NotTransformed)); +} + +} // namespace clang::ssaf + +namespace clang::ssaf { +// NOLINTNEXTLINE(misc-use-internal-linkage) +volatile int CppBoundedBuffersAnchorSource = 0; +} // namespace clang::ssaf + +static clang::ssaf::TransformationRegistry::Add<CppBoundedBuffers> + RegisterCppBoundedBuffers("cpp-bounded-buffers", + "Rewrites buffers into bounded types"); diff --git a/clang/unittests/ScalableStaticAnalysis/CMakeLists.txt b/clang/unittests/ScalableStaticAnalysis/CMakeLists.txt index ed3b57168b069..0f3bf9ad4512b 100644 --- a/clang/unittests/ScalableStaticAnalysis/CMakeLists.txt +++ b/clang/unittests/ScalableStaticAnalysis/CMakeLists.txt @@ -27,6 +27,7 @@ add_distinct_clang_unittest(ClangScalableAnalysisTests Serialization/JSONFormatTest/LUSummaryTest.cpp Serialization/JSONFormatTest/SharedLexicalRepresentationFormatTest.cpp Serialization/JSONFormatTest/TUSummaryTest.cpp + SourceTransformation/CppBoundedBuffersTest.cpp SourceTransformation/EmitterTest.cpp SourceTransformation/RegistryTest.cpp SourceTransformation/SARIFFormatTest.cpp diff --git a/clang/unittests/ScalableStaticAnalysis/SourceTransformation/CppBoundedBuffersTest.cpp b/clang/unittests/ScalableStaticAnalysis/SourceTransformation/CppBoundedBuffersTest.cpp new file mode 100644 index 0000000000000..ca6e5e3894e70 --- /dev/null +++ b/clang/unittests/ScalableStaticAnalysis/SourceTransformation/CppBoundedBuffersTest.cpp @@ -0,0 +1,397 @@ +//===- CppBoundedBuffersTest.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/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.h" +#include "FindDecl.h" +#include "TestFixture.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/Basic/Sarif.h" +#include "clang/ScalableStaticAnalysis/Analyses/EntityPointerLevel/EntityPointerLevel.h" +#include "clang/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsageAnalysis.h" +#include "clang/ScalableStaticAnalysis/Core/ASTEntityMapping.h" +#include "clang/ScalableStaticAnalysis/Core/Model/EntityId.h" +#include "clang/ScalableStaticAnalysis/Core/Model/EntityIdTable.h" +#include "clang/ScalableStaticAnalysis/Core/Model/EntityName.h" +#include "clang/ScalableStaticAnalysis/Core/WholeProgramAnalysis/WPASuite.h" +#include "clang/ScalableStaticAnalysis/SourceTransformation/SourceEditEmitter.h" +#include "clang/ScalableStaticAnalysis/SourceTransformation/TransformationReportEmitter.h" +#include "clang/Tooling/Core/Replacement.h" +#include "clang/Tooling/Tooling.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/SmallSet.h" +#include "llvm/Support/Error.h" +#include "gtest/gtest.h" +#include <memory> +#include <optional> +#include <string> +#include <vector> + +using namespace clang; +using namespace clang::ssaf; + +namespace { + +class RecordingEditEmitter : public SourceEditEmitter { +public: + std::vector<tooling::Replacement> Replacements; + + void addReplacement(tooling::Replacement R) override { + Replacements.push_back(std::move(R)); + } +}; + +class RecordingReportEmitter : public TransformationReportEmitter { +public: + struct Entry { + std::string RuleId; + SarifResultLevel Level; + std::string Message; + }; + std::vector<Entry> Results; + + void addResult(StringRef RuleId, SarifResultLevel Level, CharSourceRange, + StringRef Message) override { + Results.push_back({RuleId.str(), Level, Message.str()}); + } +}; + +std::optional<EntityName> varEntity(StringRef Name, ASTContext &Ctx) { + return getEntityName(findDeclByName<VarDecl>(Name, Ctx)); +} + +std::optional<EntityName> fieldEntity(StringRef Name, ASTContext &Ctx) { + return getEntityName(findDeclByName<FieldDecl>(Name, Ctx)); +} + +std::optional<EntityName> paramEntity(StringRef Fn, unsigned Idx, + ASTContext &Ctx) { + const FunctionDecl *FD = findFnByName(Fn, Ctx); + return FD ? getEntityName(FD->getParamDecl(Idx)) : std::nullopt; +} + +std::optional<EntityName> returnEntity(StringRef Fn, ASTContext &Ctx) { + return getEntityNameForReturn(findFnByName(Fn, Ctx)); +} + +struct Captured { + std::string Rewritten; + std::vector<RecordingReportEmitter::Entry> Reports; +}; + +class CppBoundedBuffersTest : public TestFixture { +protected: + using EntityFn = llvm::function_ref<std::optional<EntityName>(ASTContext &)>; + using MarkFn = llvm::function_ref<void( + ASTContext &, WPASuite &, UnsafeBufferReachableAnalysisResult &)>; + + // Marks the entity \p Name reachable at \p Levels in \p Result. + static void markReachable(WPASuite &Suite, + UnsafeBufferReachableAnalysisResult &Result, + std::optional<EntityName> Name, + ArrayRef<unsigned> Levels) { + if (!Name || Levels.empty()) + return; + EntityId Id = getIdTable(Suite).getId(*Name); + EntityPointerLevelSet Set; + for (unsigned Level : Levels) + Set.insert(buildEntityPointerLevel(Id, Level)); + Result.Reachables[Id] = std::move(Set); + } + + // Parses \p Code, lets \p Mark populate the reachable result, runs the + // transformation, and returns the rewritten source and report entries. + Captured runMarked(StringRef Code, MarkFn Mark) { + std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCode(Code); + ASTContext &Ctx = AST->getASTContext(); + + WPASuite Suite = makeWPASuite(); + auto Result = std::make_unique<UnsafeBufferReachableAnalysisResult>(); + Mark(Ctx, Suite, *Result); + getData(Suite)[UnsafeBufferReachableAnalysisResult::analysisName()] = + std::move(Result); + + RecordingEditEmitter Edits; + RecordingReportEmitter Report; + CppBoundedBuffers(Suite, Edits, Report).HandleTranslationUnit(Ctx); + + tooling::Replacements Replacements; + for (const tooling::Replacement &R : Edits.Replacements) + cantFail(Replacements.add(R)); + return {cantFail(tooling::applyAllReplacements(Code, Replacements)), + std::move(Report.Results)}; + } + + Captured run(StringRef Code, EntityFn EntityOf, ArrayRef<unsigned> Levels) { + return runMarked(Code, [&](ASTContext &Ctx, WPASuite &Suite, + UnsafeBufferReachableAnalysisResult &Result) { + markReachable(Suite, Result, EntityOf(Ctx), Levels); + }); + } +}; + +//===----------------------------------------------------------------------===// +// Rewrites: assert the rewritten source and that nothing is reported. +//===----------------------------------------------------------------------===// + +TEST_F(CppBoundedBuffersTest, PointerLocal) { + Captured C = run("void f() { int *p; }", + [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, "void f() { bounded_ptr<int> p; }"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, PointerParameter) { + Captured C = + run("void f(int *p);", + [](ASTContext &Ctx) { return paramEntity("f", 0, Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, "void f(bounded_ptr<int> p);"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, ConstQualifiedPointee) { + Captured C = run("const char *s;", + [](ASTContext &Ctx) { return varEntity("s", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, "bounded_ptr<const char> s;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, VoidPointer) { + Captured C = + run("void *p;", [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, "bounded_ptr<char> p;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, ArrayField) { + Captured C = run("struct S { int a[10]; };", + [](ASTContext &Ctx) { return fieldEntity("a", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, "struct S { bounded_array<int, 10> a; };"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, FunctionReturn) { + Captured C = + run("int *foo();", + [](ASTContext &Ctx) { return returnEntity("foo", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, "bounded_ptr<int> foo();"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, GlobalPointer) { + Captured C = + run("int *g;", [](ASTContext &Ctx) { return varEntity("g", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, "bounded_ptr<int> g;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, PointerField) { + Captured C = run("struct S { int *p; };", + [](ASTContext &Ctx) { return fieldEntity("p", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, "struct S { bounded_ptr<int> p; };"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, ArrayOfPointers) { + Captured C = run("int *a[10];", + [](ASTContext &Ctx) { return varEntity("a", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, "bounded_array<int *, 10> a;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, ArrayOfFunctionPointers) { + // A function-pointer element is accepted (not rejected like a bare function + // pointer); the typedef keeps the declarator a clean prefix + [N] suffix. + Captured C = run("typedef void (*FP)(); FP fps[4];", + [](ASTContext &Ctx) { return varEntity("fps", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, "typedef void (*FP)(); bounded_array<FP, 4> fps;"); + EXPECT_TRUE(C.Reports.empty()); +} + +//===----------------------------------------------------------------------===// +// Skips: assert no edit and a single reported reason. +//===----------------------------------------------------------------------===// + +void expectSkip(const Captured &C, StringRef Original, ReportReason Reason) { + EXPECT_EQ(C.Rewritten, Original); + ASSERT_EQ(C.Reports.size(), 1u); + EXPECT_EQ(C.Reports[0].Level, SarifResultLevel::Note); + EXPECT_EQ(C.Reports[0].Message, messageFor(Reason).str()); +} + +TEST_F(CppBoundedBuffersTest, MultiLevelPointer) { + StringRef Code = "int **pp;"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("pp", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::MultiLevelPointer); +} + +TEST_F(CppBoundedBuffersTest, MultiDimensionalArray) { + StringRef Code = "int a[2][3];"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("a", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::MultiDimensionalArray); +} + +TEST_F(CppBoundedBuffersTest, IncompleteArray) { + StringRef Code = "extern int a[];"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("a", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::IncompleteArray); +} + +TEST_F(CppBoundedBuffersTest, PointerToArray) { + StringRef Code = "int (*p)[10];"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::PointerToArray); +} + +TEST_F(CppBoundedBuffersTest, ReferenceToPointer) { + StringRef Code = "void f(int *&r);"; + Captured C = + run(Code, [](ASTContext &Ctx) { return paramEntity("f", 0, Ctx); }, {1}); + expectSkip(C, Code, ReportReason::ReferenceToPointer); +} + +TEST_F(CppBoundedBuffersTest, UnreproducibleType) { + StringRef Code = "struct { int x; } *p;"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::UnreproducibleType); +} + +TEST_F(CppBoundedBuffersTest, DeclarationGroup) { + // Both declarators share one type specifier; comma-group splitting is not + // yet supported, so both are reported and neither is rewritten. + Captured C = + runMarked("int *p, *q;", [](ASTContext &Ctx, WPASuite &Suite, + UnsafeBufferReachableAnalysisResult &Result) { + markReachable(Suite, Result, varEntity("p", Ctx), {1}); + markReachable(Suite, Result, varEntity("q", Ctx), {1}); + }); + EXPECT_EQ(C.Rewritten, "int *p, *q;"); + ASSERT_EQ(C.Reports.size(), 2u); + for (const auto &R : C.Reports) { + EXPECT_EQ(R.Level, SarifResultLevel::Note); + EXPECT_EQ(R.Message, messageFor(ReportReason::DeclarationGroup).str()); + } +} + +TEST_F(CppBoundedBuffersTest, MacroSpelledDeclarator) { + StringRef Code = "#define PTR int *\nPTR p;\n"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::MacroExpansion); +} + +TEST_F(CppBoundedBuffersTest, TrailingReturnType) { + StringRef Code = "auto f() -> int *;"; + Captured C = + run(Code, [](ASTContext &Ctx) { return returnEntity("f", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::TrailingReturnType); +} + +TEST_F(CppBoundedBuffersTest, EmissionFailureOnRawFunctionPointerArray) { + // A raw array-of-function-pointers has no clean prefix + [N] suffix, so the + // edit cannot be formed and the entity is reported rather than mangled. + StringRef Code = "void (*fps[4])();"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("fps", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::EmissionFailed); +} + +//===----------------------------------------------------------------------===// +// Completeness and negative cases. +//===----------------------------------------------------------------------===// + +TEST_F(CppBoundedBuffersTest, UnaccountedReachableIsReported) { + // A single pointer has only level 1; marking level 2 reachable leaves the + // entity neither rewritten nor shape-skipped, so the sweep reports it. + StringRef Code = "int *p;"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {2}); + expectSkip(C, Code, ReportReason::NotTransformed); +} + +TEST_F(CppBoundedBuffersTest, NotReachable) { + StringRef Code = "int *p;"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {}); + EXPECT_EQ(C.Rewritten, Code); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, RewriteAndReportCoexist) { + // A rewritten entity and a reported one in the same TU: the rewrite happens + // and only the not-rewritten entity is reported. + Captured C = runMarked( + "int *good; int **bad;", [](ASTContext &Ctx, WPASuite &Suite, + UnsafeBufferReachableAnalysisResult &Result) { + markReachable(Suite, Result, varEntity("good", Ctx), {1}); + markReachable(Suite, Result, varEntity("bad", Ctx), {1}); + }); + EXPECT_EQ(C.Rewritten, "bounded_ptr<int> good; int **bad;"); + ASSERT_EQ(C.Reports.size(), 1u); + EXPECT_EQ(C.Reports[0].Message, + messageFor(ReportReason::MultiLevelPointer).str()); +} + +//===----------------------------------------------------------------------===// +// Classifier unit tests: pin the level direction and message coverage. +//===----------------------------------------------------------------------===// + +QualType typeOf(StringRef Name, ASTContext &Ctx) { + return findDeclByName<VarDecl>(Name, Ctx)->getType(); +} + +TEST_F(CppBoundedBuffersTest, ClassifyRewritesOutermostReachablePointer) { + auto AST = tooling::buildASTFromCode("int *p;"); + llvm::SmallSet<unsigned, 4> Levels; + Levels.insert(1); + ClassifyResult R = classifyDeclType(typeOf("p", AST->getASTContext()), Levels, + AST->getASTContext()); + ASSERT_TRUE(R.NewType.has_value()); + EXPECT_EQ(*R.NewType, BoundedType::Ptr); + EXPECT_EQ(R.InnerSpelling, "int"); + EXPECT_FALSE(R.Skip.has_value()); +} + +TEST_F(CppBoundedBuffersTest, ClassifyIgnoresInnerOnlyReachablePointer) { + auto AST = tooling::buildASTFromCode("int *p;"); + llvm::SmallSet<unsigned, 4> Levels; + Levels.insert(2); + ClassifyResult R = classifyDeclType(typeOf("p", AST->getASTContext()), Levels, + AST->getASTContext()); + EXPECT_FALSE(R.NewType.has_value()); + EXPECT_FALSE(R.Skip.has_value()); +} + +TEST_F(CppBoundedBuffersTest, ClassifyMultiLevelPointerIsSkipped) { + auto AST = tooling::buildASTFromCode("int **pp;"); + llvm::SmallSet<unsigned, 4> Levels; + Levels.insert(1); + ClassifyResult R = classifyDeclType(typeOf("pp", AST->getASTContext()), + Levels, AST->getASTContext()); + EXPECT_FALSE(R.NewType.has_value()); + ASSERT_TRUE(R.Skip.has_value()); + EXPECT_EQ(*R.Skip, ReportReason::MultiLevelPointer); +} + +TEST_F(CppBoundedBuffersTest, MessageForIsNonEmpty) { + for (ReportReason Reason : + {ReportReason::MultiLevelPointer, ReportReason::PointerToArray, + ReportReason::ReferenceToPointer, ReportReason::MultiDimensionalArray, + ReportReason::IncompleteArray, ReportReason::UnreproducibleType, + ReportReason::DeclarationGroup, ReportReason::MacroExpansion, + ReportReason::TrailingReturnType, ReportReason::EmissionFailed, + ReportReason::NotTransformed}) + EXPECT_FALSE(messageFor(Reason).empty()); +} + +} // namespace >From 54e8b298437bc83771b3717fa77354cc0424bd4a Mon Sep 17 00:00:00 2001 From: Ziqing Luo <[email protected]> Date: Fri, 14 Aug 2026 22:51:24 -0700 Subject: [PATCH 2/2] Take over from Jan. The main change is to narrow the rewrite range: For pointers, let T mark the spelling of the pointee type according to its TypeLoc: ``` other-specifiers cv-qualifiers T * other-specifiers pointer-name |--- rewrite ---| ``` The rewrite range starts at the leading cv-qualifiers and ends at the STAR of the pointer. other-specifiers before the leading cv-qualifiers and after the STAR do not belong to the pointee type; they are therefore left untouched, regardless of whether they are macros or not. There should be nothing but trailing cv-qualifiers between T and the STAR; those cv-qualifiers do belong to the pointee type, so they are included in the rewrite range. For arrays, let T mark the spelling of the element type according to its TypeLoc: ``` other-specifiers cv-qualifiers T cv-qualifiers other-specifiers array-name [N] |--------- rewrite ---------| ``` The rewrite range starts at the leading cv-qualifiers and ends at the trailing cv-qualifiers. other-specifiers do not belong to the element type, so if any token intervenes between T and the trailing cv-qualifiers, the function fails to find a rewrite range and bails. --- .../Transformations/CppBoundedBuffers.h | 30 +- .../Transformations/CppBoundedBuffers.cpp | 397 ++++++++++++------ .../CppBoundedBuffersTest.cpp | 361 ++++++++++++++-- 3 files changed, 599 insertions(+), 189 deletions(-) diff --git a/clang/include/clang/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.h b/clang/include/clang/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.h index 5b37282b313cf..73d3b56586a45 100644 --- a/clang/include/clang/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.h +++ b/clang/include/clang/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.h @@ -34,29 +34,35 @@ enum class BoundedType { Ptr, Array }; /// Why a reachable declarator was reported instead of rewritten. enum class ReportReason { + ArrayNotEndInBracket, + DeclarationGroup, + EmissionFailed, + IncompleteArray, + MacroExpansion, + MultiDimensionalArray, MultiLevelPointer, + NoInnerTypeLoc, + NotPointerTypeEndWithStar, + NotTransformed, PointerToArray, ReferenceToPointer, - MultiDimensionalArray, - IncompleteArray, - UnreproducibleType, - DeclarationGroup, - MacroExpansion, TrailingReturnType, - EmissionFailed, - NotTransformed, + UnexpectedLeadingQualifier, + UnexpectedTrailingQualifier, + UnnamableType, }; /// Returns the report message for \p Reason. llvm::StringRef messageFor(ReportReason Reason); /// The outcome of classifying a declared type against the reachable pointer -/// levels of its entity: a bounded-type rewrite, a report reason, or neither. +/// levels of its entity: a bounded-type rewrite, or a report reason. struct ClassifyResult { - std::optional<BoundedType> NewType; - // Pointee/element spelling; meaningful only when NewType is set. + // Meaningful only when Skip is nullopt. + BoundedType NewType = BoundedType::Ptr; + // Pointee/element spelling; meaningful only when Skip is nullopt. std::string InnerSpelling; - std::optional<ReportReason> Skip; + std::optional<ReportReason> Skip = ReportReason::NotTransformed; }; /// Classifies the declared type \p T of a reachable entity. \p ReachableLevels @@ -65,7 +71,7 @@ ClassifyResult classifyDeclType(QualType T, const llvm::SmallSet<unsigned, 4> &ReachableLevels, const ASTContext &Ctx); -class CppBoundedBuffers : public Transformation { +class CppBoundedBuffers final : public Transformation { public: using Transformation::Transformation; diff --git a/clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp b/clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp index 25816420e504a..180d5e9f9c7d4 100644 --- a/clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp +++ b/clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp @@ -14,6 +14,7 @@ #include "clang/AST/DynamicRecursiveASTVisitor.h" #include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" +#include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Lex/Lexer.h" @@ -29,6 +30,7 @@ #include "llvm/ADT/SmallVector.h" #include <cassert> #include <map> +#include <optional> #include <string> using namespace clang; @@ -49,35 +51,24 @@ std::string spell(QualType T, const ASTContext &Ctx) { return T.getAsString(Ctx.getPrintingPolicy()); } -/// Whether \p T can be re-emitted as written. Anonymous records and lambdas -/// have no usable spelling. -bool isReproducible(QualType T) { - const auto *RT = T->getAs<RecordType>(); - if (!RT) - return true; - const RecordDecl *RD = RT->getDecl(); - if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) - if (CXXRD->isLambda()) - return false; - return RD->getIdentifier() || RD->getTypedefNameForAnonDecl(); -} - -std::string cvPrefix(QualType T) { - std::string Prefix; - if (T.isLocalConstQualified()) - Prefix += "const "; - if (T.isLocalVolatileQualified()) - Prefix += "volatile "; - return Prefix; +/// Whether \p T is a type with a name that can be used in template arguments. +bool isNamable(QualType T) { + if (!T->isTypedefNameType()) + if (const auto *RT = T->getAs<RecordType>()) { + const RecordDecl *RD = RT->getDecl(); + return RD->getIdentifier() || RD->getTypedefNameForAnonDecl(); + } + return true; } std::string renderNewType(const ClassifyResult &R, QualType T, const ASTContext &Ctx) { - if (*R.NewType == BoundedType::Ptr) - return cvPrefix(T) + "bounded_ptr<" + R.InnerSpelling + "> "; + assert(!R.Skip); + if (R.NewType == BoundedType::Ptr) + return "bounded_ptr<" + R.InnerSpelling + "> "; const auto *CAT = Ctx.getAsConstantArrayType(T); std::string N = std::to_string(CAT->getSize().getZExtValue()); - return "bounded_array<" + R.InnerSpelling + ", " + N + "> "; + return "bounded_array<" + R.InnerSpelling + ", " + N + ">"; } /// Whether another declarator in \p D's lexical context shares its type @@ -110,25 +101,113 @@ CharSourceRange declTypeRange(const DeclaratorDecl *D) { return CharSourceRange::getTokenRange(D->getSourceRange()); } -/// A leading cv-qualifier keyword (e.g. the `const` in `const char *`) is not -/// covered by the type-loc's begin location; extend \p TypeBegin left over it. -SourceLocation extendOverLeadingQualifiers(SourceLocation TypeBegin, - const ASTContext &Ctx) { +/// \return the pointee or element types TypeLoc if TL is a (qualified) pointer +/// or array type. +TypeLoc getInnerTypeLoc(TypeLoc TL) { + TL = TL.getUnqualifiedLoc(); + if (auto PTL = TL.getAs<PointerTypeLoc>()) + return PTL.getPointeeLoc(); + if (auto ATL = TL.getAs<ArrayTypeLoc>()) + return ATL.getElementLoc(); + return {}; +} + +/// Whether \p T spells a cv-qualifier keyword. +bool isCVQualifier(const Token &T) { + return T.is(tok::raw_identifier) && (T.getRawIdentifier() == "const" || + T.getRawIdentifier() == "volatile"); +} + +/// Probe leading qualifiers for a type 'T'. The probe is bounded in the range +/// [ \p DeclBegin, \p TypeBegin ), where the lower bound is the begin location +/// of the declaration where 'T' is spelled and the upper bound is the begin of +/// the spell of 'T'. +/// +/// The function updates \p TypeBegin if it finds cv-qualifiers preceding the +/// original \p TypeBegin without any other token intervening in between. \p +/// TypeBegin is not updated if there is no leading cv-qualifier. Otherwise, +/// returns the probe failed reason. +/// +/// \p TypeBegin is always token location. +std::optional<ReportReason> extendLeadingQualifiers(SourceLocation DeclBegin, + SourceLocation &TypeBegin, + const ASTContext &Ctx) { const SourceManager &SM = Ctx.getSourceManager(); const LangOptions &LangOpts = Ctx.getLangOpts(); - while (std::optional<Token> Prev = Lexer::findPreviousToken( - TypeBegin, SM, LangOpts, /*IncludeComments=*/false)) { - // findPreviousToken lexes raw tokens, so keywords arrive as identifiers. - if (!Prev->is(tok::raw_identifier)) - break; - StringRef Text = Prev->getRawIdentifier(); - if (Text != "const" && Text != "volatile") - break; - TypeBegin = Prev->getLocation(); + + std::optional<SourceLocation> FirstCVBegin; + std::optional<Token> Tok = Token(); + + if (Lexer::getRawToken(DeclBegin, *Tok, SM, LangOpts, + /*IgnoreWhiteSpace=*/true)) + return ReportReason::EmissionFailed; + while (SM.isBeforeInTranslationUnit(Tok->getLocation(), TypeBegin)) { + if (isCVQualifier(*Tok)) { + if (!FirstCVBegin) { + // Found first cv-qualifier, set `FirstCVBegin`. + FirstCVBegin = Tok->getLocation(); + } + } else if (FirstCVBegin) + // Bail when there is unexpected token between cv-qualifiers and the + // original TypeBegin: + return ReportReason::UnexpectedLeadingQualifier; + Tok = Lexer::findNextToken(Tok->getEndLoc(), SM, LangOpts, + /*IncludeComments=*/true); + if (!Tok) + return ReportReason::EmissionFailed; + } + if (FirstCVBegin) + TypeBegin = *FirstCVBegin; // set the real TypeBegin after propagation + return std::nullopt; +} + +/// Probe trailing qualifiers for a type 'T'. The probe is bounded in the range +/// ( \p TypeEnd, \p UpperBound ), where the lower bound is the end location +/// of 'T' and the upper bound should be a location within the declaration where +/// 'T' is spelled. +/// +/// The function updates \p TypeEnd if it finds cv-qualifiers following the +/// original \p TypeEnd without any other token intervening in between. +/// \p TypeEnd is not updated if there is no following cv-qualifier. Otherwise, +/// returns the probe failed reason. +/// +/// \p TypeBegin is always token location. +std::optional<ReportReason> extendTrailingQualifiers(SourceLocation &TypeEnd, + SourceLocation UpperBound, + const ASTContext &Ctx) { + const SourceManager &SM = Ctx.getSourceManager(); + const LangOptions &LangOpts = Ctx.getLangOpts(); + + std::optional<SourceLocation> LastCVBegin; + bool RunEnded = false; + + std::optional<Token> Tok = Lexer::findNextToken(TypeEnd, SM, LangOpts, + /*IncludeComments=*/true); + if (!Tok) + return ReportReason::EmissionFailed; + while (SM.isBeforeInTranslationUnit(Tok->getLocation(), UpperBound)) { + if (isCVQualifier(*Tok)) { + // Bail if there is anything unexpected between TypeEnd and a + // cv-qualifier. + if (RunEnded) + return ReportReason::UnexpectedTrailingQualifier; + LastCVBegin = Tok->getLocation(); + } else + RunEnded = true; + Tok = Lexer::findNextToken(Tok->getEndLoc(), SM, LangOpts, + /*IncludeComments=*/true); + if (!Tok) + return ReportReason::EmissionFailed; } - return TypeBegin; + if (LastCVBegin) + TypeEnd = *LastCVBegin; // set the real TypeEnd after propagation + return std::nullopt; } +using Levels = llvm::SmallSet<unsigned, 4>; +using DeclLevels = std::map<const Decl *, Levels>; +using ReturnLevels = std::map<const FunctionDecl *, Levels>; + /// Reverse index from the whole-program reachability result onto entity names, /// so a declaration in this TU can look up its reachable pointer levels. class ReachabilityMap { @@ -160,14 +239,6 @@ class ReachabilityMap { } }; -struct Candidate { - llvm::SmallSet<unsigned, 4> Levels; - bool AccountedFor = false; -}; - -using DeclLevels = std::map<const Decl *, Candidate>; -using ReturnLevels = std::map<const FunctionDecl *, Candidate>; - /// Collects the reachable pointer/array declarators and function returns /// declared in this TU. class CollectVisitor : public DynamicRecursiveASTVisitor { @@ -191,7 +262,7 @@ class CollectVisitor : public DynamicRecursiveASTVisitor { llvm::SmallSet<unsigned, 4> Levels = Reach.levelsFor(getEntityNameForReturn(FD)); if (!Levels.empty()) - Returns[FD].Levels = std::move(Levels); + Returns[FD] = std::move(Levels); } return true; } @@ -202,7 +273,7 @@ class CollectVisitor : public DynamicRecursiveASTVisitor { return; llvm::SmallSet<unsigned, 4> Levels = Reach.levelsFor(Name); if (!Levels.empty()) - Decls[D].Levels = std::move(Levels); + Decls[D] = std::move(Levels); } const ReachabilityMap &Reach; @@ -232,26 +303,23 @@ class RewriteVisitor : public DynamicRecursiveASTVisitor { auto It = Returns.find(FD); if (It == Returns.end()) return true; - Candidate &Cand = It->second; + const Levels &ReachableLevels = It->second; if (hasTrailingReturnType(FD)) - return account(Cand, FD, ReportReason::TrailingReturnType); + return report(FD, ReportReason::TrailingReturnType); - SourceLocation TypeBegin = FD->getReturnTypeSourceRange().getBegin(); SourceLocation NameLoc = FD->getLocation(); - if (TypeBegin.isMacroID() || NameLoc.isMacroID()) - return account(Cand, FD, ReportReason::MacroExpansion); - ClassifyResult R = classifyDeclType(FD->getReturnType(), Cand.Levels, Ctx); + ClassifyResult R = + classifyDeclType(FD->getReturnType(), ReachableLevels, Ctx); if (R.Skip) - return account(Cand, FD, *R.Skip); - if (R.NewType) { - bool Ok = emit(TypeBegin, NameLoc, FD->getReturnType(), R, - /*ArrayTypeLoc=*/std::nullopt); - return account(Cand, FD, - Ok ? std::nullopt - : std::optional(ReportReason::EmissionFailed)); - } - return true; + return report(FD, *R.Skip); + + FunctionTypeLoc FunTypeLoc = FD->getFunctionTypeLoc(); + + if (!FunTypeLoc) + return report(FD, ReportReason::EmissionFailed); + return report(FD, emit(FD->getBeginLoc(), NameLoc, + FunTypeLoc.getReturnLoc(), FD->getReturnType(), R)); } private: @@ -259,77 +327,139 @@ class RewriteVisitor : public DynamicRecursiveASTVisitor { auto It = Decls.find(D); if (It == Decls.end()) return; - Candidate &Cand = It->second; + const Levels &ReachableLevels = It->second; if (sharesTypeSpecifier(D)) - return (void)account(Cand, D, ReportReason::DeclarationGroup); + return (void)report(D, ReportReason::DeclarationGroup); const TypeSourceInfo *TSI = D->getTypeSourceInfo(); - SourceLocation TypeBegin = - TSI ? TSI->getTypeLoc().getBeginLoc() : SourceLocation(); + + if (!TSI) + return (void)report(D, ReportReason::EmissionFailed); + SourceLocation NameLoc = D->getLocation(); - if (TypeBegin.isMacroID() || NameLoc.isMacroID()) - return (void)account(Cand, D, ReportReason::MacroExpansion); + ClassifyResult R = classifyDeclType(T, ReachableLevels, Ctx); - ClassifyResult R = classifyDeclType(T, Cand.Levels, Ctx); if (R.Skip) - return (void)account(Cand, D, *R.Skip); - if (R.NewType) { - std::optional<TypeLoc> ArrayTypeLoc; - if (*R.NewType == BoundedType::Array && TSI) - ArrayTypeLoc = TSI->getTypeLoc(); - bool Ok = emit(TypeBegin, NameLoc, T, R, ArrayTypeLoc); - account(Cand, D, - Ok ? std::nullopt : std::optional(ReportReason::EmissionFailed)); + return (void)report(D, *R.Skip); + report(D, emit(D->getBeginLoc(), NameLoc, TSI->getTypeLoc(), T, R)); + } + + /// Compute the precise source range for rewriting. The produced range is + /// token range. + /// + /// For pointer types, the rewrite range is from the leading cv-qualifier of + /// the pointee type to the '*' token of the pointer type. + /// + /// For array types, the rewrite range is from the leading cv-qualifier to the + /// trailing cv-qualifier around the element type. It stops short of the + /// declarator name, leaving the name and the extent that follows it to be + /// handled separately. + /// + /// \param DeclBegin the begin location of the declaration, the lower bound of + /// the source range before narrowing down to the precise one. + /// \param NameLoc the location of the name of the declaration, the upper + /// bound of the source range before narrowing down to the precise one. + /// \param TLoc the TypeLoc of the type of the declaration + /// \param BoundedType indicates whether it is a pointer or an array + /// \return ReportReason if it cannot narrow down the rewrite range to the + /// aforementioned range. std::nullopt and updated \p Result otherwise. + std::optional<ReportReason> + computeRewriteRange(SourceLocation DeclBegin, SourceLocation NameLoc, + TypeLoc TLoc, BoundedType BoundedType, + const ASTContext &Ctx, SourceRange &RewriteRange) { + TypeLoc InnerTypeLoc = getInnerTypeLoc(TLoc); + + if (!InnerTypeLoc) + return ReportReason::NoInnerTypeLoc; + + SourceLocation RewriteRangeBegin = InnerTypeLoc.getBeginLoc(); + SourceRange Result; + + if (BoundedType == BoundedType::Ptr) { + auto PTL = TLoc.getUnqualifiedLoc().getAs<PointerTypeLoc>(); + + if (!PTL || TLoc.getEndLoc() != PTL.getStarLoc()) + return ReportReason::NotPointerTypeEndWithStar; + if (auto Reason = + extendLeadingQualifiers(DeclBegin, RewriteRangeBegin, Ctx)) + return Reason; + Result = {RewriteRangeBegin, PTL.getStarLoc()}; + } else { + SourceLocation RewriteRangeEnd = InnerTypeLoc.getEndLoc(); + + if (auto Reason = + extendLeadingQualifiers(DeclBegin, RewriteRangeBegin, Ctx)) + return Reason; + if (auto Reason = extendTrailingQualifiers(RewriteRangeEnd, NameLoc, Ctx)) + return Reason; + Result = {RewriteRangeBegin, RewriteRangeEnd}; } + + if (Result.getBegin().isMacroID() || Result.getEnd().isMacroID()) + return ReportReason::MacroExpansion; + if (Result.getBegin().isInvalid() || Result.getEnd().isInvalid()) + return ReportReason::EmissionFailed; + + const SourceManager &SM = Ctx.getSourceManager(); + if (SM.getFileID(Result.getBegin()) != SM.getFileID(Result.getEnd())) + return ReportReason::EmissionFailed; + RewriteRange = Result; + return std::nullopt; } /// Emits the type-token replacement (and, for arrays, deletes the trailing /// extent). Returns false without emitting anything if a valid, /// self-contained edit cannot be formed. - bool emit(SourceLocation TypeBegin, SourceLocation NameLoc, QualType T, - const ClassifyResult &R, std::optional<TypeLoc> ForArray) { + std::optional<ReportReason> emit(SourceLocation DeclBegin, + SourceLocation NameLoc, TypeLoc TLoc, + QualType T, const ClassifyResult &R) { const SourceManager &SM = Ctx.getSourceManager(); - if (TypeBegin.isValid() && !TypeBegin.isMacroID()) - TypeBegin = extendOverLeadingQualifiers(TypeBegin, Ctx); - if (TypeBegin.isInvalid() || NameLoc.isInvalid() || TypeBegin.isMacroID() || - NameLoc.isMacroID() || - SM.getFileID(TypeBegin) != SM.getFileID(NameLoc) || - SM.getFileOffset(NameLoc) <= SM.getFileOffset(TypeBegin)) - return false; + SourceRange TypeRewriteRange; + + if (auto Reason = computeRewriteRange(DeclBegin, NameLoc, TLoc, R.NewType, + Ctx, TypeRewriteRange)) + return Reason; + // TypeRewriteRange is bounded by the tokens (begin location) of the two + // ends. Now convert it to char range for source edit, which requires the + // bounds to be the characters of the two ends. + CharSourceRange TypeRewriteCharRange = + Lexer::getAsCharRange(TypeRewriteRange, SM, Ctx.getLangOpts()); llvm::SmallVector<tooling::Replacement, 2> Edited; - Edited.emplace_back(SM, CharSourceRange::getCharRange(TypeBegin, NameLoc), - renderNewType(R, T, Ctx), Ctx.getLangOpts()); - if (ForArray) { - ArrayTypeLoc ATL = ForArray->getAs<ArrayTypeLoc>(); + Edited.emplace_back(SM, TypeRewriteCharRange, renderNewType(R, T, Ctx), + Ctx.getLangOpts()); + + if (R.NewType == BoundedType::Array) { + ArrayTypeLoc ATL = TLoc.getUnqualifiedLoc().getAs<ArrayTypeLoc>(); + if (!ATL) - return false; + return ReportReason::EmissionFailed; + SourceLocation LBracket = ATL.getLBracketLoc(); SourceLocation RBracket = ATL.getRBracketLoc(); // A clean array declarator ends at its closing bracket; otherwise the // element spelling wraps the name (e.g. an array of function pointers) // and cannot be rewritten by stripping a trailing extent. - if (LBracket.isInvalid() || RBracket.isInvalid() || - ForArray->getEndLoc() != RBracket) - return false; + if (ATL.getEndLoc() != RBracket) + return ReportReason::ArrayNotEndInBracket; + if (LBracket.isInvalid() || RBracket.isInvalid()) + return ReportReason::EmissionFailed; Edited.emplace_back(SM, CharSourceRange::getTokenRange(LBracket, RBracket), "", Ctx.getLangOpts()); } - for (const tooling::Replacement &Repl : Edited) - if (!Repl.isApplicable()) - return false; + if (!llvm::all_of(Edited, std::mem_fn(&tooling::Replacement::isApplicable))) + return ReportReason::EmissionFailed; for (tooling::Replacement &Repl : Edited) Edits.addReplacement(std::move(Repl)); - return true; + return std::nullopt; } - /// Marks \p Cand accounted for, reporting \p Reason if one is given. - bool account(Candidate &Cand, const DeclaratorDecl *D, - std::optional<ReportReason> Reason) { - Cand.AccountedFor = true; + /// Reports \p Reason for \p D, if one is given. Always returns true so that + /// visitors can tail-call it. + bool report(const DeclaratorDecl *D, std::optional<ReportReason> Reason) { if (Reason) Report.addResult(SkippedRuleId, SarifResultLevel::Note, declTypeRange(D), messageFor(*Reason)); @@ -349,28 +479,39 @@ namespace clang::ssaf { llvm::StringRef messageFor(ReportReason Reason) { switch (Reason) { + case ReportReason::ArrayNotEndInBracket: + return "the array type does not end in a closing bracket"; + case ReportReason::DeclarationGroup: + return "declarator of a multi-declarator group is not yet rewritten"; + case ReportReason::EmissionFailed: + return "no source edit could be formed for this declarator"; + case ReportReason::IncompleteArray: + return "array of unknown bound is not yet rewritten"; + case ReportReason::MacroExpansion: + return "declarator spelled through a macro is not yet rewritten"; + case ReportReason::MultiDimensionalArray: + return "multi-dimensional array is not yet rewritten"; case ReportReason::MultiLevelPointer: return "multi-level pointer indirection is not yet rewritten"; + case ReportReason::NoInnerTypeLoc: + return "no TypeLoc for the pointee or array element type"; + case ReportReason::NotPointerTypeEndWithStar: + return "pointer declarator does not end at its '*'"; + case ReportReason::NotTransformed: + return "this declaration was not transformed"; case ReportReason::PointerToArray: return "pointer to array is not yet rewritten"; case ReportReason::ReferenceToPointer: return "reference to pointer is not yet rewritten"; - case ReportReason::MultiDimensionalArray: - return "multi-dimensional array is not yet rewritten"; - case ReportReason::IncompleteArray: - return "array of unknown bound is not yet rewritten"; - case ReportReason::UnreproducibleType: - return "type spelling cannot be reproduced"; - case ReportReason::DeclarationGroup: - return "declarator of a multi-declarator group is not yet rewritten"; - case ReportReason::MacroExpansion: - return "declarator spelled through a macro is not yet rewritten"; case ReportReason::TrailingReturnType: return "trailing return type is not yet rewritten"; - case ReportReason::EmissionFailed: - return "no source edit could be formed for this declarator"; - case ReportReason::NotTransformed: - return "reachable buffer was not transformed"; + case ReportReason::UnexpectedLeadingQualifier: + return "unexpected token between a leading cv-qualifier and the type"; + case ReportReason::UnexpectedTrailingQualifier: + return "unexpected token between the type and a trailing cv-qualifier"; + case ReportReason::UnnamableType: + return "the pointee or array element type has no name that can be written " + "as a template argument"; } llvm_unreachable("unhandled ReportReason"); } @@ -411,12 +552,13 @@ classifyDeclType(QualType T, const llvm::SmallSet<unsigned, 4> &ReachableLevels, R.Skip = ReportReason::PointerToArray; return R; } - if (!isReproducible(Pointee)) { - R.Skip = ReportReason::UnreproducibleType; + if (!isNamable(Pointee)) { + R.Skip = ReportReason::UnnamableType; return R; } R.NewType = BoundedType::Ptr; R.InnerSpelling = Pointee->isVoidType() ? "char" : spell(Pointee, Ctx); + R.Skip = std::nullopt; return R; } @@ -426,12 +568,13 @@ classifyDeclType(QualType T, const llvm::SmallSet<unsigned, 4> &ReachableLevels, R.Skip = ReportReason::MultiDimensionalArray; return R; } - if (!isReproducible(Element)) { - R.Skip = ReportReason::UnreproducibleType; + if (!isNamable(Element)) { + R.Skip = ReportReason::UnnamableType; return R; } R.NewType = BoundedType::Array; R.InnerSpelling = spell(Element, Ctx); + R.Skip = std::nullopt; return R; } @@ -454,20 +597,6 @@ void CppBoundedBuffers::HandleTranslationUnit(ASTContext &Ctx) { Decl *TU = Ctx.getTranslationUnitDecl(); CollectVisitor(Reach, Decls, Returns).TraverseDecl(TU); RewriteVisitor(Ctx, Decls, Returns, Edits, Report).TraverseDecl(TU); - - // Every reachable buffer in this TU is either rewritten or reported; a - // leftover means it was neither, which must still be surfaced. - for (const auto &[D, Cand] : Decls) - if (!Cand.AccountedFor) - Report.addResult(SkippedRuleId, SarifResultLevel::Note, - declTypeRange(cast<DeclaratorDecl>(D)), - messageFor(ReportReason::NotTransformed)); - for (const auto &[FD, Cand] : Returns) - if (!Cand.AccountedFor) - Report.addResult( - SkippedRuleId, SarifResultLevel::Note, - CharSourceRange::getTokenRange(FD->getReturnTypeSourceRange()), - messageFor(ReportReason::NotTransformed)); } } // namespace clang::ssaf diff --git a/clang/unittests/ScalableStaticAnalysis/SourceTransformation/CppBoundedBuffersTest.cpp b/clang/unittests/ScalableStaticAnalysis/SourceTransformation/CppBoundedBuffersTest.cpp index ca6e5e3894e70..2bc5dfda48852 100644 --- a/clang/unittests/ScalableStaticAnalysis/SourceTransformation/CppBoundedBuffersTest.cpp +++ b/clang/unittests/ScalableStaticAnalysis/SourceTransformation/CppBoundedBuffersTest.cpp @@ -25,6 +25,7 @@ #include "clang/Tooling/Tooling.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallSet.h" +#include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" #include "gtest/gtest.h" #include <memory> @@ -107,7 +108,8 @@ class CppBoundedBuffersTest : public TestFixture { // Parses \p Code, lets \p Mark populate the reachable result, runs the // transformation, and returns the rewritten source and report entries. Captured runMarked(StringRef Code, MarkFn Mark) { - std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCode(Code); + std::unique_ptr<ASTUnit> AST = + tooling::buildASTFromCodeWithArgs(Code, {"-std=c++20"}); ASTContext &Ctx = AST->getASTContext(); WPASuite Suite = makeWPASuite(); @@ -142,7 +144,7 @@ class CppBoundedBuffersTest : public TestFixture { TEST_F(CppBoundedBuffersTest, PointerLocal) { Captured C = run("void f() { int *p; }", [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); - EXPECT_EQ(C.Rewritten, "void f() { bounded_ptr<int> p; }"); + ASSERT_TRUE(C.Rewritten == "void f() { bounded_ptr<int> p; }"); EXPECT_TRUE(C.Reports.empty()); } @@ -150,28 +152,28 @@ TEST_F(CppBoundedBuffersTest, PointerParameter) { Captured C = run("void f(int *p);", [](ASTContext &Ctx) { return paramEntity("f", 0, Ctx); }, {1}); - EXPECT_EQ(C.Rewritten, "void f(bounded_ptr<int> p);"); + ASSERT_TRUE(C.Rewritten == "void f(bounded_ptr<int> p);"); EXPECT_TRUE(C.Reports.empty()); } TEST_F(CppBoundedBuffersTest, ConstQualifiedPointee) { Captured C = run("const char *s;", [](ASTContext &Ctx) { return varEntity("s", Ctx); }, {1}); - EXPECT_EQ(C.Rewritten, "bounded_ptr<const char> s;"); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<const char> s;"); EXPECT_TRUE(C.Reports.empty()); } TEST_F(CppBoundedBuffersTest, VoidPointer) { Captured C = run("void *p;", [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); - EXPECT_EQ(C.Rewritten, "bounded_ptr<char> p;"); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<char> p;"); EXPECT_TRUE(C.Reports.empty()); } TEST_F(CppBoundedBuffersTest, ArrayField) { Captured C = run("struct S { int a[10]; };", [](ASTContext &Ctx) { return fieldEntity("a", Ctx); }, {1}); - EXPECT_EQ(C.Rewritten, "struct S { bounded_array<int, 10> a; };"); + ASSERT_TRUE(C.Rewritten == "struct S { bounded_array<int, 10> a; };"); EXPECT_TRUE(C.Reports.empty()); } @@ -179,28 +181,28 @@ TEST_F(CppBoundedBuffersTest, FunctionReturn) { Captured C = run("int *foo();", [](ASTContext &Ctx) { return returnEntity("foo", Ctx); }, {1}); - EXPECT_EQ(C.Rewritten, "bounded_ptr<int> foo();"); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<int> foo();"); EXPECT_TRUE(C.Reports.empty()); } TEST_F(CppBoundedBuffersTest, GlobalPointer) { Captured C = run("int *g;", [](ASTContext &Ctx) { return varEntity("g", Ctx); }, {1}); - EXPECT_EQ(C.Rewritten, "bounded_ptr<int> g;"); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<int> g;"); EXPECT_TRUE(C.Reports.empty()); } TEST_F(CppBoundedBuffersTest, PointerField) { Captured C = run("struct S { int *p; };", [](ASTContext &Ctx) { return fieldEntity("p", Ctx); }, {1}); - EXPECT_EQ(C.Rewritten, "struct S { bounded_ptr<int> p; };"); + ASSERT_TRUE(C.Rewritten == "struct S { bounded_ptr<int> p; };"); EXPECT_TRUE(C.Reports.empty()); } TEST_F(CppBoundedBuffersTest, ArrayOfPointers) { Captured C = run("int *a[10];", [](ASTContext &Ctx) { return varEntity("a", Ctx); }, {1}); - EXPECT_EQ(C.Rewritten, "bounded_array<int *, 10> a;"); + ASSERT_TRUE(C.Rewritten == "bounded_array<int *, 10> a;"); EXPECT_TRUE(C.Reports.empty()); } @@ -209,7 +211,144 @@ TEST_F(CppBoundedBuffersTest, ArrayOfFunctionPointers) { // pointer); the typedef keeps the declarator a clean prefix + [N] suffix. Captured C = run("typedef void (*FP)(); FP fps[4];", [](ASTContext &Ctx) { return varEntity("fps", Ctx); }, {1}); - EXPECT_EQ(C.Rewritten, "typedef void (*FP)(); bounded_array<FP, 4> fps;"); + ASSERT_TRUE(C.Rewritten == "typedef void (*FP)(); bounded_array<FP, 4> fps;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, ConstQualifiedPointeeSpelledAfter) { + // `char const *` means the same as `const char *`; the qualifier belongs to + // the pointee either way and is reproduced inside the angle brackets. + Captured C = run("char const *s;", + [](ASTContext &Ctx) { return varEntity("s", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<const char> s;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, ConstVolatileQualifiedPointee) { + Captured C = run("const volatile char *s;", + [](ASTContext &Ctx) { return varEntity("s", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<const volatile char> s;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, ConstPointerKeepsItsOwnQualifier) { + // The `const` applies to the pointer, not the pointee, so it lies outside the + // rewrite range and stays where it was written. + Captured C = run("int *const p = nullptr;", + [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<int> const p = nullptr;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, ConstPointerToConstPointee) { + Captured C = run("const int *const p = nullptr;", + [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<const int> const p = nullptr;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, StorageClassBeforeQualifiedPointee) { + // `static` precedes the qualifier run and is left untouched. + Captured C = run("static const char *s;", + [](ASTContext &Ctx) { return varEntity("s", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "static bounded_ptr<const char> s;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, ConstQualifiedArrayElement) { + Captured C = run("const int a[10] = {};", + [](ASTContext &Ctx) { return varEntity("a", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "bounded_array<const int, 10> a = {};"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, ConstQualifiedArrayElementSpelledAfter) { + Captured C = run("int const a[10] = {};", + [](ASTContext &Ctx) { return varEntity("a", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "bounded_array<const int, 10> a = {};"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, QualifiedPointerFunctionReturn) { + Captured C = + run("const char *foo();", + [](ASTContext &Ctx) { return returnEntity("foo", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<const char> foo();"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, MultipleTrailingPointerQualifiers) { + Captured C = run("int *volatile const p = nullptr;", + [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<int> volatile const p = nullptr;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, LeadingAndMultipleTrailingPointerQualifiers) { + Captured C = run("const int *const volatile p = nullptr;", + [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == + "bounded_ptr<const int> const volatile p = nullptr;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, MultipleTrailingArrayQualifiers) { + // Both qualify the element, so the range grows right over the whole run. + Captured C = run("int const volatile a[10] = {};", + [](ASTContext &Ctx) { return varEntity("a", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "bounded_array<const volatile int, 10> a = {};"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, MultipleTrailingArrayQualifiersReversed) { + Captured C = run("int volatile const a[10] = {};", + [](ASTContext &Ctx) { return varEntity("a", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "bounded_array<const volatile int, 10> a = {};"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, CommentBetweenPointeeTypeAndStar) { + Captured C = run("int /* c */ *p;", + [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<int> p;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, AttributeBetweenPointeeTypeAndStar) { + // A type attribute also sits inside the rewrite range. It is part of the + // pointee type, so the pretty-printed spelling reproduces it. + Captured C = run("int __attribute__((address_space(1))) *p;", + [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, + "bounded_ptr<__attribute__((address_space(1))) int> p;"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, DeclAttributeAfterArrayBrackets) { + // A declaration attribute is not part of the type-loc, so it lies beyond the + // deleted extent and survives untouched. + Captured C = run("int a[10] __attribute__((aligned(16)));", + [](ASTContext &Ctx) { return varEntity("a", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, + "bounded_array<int, 10> a __attribute__((aligned(16)));"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, DeclAttributeAfterPointerDeclarator) { + Captured C = run("int *p __attribute__((aligned(16)));", + [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + ASSERT_TRUE(C.Rewritten == + "bounded_ptr<int> p __attribute__((aligned(16)));"); + EXPECT_TRUE(C.Reports.empty()); +} + +TEST_F(CppBoundedBuffersTest, AliasedLambdaPointee) { + // The closure type itself is unnamed, but the alias supplies a name that can + // be written as the template argument, and it denotes that same closure type. + Captured C = run("using L = decltype([](int x) { return x; });\nL *p;\n", + [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + EXPECT_EQ(C.Rewritten, "using L = decltype([](int x) { return x; });\n" + "bounded_ptr<L> p;\n"); EXPECT_TRUE(C.Reports.empty()); } @@ -218,10 +357,10 @@ TEST_F(CppBoundedBuffersTest, ArrayOfFunctionPointers) { //===----------------------------------------------------------------------===// void expectSkip(const Captured &C, StringRef Original, ReportReason Reason) { - EXPECT_EQ(C.Rewritten, Original); - ASSERT_EQ(C.Reports.size(), 1u); - EXPECT_EQ(C.Reports[0].Level, SarifResultLevel::Note); - EXPECT_EQ(C.Reports[0].Message, messageFor(Reason).str()); + ASSERT_TRUE(C.Rewritten == Original); + ASSERT_TRUE(C.Reports.size() == 1u); + ASSERT_TRUE(C.Reports[0].Level == SarifResultLevel::Note); + ASSERT_TRUE(C.Reports[0].Message == messageFor(Reason).str()); } TEST_F(CppBoundedBuffersTest, MultiLevelPointer) { @@ -259,11 +398,30 @@ TEST_F(CppBoundedBuffersTest, ReferenceToPointer) { expectSkip(C, Code, ReportReason::ReferenceToPointer); } -TEST_F(CppBoundedBuffersTest, UnreproducibleType) { +TEST_F(CppBoundedBuffersTest, UnnamableType) { StringRef Code = "struct { int x; } *p;"; Captured C = run(Code, [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); - expectSkip(C, Code, ReportReason::UnreproducibleType); + expectSkip(C, Code, ReportReason::UnnamableType); +} + +TEST_F(CppBoundedBuffersTest, InlineLambdaPointee) { + // Each lambda-expression yields a distinct closure type, so there is no name + // to write: re-spelling the expression would denote a different type. + StringRef Code = "decltype([](int x) { return x; }) *p;"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::UnnamableType); +} + +TEST_F(CppBoundedBuffersTest, UnaliasedLambdaPointee) { + // decltype of a variable names the closure type but is not a typedef-name, so + // the unnamed record is what the check sees. + StringRef Code = + "void f() { auto lam = [](int x) { return x; }; decltype(lam) *p; }"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::UnnamableType); } TEST_F(CppBoundedBuffersTest, DeclarationGroup) { @@ -275,11 +433,70 @@ TEST_F(CppBoundedBuffersTest, DeclarationGroup) { markReachable(Suite, Result, varEntity("p", Ctx), {1}); markReachable(Suite, Result, varEntity("q", Ctx), {1}); }); - EXPECT_EQ(C.Rewritten, "int *p, *q;"); - ASSERT_EQ(C.Reports.size(), 2u); + ASSERT_TRUE(C.Rewritten == "int *p, *q;"); + ASSERT_TRUE(C.Reports.size() == 2u); + for (const auto &R : C.Reports) { + ASSERT_TRUE(R.Level == SarifResultLevel::Note); + ASSERT_TRUE(R.Message == messageFor(ReportReason::DeclarationGroup).str()); + } +} + +TEST_F(CppBoundedBuffersTest, GlobalDeclarationGroupOfThree) { + // A three-way (not just two-way) comma group at namespace scope; every + // declarator, including the multi-level pointer, is reported. + StringRef Code = "extern int *const p, *const q, *volatile *pp;"; + Captured C = runMarked(Code, [](ASTContext &Ctx, WPASuite &Suite, + UnsafeBufferReachableAnalysisResult &Result) { + markReachable(Suite, Result, varEntity("p", Ctx), {1}); + markReachable(Suite, Result, varEntity("q", Ctx), {1}); + markReachable(Suite, Result, varEntity("pp", Ctx), {1, 2}); + }); + ASSERT_TRUE(C.Rewritten == Code); + ASSERT_TRUE(C.Reports.size() == 3u); for (const auto &R : C.Reports) { - EXPECT_EQ(R.Level, SarifResultLevel::Note); - EXPECT_EQ(R.Message, messageFor(ReportReason::DeclarationGroup).str()); + ASSERT_TRUE(R.Level == SarifResultLevel::Note); + ASSERT_TRUE(R.Message == messageFor(ReportReason::DeclarationGroup).str()); + } +} + +TEST_F(CppBoundedBuffersTest, FieldDeclarationGroupOfThree) { + // Same comma group, but as FieldDecls inside a RecordDecl rather than + // VarDecls inside the TranslationUnitDecl. + StringRef Code = "struct Tup { int *const p, *const q, *volatile *pp; };"; + Captured C = runMarked(Code, [](ASTContext &Ctx, WPASuite &Suite, + UnsafeBufferReachableAnalysisResult &Result) { + markReachable(Suite, Result, fieldEntity("p", Ctx), {1}); + markReachable(Suite, Result, fieldEntity("q", Ctx), {1}); + markReachable(Suite, Result, fieldEntity("pp", Ctx), {1, 2}); + }); + ASSERT_TRUE(C.Rewritten == Code); + ASSERT_TRUE(C.Reports.size() == 3u); + for (const auto &R : C.Reports) { + ASSERT_TRUE(R.Level == SarifResultLevel::Note); + ASSERT_TRUE(R.Message == messageFor(ReportReason::DeclarationGroup).str()); + } +} + +TEST_F(CppBoundedBuffersTest, ForInitDeclarationGroupOfThree) { + // Same comma group again, but as a DeclStmt in a for-loop init-statement; + // the lexical DeclContext is the enclosing function, not the loop itself. + StringRef Code = "void test() {\n" + " for (int *const p = {}, *const q = {}, " + "*volatile *pp = {}; true;) {\n" + " return;\n" + " }\n" + "}\n"; + Captured C = runMarked(Code, [](ASTContext &Ctx, WPASuite &Suite, + UnsafeBufferReachableAnalysisResult &Result) { + markReachable(Suite, Result, varEntity("p", Ctx), {1}); + markReachable(Suite, Result, varEntity("q", Ctx), {1}); + markReachable(Suite, Result, varEntity("pp", Ctx), {1, 2}); + }); + ASSERT_TRUE(C.Rewritten == Code); + ASSERT_TRUE(C.Reports.size() == 3u); + for (const auto &R : C.Reports) { + ASSERT_TRUE(R.Level == SarifResultLevel::Note); + ASSERT_TRUE(R.Message == messageFor(ReportReason::DeclarationGroup).str()); } } @@ -297,13 +514,61 @@ TEST_F(CppBoundedBuffersTest, TrailingReturnType) { expectSkip(C, Code, ReportReason::TrailingReturnType); } -TEST_F(CppBoundedBuffersTest, EmissionFailureOnRawFunctionPointerArray) { - // A raw array-of-function-pointers has no clean prefix + [N] suffix, so the - // edit cannot be formed and the entity is reported rather than mangled. +TEST_F(CppBoundedBuffersTest, RawFunctionPointerArrayDoesNotEndInBracket) { + // A raw array-of-function-pointers has no clean prefix + [N] suffix: the + // element spelling wraps the name, so the array type ends at the trailing + // `()` rather than at its closing bracket. StringRef Code = "void (*fps[4])();"; Captured C = run(Code, [](ASTContext &Ctx) { return varEntity("fps", Ctx); }, {1}); - expectSkip(C, Code, ReportReason::EmissionFailed); + expectSkip(C, Code, ReportReason::ArrayNotEndInBracket); +} + +TEST_F(CppBoundedBuffersTest, ParenthesizedPointerDeclarator) { + // The parens wrap the declarator, so the type ends at the ')' rather than at + // the '*'. A range anchored on the type would span the name and unbalance the + // parens, so the declarator is reported instead. + StringRef Code = "int (*par);"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("par", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::NotPointerTypeEndWithStar); +} + +TEST_F(CppBoundedBuffersTest, TypedefSpelledPointer) { + // The declarator spells no pointer of its own, so there is no pointee + // type-loc to build a rewrite range from. + StringRef Code = "typedef int *IP;\nIP p;\n"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::NoInnerTypeLoc); +} + +TEST_F(CppBoundedBuffersTest, QualifierSeparatedFromPointeeType) { + // `const` is separated from the type by `static`, so absorbing it into the + // rewrite range would need a non-contiguous edit. + StringRef Code = "const static char *s;"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("s", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::UnexpectedLeadingQualifier); +} + +TEST_F(CppBoundedBuffersTest, CommentBetweenQualifierAndPointeeType) { + // A comment interrupts the qualifier run; absorbing the `const` would delete + // the comment along with it. + StringRef Code = "const /* c */ char *s;"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("s", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::UnexpectedLeadingQualifier); +} + +TEST_F(CppBoundedBuffersTest, CommentBetweenElementTypeAndTrailingQualifier) { + // The `const` qualifies the element, so its meaning moves inside the bounded + // type and it must be absorbed by the rewrite range. The comment separates it + // from the element type, which would need a non-contiguous edit. + StringRef Code = "int /* c */ const a[10] = {};"; + Captured C = + run(Code, [](ASTContext &Ctx) { return varEntity("a", Ctx); }, {1}); + expectSkip(C, Code, ReportReason::UnexpectedTrailingQualifier); } //===----------------------------------------------------------------------===// @@ -323,7 +588,7 @@ TEST_F(CppBoundedBuffersTest, NotReachable) { StringRef Code = "int *p;"; Captured C = run(Code, [](ASTContext &Ctx) { return varEntity("p", Ctx); }, {}); - EXPECT_EQ(C.Rewritten, Code); + ASSERT_TRUE(C.Rewritten == Code); EXPECT_TRUE(C.Reports.empty()); } @@ -336,8 +601,8 @@ TEST_F(CppBoundedBuffersTest, RewriteAndReportCoexist) { markReachable(Suite, Result, varEntity("good", Ctx), {1}); markReachable(Suite, Result, varEntity("bad", Ctx), {1}); }); - EXPECT_EQ(C.Rewritten, "bounded_ptr<int> good; int **bad;"); - ASSERT_EQ(C.Reports.size(), 1u); + ASSERT_TRUE(C.Rewritten == "bounded_ptr<int> good; int **bad;"); + ASSERT_TRUE(C.Reports.size() == 1u); EXPECT_EQ(C.Reports[0].Message, messageFor(ReportReason::MultiLevelPointer).str()); } @@ -356,20 +621,21 @@ TEST_F(CppBoundedBuffersTest, ClassifyRewritesOutermostReachablePointer) { Levels.insert(1); ClassifyResult R = classifyDeclType(typeOf("p", AST->getASTContext()), Levels, AST->getASTContext()); - ASSERT_TRUE(R.NewType.has_value()); - EXPECT_EQ(*R.NewType, BoundedType::Ptr); - EXPECT_EQ(R.InnerSpelling, "int"); - EXPECT_FALSE(R.Skip.has_value()); + ASSERT_FALSE(R.Skip.has_value()); + ASSERT_TRUE(R.NewType == BoundedType::Ptr); + ASSERT_TRUE(R.InnerSpelling == "int"); } TEST_F(CppBoundedBuffersTest, ClassifyIgnoresInnerOnlyReachablePointer) { + // A single pointer has only level 1, so nothing is recognized and the + // catch-all reason is reported rather than leaving the entity undecided. auto AST = tooling::buildASTFromCode("int *p;"); llvm::SmallSet<unsigned, 4> Levels; Levels.insert(2); ClassifyResult R = classifyDeclType(typeOf("p", AST->getASTContext()), Levels, AST->getASTContext()); - EXPECT_FALSE(R.NewType.has_value()); - EXPECT_FALSE(R.Skip.has_value()); + ASSERT_TRUE(R.Skip.has_value()); + ASSERT_TRUE(*R.Skip == ReportReason::NotTransformed); } TEST_F(CppBoundedBuffersTest, ClassifyMultiLevelPointerIsSkipped) { @@ -378,20 +644,29 @@ TEST_F(CppBoundedBuffersTest, ClassifyMultiLevelPointerIsSkipped) { Levels.insert(1); ClassifyResult R = classifyDeclType(typeOf("pp", AST->getASTContext()), Levels, AST->getASTContext()); - EXPECT_FALSE(R.NewType.has_value()); ASSERT_TRUE(R.Skip.has_value()); - EXPECT_EQ(*R.Skip, ReportReason::MultiLevelPointer); + ASSERT_TRUE(*R.Skip == ReportReason::MultiLevelPointer); } TEST_F(CppBoundedBuffersTest, MessageForIsNonEmpty) { - for (ReportReason Reason : - {ReportReason::MultiLevelPointer, ReportReason::PointerToArray, - ReportReason::ReferenceToPointer, ReportReason::MultiDimensionalArray, - ReportReason::IncompleteArray, ReportReason::UnreproducibleType, - ReportReason::DeclarationGroup, ReportReason::MacroExpansion, - ReportReason::TrailingReturnType, ReportReason::EmissionFailed, - ReportReason::NotTransformed}) + // Walks the whole enum rather than a hand-kept list, so a reason added + // without a message is caught here and not silently left untested. + for (unsigned I = 0; I <= static_cast<unsigned>(ReportReason::UnnamableType); + ++I) { + auto Reason = static_cast<ReportReason>(I); EXPECT_FALSE(messageFor(Reason).empty()); + } +} + +TEST_F(CppBoundedBuffersTest, MessageForIsUnique) { + // Two reasons sharing a message would make reports ambiguous. + llvm::StringSet<> Seen; + for (unsigned I = 0; I <= static_cast<unsigned>(ReportReason::UnnamableType); + ++I) { + auto Reason = static_cast<ReportReason>(I); + EXPECT_TRUE(Seen.insert(messageFor(Reason)).second) + << "duplicate message: " << messageFor(Reason).str(); + } } } // namespace _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
