llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-ssaf Author: Jan Korous (jkorous-apple) <details> <summary>Changes</summary> A transformation has to find the entity the whole-program analysis result has for each declaration it looks at. cpp-bounded-buffers did that itself: it indexed the WPASuite's entity names, and qualified each declaration's name the way the entity linker does, with the namespace of its link unit and, unless it has external linkage, of its compilation unit. Move that into EntityLookup, in the SourceTransformation library, so that any transformation can use it: EntityLookup Lookup(Suite, Opts); std::optional<EntityId> Id = Lookup.getEntityId(D); getEntityIdForReturn does the same for the return value of a function. cpp-bounded-buffers now uses EntityLookup, and behaves as before. Assisted-by: Claude --- Full diff: https://github.com/llvm/llvm-project/pull/235179.diff 8 Files Affected: - (added) clang/include/clang/ScalableStaticAnalysis/SourceTransformation/EntityLookup.h (+58) - (modified) clang/lib/ScalableStaticAnalysis/SourceTransformation/CMakeLists.txt (+1) - (added) clang/lib/ScalableStaticAnalysis/SourceTransformation/EntityLookup.cpp (+44) - (modified) clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp (+27-55) - (modified) clang/unittests/ScalableStaticAnalysis/CMakeLists.txt (+1) - (added) clang/unittests/ScalableStaticAnalysis/SourceTransformation/EntityLookupTest.cpp (+145) - (modified) llvm/utils/gn/secondary/clang/lib/ScalableStaticAnalysis/SourceTransformation/BUILD.gn (+1) - (modified) llvm/utils/gn/secondary/clang/unittests/ScalableStaticAnalysis/BUILD.gn (+1) ``````````diff diff --git a/clang/include/clang/ScalableStaticAnalysis/SourceTransformation/EntityLookup.h b/clang/include/clang/ScalableStaticAnalysis/SourceTransformation/EntityLookup.h new file mode 100644 index 00000000000000..f1553cf7afb127 --- /dev/null +++ b/clang/include/clang/ScalableStaticAnalysis/SourceTransformation/EntityLookup.h @@ -0,0 +1,58 @@ +//===- EntityLookup.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 +// +//===----------------------------------------------------------------------===// +// +// Maps the declarations of the translation unit a transformation runs on to +// the entities of the whole-program analysis result. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_SCALABLESTATICANALYSIS_SOURCETRANSFORMATION_ENTITYLOOKUP_H +#define LLVM_CLANG_SCALABLESTATICANALYSIS_SOURCETRANSFORMATION_ENTITYLOOKUP_H + +#include "clang/ScalableStaticAnalysis/Core/Model/BuildNamespace.h" +#include "clang/ScalableStaticAnalysis/Core/Model/EntityId.h" +#include "clang/ScalableStaticAnalysis/Core/Model/EntityName.h" +#include <map> +#include <optional> + +namespace clang { +class Decl; +class FunctionDecl; +} // namespace clang + +namespace clang::ssaf { + +class SSAFOptions; +class WPASuite; + +/// Looks up the EntityId that a WPASuite gives a declaration of the +/// translation unit identified by SSAFOptions::CompilationUnitId, linked into +/// SSAFOptions::LinkUnitId. +class EntityLookup { +public: + EntityLookup(const WPASuite &Suite, const SSAFOptions &Opts); + + /// Returns the EntityId of \p D, or std::nullopt if \p D is not an entity or + /// the suite does not contain it. + std::optional<EntityId> getEntityId(const Decl *D) const; + + /// Returns the EntityId of the return value of \p FD, or std::nullopt if the + /// return value is not an entity or the suite does not contain it. + std::optional<EntityId> getEntityIdForReturn(const FunctionDecl *FD) const; + +private: + std::optional<EntityId> lookup(const std::optional<EntityName> &Name) const; + + std::map<EntityName, EntityId> Ids; + NestedBuildNamespace TUNamespace; + NestedBuildNamespace LUNamespace; +}; + +} // namespace clang::ssaf + +#endif // LLVM_CLANG_SCALABLESTATICANALYSIS_SOURCETRANSFORMATION_ENTITYLOOKUP_H diff --git a/clang/lib/ScalableStaticAnalysis/SourceTransformation/CMakeLists.txt b/clang/lib/ScalableStaticAnalysis/SourceTransformation/CMakeLists.txt index 96e85ec3a50a8d..7b110004d89a5b 100644 --- a/clang/lib/ScalableStaticAnalysis/SourceTransformation/CMakeLists.txt +++ b/clang/lib/ScalableStaticAnalysis/SourceTransformation/CMakeLists.txt @@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS ) add_clang_library(clangScalableStaticAnalysisSourceTransformation + EntityLookup.cpp SARIFTransformationReportFormat.cpp TransformationRegistry.cpp Transformations/CppBoundedBuffers.cpp diff --git a/clang/lib/ScalableStaticAnalysis/SourceTransformation/EntityLookup.cpp b/clang/lib/ScalableStaticAnalysis/SourceTransformation/EntityLookup.cpp new file mode 100644 index 00000000000000..12a087d13db19e --- /dev/null +++ b/clang/lib/ScalableStaticAnalysis/SourceTransformation/EntityLookup.cpp @@ -0,0 +1,44 @@ +//===- EntityLookup.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/EntityLookup.h" +#include "clang/AST/Decl.h" +#include "clang/Frontend/SSAFOptions.h" +#include "clang/ScalableStaticAnalysis/Core/ASTEntityMapping.h" +#include "clang/ScalableStaticAnalysis/Core/Model/EntityIdTable.h" +#include "clang/ScalableStaticAnalysis/Core/WholeProgramAnalysis/WPASuite.h" + +using namespace clang; +using namespace clang::ssaf; + +EntityLookup::EntityLookup(const WPASuite &Suite, const SSAFOptions &Opts) + : TUNamespace( + NestedBuildNamespace::makeCompilationUnit(Opts.CompilationUnitId)), + LUNamespace(NestedBuildNamespace::makeLinkUnit(Opts.LinkUnitId)) { + Suite.getIdTable().forEach( + [this](const EntityName &Name, EntityId Id) { Ids.emplace(Name, Id); }); +} + +std::optional<EntityId> EntityLookup::getEntityId(const Decl *D) const { + return lookup(getQualifiedEntityName(D, TUNamespace, LUNamespace)); +} + +std::optional<EntityId> +EntityLookup::getEntityIdForReturn(const FunctionDecl *FD) const { + return lookup(getQualifiedEntityNameForReturn(FD, TUNamespace, LUNamespace)); +} + +std::optional<EntityId> +EntityLookup::lookup(const std::optional<EntityName> &Name) const { + if (!Name) + return std::nullopt; + auto It = Ids.find(*Name); + if (It == Ids.end()) + return std::nullopt; + return It->second; +} diff --git a/clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp b/clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp index c119ee2f01c5ea..78d6985dfa20e6 100644 --- a/clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp +++ b/clang/lib/ScalableStaticAnalysis/SourceTransformation/Transformations/CppBoundedBuffers.cpp @@ -17,14 +17,11 @@ #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" -#include "clang/Frontend/SSAFOptions.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/EntityLookup.h" #include "clang/ScalableStaticAnalysis/SourceTransformation/TransformationRegistry.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/STLExtras.h" @@ -209,62 +206,30 @@ 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 { - const EntityPointerLevelSet &Reachables; - std::map<EntityName, EntityId> NameToId; - -public: - ReachabilityMap(const WPASuite &Suite, - const 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 [Begin, End] = Reachables.equal_range(NameIt->second); - for (const EntityPointerLevel &EPL : llvm::make_range(Begin, End)) - Levels.insert(EPL.getPointerLevel()); - return Levels; - } -}; - /// Collects the reachable pointer/array declarators and function returns /// declared in this TU. class CollectVisitor : public DynamicRecursiveASTVisitor { public: - CollectVisitor(const ReachabilityMap &Reach, - const NestedBuildNamespace &TUNamespace, - const NestedBuildNamespace &LUNamespace, DeclLevels &Decls, + CollectVisitor(const EntityLookup &Lookup, + const EntityPointerLevelSet &Reachables, DeclLevels &Decls, ReturnLevels &Returns) - : Reach(Reach), TUNamespace(TUNamespace), LUNamespace(LUNamespace), - Decls(Decls), Returns(Returns) {} + : Lookup(Lookup), Reachables(Reachables), Decls(Decls), Returns(Returns) { + } bool VisitVarDecl(VarDecl *D) override { - collect(D, D->getType(), - getQualifiedEntityName(D, TUNamespace, LUNamespace)); + collect(D, D->getType()); return true; } bool VisitFieldDecl(FieldDecl *D) override { - collect(D, D->getType(), - getQualifiedEntityName(D, TUNamespace, LUNamespace)); + collect(D, D->getType()); return true; } bool VisitFunctionDecl(FunctionDecl *FD) override { if (!FD->isTemplated() && isCandidateType(FD->getReturnType())) { - llvm::SmallSet<unsigned, 4> Levels = Reach.levelsFor( - getQualifiedEntityNameForReturn(FD, TUNamespace, LUNamespace)); + llvm::SmallSet<unsigned, 4> Levels = + levelsFor(Lookup.getEntityIdForReturn(FD)); if (!Levels.empty()) Returns[FD] = std::move(Levels); } @@ -272,17 +237,28 @@ class CollectVisitor : public DynamicRecursiveASTVisitor { } private: - void collect(const Decl *D, QualType T, std::optional<EntityName> Name) { + void collect(const Decl *D, QualType T) { if (D->isTemplated() || !isCandidateType(T)) return; - llvm::SmallSet<unsigned, 4> Levels = Reach.levelsFor(Name); + llvm::SmallSet<unsigned, 4> Levels = levelsFor(Lookup.getEntityId(D)); if (!Levels.empty()) Decls[D] = std::move(Levels); } - const ReachabilityMap &Reach; - NestedBuildNamespace TUNamespace; - NestedBuildNamespace LUNamespace; + /// The pointer levels at which the whole-program result marks \p Id + /// reachable. + llvm::SmallSet<unsigned, 4> levelsFor(std::optional<EntityId> Id) const { + llvm::SmallSet<unsigned, 4> Levels; + if (!Id) + return Levels; + auto [Begin, End] = Reachables.equal_range(*Id); + for (const EntityPointerLevel &EPL : llvm::make_range(Begin, End)) + Levels.insert(EPL.getPointerLevel()); + return Levels; + } + + const EntityLookup &Lookup; + const EntityPointerLevelSet &Reachables; DeclLevels &Decls; ReturnLevels &Returns; }; @@ -599,16 +575,12 @@ void CppBoundedBuffers::HandleTranslationUnit(ASTContext &Ctx) { return; } - ReachabilityMap Reach(Suite, Reachable->Reachables); - NestedBuildNamespace TUNamespace = - NestedBuildNamespace::makeCompilationUnit(Opts.CompilationUnitId); - NestedBuildNamespace LUNamespace = - NestedBuildNamespace::makeLinkUnit(Opts.LinkUnitId); + EntityLookup Lookup(Suite, Opts); DeclLevels Decls; ReturnLevels Returns; Decl *TU = Ctx.getTranslationUnitDecl(); - CollectVisitor(Reach, TUNamespace, LUNamespace, Decls, Returns) + CollectVisitor(Lookup, Reachable->Reachables, Decls, Returns) .TraverseDecl(TU); RewriteVisitor(Ctx, Decls, Returns, Edits, Report).TraverseDecl(TU); } diff --git a/clang/unittests/ScalableStaticAnalysis/CMakeLists.txt b/clang/unittests/ScalableStaticAnalysis/CMakeLists.txt index a8195cedefd141..4154056a1e60bf 100644 --- a/clang/unittests/ScalableStaticAnalysis/CMakeLists.txt +++ b/clang/unittests/ScalableStaticAnalysis/CMakeLists.txt @@ -33,6 +33,7 @@ add_distinct_clang_unittest(ClangScalableAnalysisTests Serialization/JSONFormatTest/VirtualMethodFamilyFormatTest.cpp SourceTransformation/CppBoundedBuffersTest.cpp SourceTransformation/EmitterTest.cpp + SourceTransformation/EntityLookupTest.cpp SourceTransformation/RegistryTest.cpp SourceTransformation/SARIFFormatTest.cpp SourceTransformation/YAMLFormatTest.cpp diff --git a/clang/unittests/ScalableStaticAnalysis/SourceTransformation/EntityLookupTest.cpp b/clang/unittests/ScalableStaticAnalysis/SourceTransformation/EntityLookupTest.cpp new file mode 100644 index 00000000000000..601e605162ce42 --- /dev/null +++ b/clang/unittests/ScalableStaticAnalysis/SourceTransformation/EntityLookupTest.cpp @@ -0,0 +1,145 @@ +//===- EntityLookupTest.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/EntityLookup.h" +#include "FindDecl.h" +#include "TestFixture.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/Frontend/ASTUnit.h" +#include "clang/Frontend/SSAFOptions.h" +#include "clang/ScalableStaticAnalysis/Core/ASTEntityMapping.h" +#include "clang/ScalableStaticAnalysis/Core/Model/BuildNamespace.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/Tooling/Tooling.h" +#include "gtest/gtest.h" +#include <cassert> +#include <memory> +#include <optional> + +using namespace clang; +using namespace clang::ssaf; + +namespace { + +constexpr llvm::StringLiteral TestCompilationUnitId = "test-cu"; +constexpr llvm::StringLiteral TestLinkUnitId = "test-lu"; + +NestedBuildNamespace testTUNamespace() { + return NestedBuildNamespace::makeCompilationUnit(TestCompilationUnitId); +} + +NestedBuildNamespace testLUNamespace() { + return NestedBuildNamespace::makeLinkUnit(TestLinkUnitId); +} + +SSAFOptions options(StringRef CompilationUnitId = TestCompilationUnitId, + StringRef LinkUnitId = TestLinkUnitId) { + SSAFOptions Opts; + Opts.CompilationUnitId = CompilationUnitId.str(); + Opts.LinkUnitId = LinkUnitId.str(); + return Opts; +} + +class EntityLookupTest : public TestFixture { +protected: + ASTContext &parse(StringRef Code) { + AST = tooling::buildASTFromCode(Code); + return AST->getASTContext(); + } + + EntityId add(std::optional<EntityName> Name) { + assert(Name && "the declaration has no entity name"); + return getIdTable(Suite).getId(*Name); + } + + EntityId addEntityOf(const Decl *D) { + return add(getQualifiedEntityName(D, testTUNamespace(), testLUNamespace())); + } + + EntityId addReturnOf(const FunctionDecl *FD) { + return add(getQualifiedEntityNameForReturn(FD, testTUNamespace(), + testLUNamespace())); + } + + std::unique_ptr<ASTUnit> AST; + WPASuite Suite = makeWPASuite(); +}; + +TEST_F(EntityLookupTest, Declaration) { + ASTContext &Ctx = parse("void f(); void g();"); + const FunctionDecl *F = findFnByName("f", Ctx); + const FunctionDecl *G = findFnByName("g", Ctx); + EntityId FId = addEntityOf(F); + EntityId GId = addEntityOf(G); + + EntityLookup Lookup(Suite, options()); + EXPECT_EQ(Lookup.getEntityId(F), FId); + EXPECT_EQ(Lookup.getEntityId(G), GId); +} + +TEST_F(EntityLookupTest, Parameter) { + ASTContext &Ctx = parse("void f(int *p, int *q);"); + const FunctionDecl *F = findFnByName("f", Ctx); + EntityId QId = addEntityOf(F->getParamDecl(1)); + + EntityLookup Lookup(Suite, options()); + EXPECT_EQ(Lookup.getEntityId(F->getParamDecl(1)), QId); + EXPECT_EQ(Lookup.getEntityId(F->getParamDecl(0)), std::nullopt); +} + +TEST_F(EntityLookupTest, ReturnValue) { + ASTContext &Ctx = parse("int *f();"); + const FunctionDecl *F = findFnByName("f", Ctx); + EntityId FId = addEntityOf(F); + EntityId ReturnId = addReturnOf(F); + ASSERT_NE(FId, ReturnId); + + EntityLookup Lookup(Suite, options()); + EXPECT_EQ(Lookup.getEntityId(F), FId); + EXPECT_EQ(Lookup.getEntityIdForReturn(F), ReturnId); +} + +TEST_F(EntityLookupTest, DeclarationMissingFromSuite) { + ASTContext &Ctx = parse("void f(); void g();"); + const FunctionDecl *F = findFnByName("f", Ctx); + addEntityOf(F); + + EntityLookup Lookup(Suite, options()); + EXPECT_EQ(Lookup.getEntityId(findFnByName("g", Ctx)), std::nullopt); + EXPECT_EQ(Lookup.getEntityIdForReturn(F), std::nullopt); +} + +TEST_F(EntityLookupTest, InternalLinkageIsQualifiedByCompilationUnit) { + ASTContext &Ctx = parse("static void f() {}"); + const FunctionDecl *F = findFnByName("f", Ctx); + EntityId Id = addEntityOf(F); + + EXPECT_EQ(EntityLookup(Suite, options()).getEntityId(F), Id); + EXPECT_EQ(EntityLookup(Suite, options("other-cu")).getEntityId(F), + std::nullopt); + EXPECT_EQ(EntityLookup(Suite, options(TestCompilationUnitId, "other-lu")) + .getEntityId(F), + std::nullopt); +} + +TEST_F(EntityLookupTest, ExternalLinkageIsQualifiedByLinkUnitOnly) { + ASTContext &Ctx = parse("void f();"); + const FunctionDecl *F = findFnByName("f", Ctx); + EntityId Id = addEntityOf(F); + + EXPECT_EQ(EntityLookup(Suite, options("other-cu")).getEntityId(F), Id); + EXPECT_EQ(EntityLookup(Suite, options(TestCompilationUnitId, "other-lu")) + .getEntityId(F), + std::nullopt); +} + +} // namespace diff --git a/llvm/utils/gn/secondary/clang/lib/ScalableStaticAnalysis/SourceTransformation/BUILD.gn b/llvm/utils/gn/secondary/clang/lib/ScalableStaticAnalysis/SourceTransformation/BUILD.gn index e6e1af033db8ad..c08a7467ebd7ee 100644 --- a/llvm/utils/gn/secondary/clang/lib/ScalableStaticAnalysis/SourceTransformation/BUILD.gn +++ b/llvm/utils/gn/secondary/clang/lib/ScalableStaticAnalysis/SourceTransformation/BUILD.gn @@ -9,6 +9,7 @@ static_library("SourceTransformation") { "//llvm/lib/Support", ] sources = [ + "EntityLookup.cpp", "SARIFTransformationReportFormat.cpp", "TransformationRegistry.cpp", "Transformations/CppBoundedBuffers.cpp", diff --git a/llvm/utils/gn/secondary/clang/unittests/ScalableStaticAnalysis/BUILD.gn b/llvm/utils/gn/secondary/clang/unittests/ScalableStaticAnalysis/BUILD.gn index a4ef763fc341e5..5e7885ed939caa 100644 --- a/llvm/utils/gn/secondary/clang/unittests/ScalableStaticAnalysis/BUILD.gn +++ b/llvm/utils/gn/secondary/clang/unittests/ScalableStaticAnalysis/BUILD.gn @@ -54,6 +54,7 @@ unittest("ClangScalableAnalysisTests") { "Serialization/JSONFormatTest/VirtualMethodFamilyFormatTest.cpp", "SourceTransformation/CppBoundedBuffersTest.cpp", "SourceTransformation/EmitterTest.cpp", + "SourceTransformation/EntityLookupTest.cpp", "SourceTransformation/RegistryTest.cpp", "SourceTransformation/SARIFFormatTest.cpp", "SourceTransformation/YAMLFormatTest.cpp", `````````` </details> https://github.com/llvm/llvm-project/pull/235179 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
