[clang] [-Wunsafe-buffer-usage] Add FixableGadget for AddAssign in UnspecifiedUntypedContext (PR #71862)

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 13 13:09:16 PST 2023


================
@@ -1028,6 +1028,50 @@ class UPCPreIncrementGadget : public FixableGadget {
   }
 };
 
+// Representing a pointer type expression of the form `Ptr += n` in an
+// Unspecified Untyped Context (UUC):
+class UUCAddAssignGadget : public FixableGadget {
+private:
+  static constexpr const char *const UUCAddAssignTag =
+      "PointerAddAssignUnderUUC";
+  static constexpr const char *const IntOffsetTag = "IntOffset";
+  static constexpr const char *const OffsetTag = "Offset";
+
+  const BinaryOperator *Node; // the `Ptr += n` node
+  const IntegerLiteral *IntOffset = nullptr;
+  const DeclRefExpr *Offset = nullptr;
+
+public:
+  UUCAddAssignGadget(const MatchFinder::MatchResult &Result)
+      : FixableGadget(Kind::UUCAddAssign),
+        Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)),
+        IntOffset(Result.Nodes.getNodeAs<IntegerLiteral>(IntOffsetTag)),
+        Offset(Result.Nodes.getNodeAs<DeclRefExpr>(OffsetTag)) {
+    assert(Node != nullptr && "Expecting a non-null matching result");
+  }
+
+  static bool classof(const Gadget *G) {
+    return G->getKind() == Kind::UUCAddAssign;
+  }
+
+  static Matcher matcher() {
+    return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts(
+        binaryOperator(
+            hasOperatorName("+="), hasLHS(declRefExpr(toSupportedVariable())),
+            hasRHS(expr(anyOf(ignoringImpCasts(declRefExpr().bind(OffsetTag)),
+                              integerLiteral().bind(IntOffsetTag)))))
----------------
haoNoQ wrote:

Oh right! `ULCArraySubscriptGadget` already has a similar facility for determining if the value is non-negative. It's probably good idea to reuse code, because this is the exact same problem. IIRC, @ziqingluo-90 handled the non-constant case by confirming that the expression has unsigned integer type, which sounds like an ok compromise. Also constant-folded the integer before checking.

Might be a good idea to define a matcher "`isNonNegativeIntegerExpr()`" or something, and include it in both gadgets.

I also wonder if implicit casts need to be ignored in both cases. IIRC array subscript expressions don't do implicit promotion, they just stuff arbitrary integer types into the index. But for arithmetic there will be implicit casts to represent promotions, so they may need to be skipped(?)

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


More information about the cfe-commits mailing list