[llvm-branch-commits] [clang] [SSAF][UnsafeBufferUsage] Implement AST visitor that respects the contribution model (PR #188652)

[Balázs Benics via llvm-branch-commits]

================
@@ -16,148 +16,102 @@
 #include "clang/ScalableStaticAnalysisFramework/Core/ASTEntityMapping.h"
 #include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityId.h"
 #include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityName.h"
+#include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/ExtractorRegistry.h"
+#include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryBuilder.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Error.h"
-#include <memory>
+#include "llvm/Support/ErrorHandling.h"
 
-namespace {
 using namespace clang;
 using namespace ssaf;
+namespace {
 
-static llvm::Error makeCreateEntityNameError(const NamedDecl *FailedDecl,
-                                             ASTContext &Ctx) {
-  std::string LocStr = FailedDecl->getSourceRange().getBegin().printToString(
-      Ctx.getSourceManager());
-  return llvm::createStringError(
-      "failed to create entity name for %s declared at %s",
-      FailedDecl->getNameAsString().c_str(), LocStr.c_str());
-}
-
-static llvm::Error makeAddEntitySummaryError(const NamedDecl *FailedContributor,
-                                             ASTContext &Ctx) {
-  std::string LocStr =
-      FailedContributor->getSourceRange().getBegin().printToString(
-          Ctx.getSourceManager());
-  return llvm::createStringError(
-      "failed to add entity summary for contributor %s declared at %s",
-      FailedContributor->getNameAsString().c_str(), LocStr.c_str());
-}
+struct UnsafePointerMatcher {
+  std::set<const Expr *> UnsafePointers;
 
-Expected<EntityPointerLevelSet>
-buildEntityPointerLevels(std::set<const Expr *> &&UnsafePointers,
-                         UnsafeBufferUsageTUSummaryExtractor &Extractor,
-                         ASTContext &Ctx,
-                         std::function<EntityId(EntityName)> AddEntity) {
-  EntityPointerLevelSet Result{};
-  llvm::Error AllErrors = llvm::ErrorSuccess();
-
-  for (const Expr *Ptr : UnsafePointers) {
-    Expected<EntityPointerLevelSet> Translation =
-        translateEntityPointerLevel(Ptr, Ctx, AddEntity);
-
-    if (Translation) {
-      // Filter out those temporary invalid EntityPointerLevels associated with
-      // `&E` pointers:
-      auto FilteredTranslation = llvm::make_filter_range(
-          *Translation, [](const EntityPointerLevel &E) -> bool {
-            return E.getPointerLevel() > 0;
-          });
-      Result.insert(FilteredTranslation.begin(), FilteredTranslation.end());
-      continue;
-    }
-    AllErrors = llvm::joinErrors(std::move(AllErrors), Translation.takeError());
+  bool matches(const DynTypedNode &N, ASTContext &Ctx,
+               const NamedDecl *Contributor) {
+    return matchUnsafePointers(N, Ctx, UnsafePointers);
   }
-  if (AllErrors)
-    return AllErrors;
-  return Result;
-}
-} // namespace
+};
 
-static std::set<const Expr *> findUnsafePointersInContributor(const Decl *D) {
-  if (isa<FunctionDecl>(D) || isa<VarDecl>(D))
-    return findUnsafePointers(D);
-  if (auto *RD = dyn_cast<RecordDecl>(D)) {
-    std::set<const Expr *> Result;
+void findFactsInContributor(const NamedDecl *Contributor, ASTContext &Ctx,
+                            std::set<const Expr *> &UnsafePointers) {
+  UnsafePointerMatcher Matcher;
+  ContributorFactFinder<UnsafePointerMatcher> Finder(Ctx, Matcher);
 
-    for (const FieldDecl *FD : RD->fields()) {
-      Result.merge(findUnsafePointers(FD));
-    }
-    return Result;
-  }
-  return {};
+  Finder.findMatches(Contributor);
+  UnsafePointers.merge(Matcher.UnsafePointers);
 }
+} // namespace
 
-std::unique_ptr<UnsafeBufferUsageEntitySummary>
-UnsafeBufferUsageTUSummaryExtractor::extractEntitySummary(
-    const Decl *Contributor, ASTContext &Ctx, llvm::Error &Error) {
-  auto AddEntity = [this](EntityName EN) { return addEntity(EN); };
-  Expected<EntityPointerLevelSet> EPLs = buildEntityPointerLevels(
-      findUnsafePointersInContributor(Contributor), *this, Ctx, AddEntity);
-
-  if (EPLs)
-    return std::make_unique<UnsafeBufferUsageEntitySummary>(
-        UnsafeBufferUsageEntitySummary(std::move(*EPLs)));
-  Error = EPLs.takeError();
-  return nullptr;
-}
+class clang::ssaf::UnsafeBufferUsageTUSummaryExtractor
+    : public TUSummaryExtractor {
+public:
+  UnsafeBufferUsageTUSummaryExtractor(TUSummaryBuilder &Builder)
+      : TUSummaryExtractor(Builder) {}
 
-void UnsafeBufferUsageTUSummaryExtractor::HandleTranslationUnit(
-    ASTContext &Ctx) {
+  EntityId addEntity(EntityName EN) { return SummaryBuilder.addEntity(EN); }
 
-  // FIXME: I suppose finding contributor Decls is commonly needed by all/many
-  // extractors
-  class ContributorFinder : public DynamicRecursiveASTVisitor {
-  public:
-    std::vector<const NamedDecl *> Contributors;
+  Expected<std::unique_ptr<UnsafeBufferUsageEntitySummary>>
+  extractEntitySummary(const NamedDecl *Contributor, ASTContext &Ctx) {
+    std::set<const Expr *> UnsafePointers;
+    EntityPointerLevelSet Results;
 
-    bool VisitFunctionDecl(FunctionDecl *D) override {
-      Contributors.push_back(D);
-      return true;
-    }
+    findFactsInContributor(Contributor, Ctx, UnsafePointers);
+    for (const Expr *Ptr : UnsafePointers) {
+      Expected<EntityPointerLevelSet> Translation =
+          translateEntityPointerLevel(Ptr, Ctx, [this](const EntityName &EN) {
+            return SummaryBuilder.addEntity(EN);
+          });
 
-    bool VisitRecordDecl(RecordDecl *D) override {
-      Contributors.push_back(D);
-      return true;
+      if (Translation) {
+        // Filter out those temporary invalid EntityPointerLevels associated
+        // with
+        // `&E` pointers. They need no transformation of entities:
----------------
steakhal wrote:

I have a suspicion that the newline in the middle of this comment was inserted by clang-format at some point. Maybe it fits nicely.

https://github.com/llvm/llvm-project/pull/188652