[clang] Respect the [[clang::unsafe_buffer_usage]] attribute for field and constructor initializers (PR #91991)

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Tue May 14 16:03:53 PDT 2024


================
@@ -3328,3 +3300,63 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
     }
   }
 }
+
+void clang::checkUnsafeBufferUsage(const Decl *D,
+                                   UnsafeBufferUsageHandler &Handler,
+                                   bool EmitSuggestions) {
+#ifndef NDEBUG
+  Handler.clearDebugNotes();
+#endif
+
+  assert(D);
+
+  SmallVector<Stmt *> Stmts;
+
+  // We do not want to visit a Lambda expression defined inside a method
+  // independently. Instead, it should be visited along with the outer method.
+  // FIXME: do we want to do the same thing for `BlockDecl`s?
+  if (const auto *fd = dyn_cast<CXXMethodDecl>(D)) {
+    if (fd->getParent()->isLambda() && fd->getParent()->isLocalClass())
+      return;
+  }
+
+  // Do not emit fixit suggestions for functions declared in an
+  // extern "C" block.
+  if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+    for (FunctionDecl *FReDecl : FD->redecls()) {
+      if (FReDecl->isExternC()) {
+        EmitSuggestions = false;
+        break;
+      }
+    }
+
+    Stmts.push_back(FD->getBody());
+
+    if (const auto *ID = dyn_cast<CXXConstructorDecl>(D)) {
+      for (const CXXCtorInitializer *CI : ID->inits()) {
+        Stmts.push_back(CI->getInit());
+      }
+    }
+  }
+
+  if (const auto *FD = dyn_cast<FieldDecl>(D)) {
+    // Visit in-class initializers for fields.
+    if (!FD->hasInClassInitializer())
+      return;
+
+    Stmts.push_back(FD->getInClassInitializer());
+  }
+
+  if (isa<BlockDecl>(D) || isa<ObjCMethodDecl>(D)) {
+    Stmts.push_back(D->getBody());
+  }
+
+  assert(!Stmts.empty());
+
+  for (Stmt *S : Stmts) {
+    auto [FixableGadgets, WarningGadgets, Tracker] =
+        findGadgets(S, D->getASTContext(), Handler, EmitSuggestions);
+    applyGadgets(D, std::move(FixableGadgets), std::move(WarningGadgets),
----------------
haoNoQ wrote:

I think `applyGadgets()` should be invoked only once per function. For instance, when we're emitting a fixit for a function parameter, we emit a single fixit that covers every use of that parameter. We can't do that separately for the use of that parameter in a `CXXCtorInitializer` and then independently do the same for the rest of the function body. That'd result in conflicting fixes.

The tracker should also be re-used for the entire function. We need to know if we can fix the use inside the `CXXCtorInitializer` at all before emitting a fixit for all other uses.

(Deep inside their hearts these data structures dream of becoming global for the entire translation unit.)

So probably `[FixableGadgets, WarningGadgets, Tracker]` should be passed by reference, to `push_back()` gadgets directly into them.

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


More information about the cfe-commits mailing list