[clang] [WIP][Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 16 16:56:24 PDT 2024


================
@@ -427,6 +427,48 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
   //    -  e. g. "Try harder to find a NamedDecl to point at in the note."
   //    already duplicated
   //  - call both from Sema and from here
+  std::function<llvm::APInt(const Expr *exp, unsigned int limit)>
+      SafeMaskedAccess;
+  unsigned int RecLimit = 5;
+
+  SafeMaskedAccess = [&](const Expr *exp, unsigned int RecLimit) -> llvm::APInt {
+    llvm::APInt Max = llvm::APInt::getMaxValue(Finder->getASTContext().getIntWidth(exp->getType()));
+    Max.setAllBits();
+
+    if (RecLimit == 0)
+      return Max;
+
+    //RecLimit--;
+
+    if (const auto *IntLit = dyn_cast<IntegerLiteral>(exp)) {
+      const APInt ArrIdx = IntLit->getValue();
+      return ArrIdx;
+    }
+
+    if (const auto *BinEx = dyn_cast<BinaryOperator>(exp)) {
+      llvm::APInt LHS = SafeMaskedAccess(BinEx->getLHS()->IgnoreParenCasts(), RecLimit);
+      llvm::APInt RHS = SafeMaskedAccess(BinEx->getRHS()->IgnoreParenCasts(), RecLimit);
----------------
haoNoQ wrote:

I'm somewhat worried about `IgnoreParenCasts()` because it disrespects the type system. You can get values of the wrong type if you skip it like this, or worse, run into sign extension issues. `IgnoreParens()` is probably fine but casts should probably be handled as a separate recursive case where you manually cast your `APInt` to the right type.

On the bright side, this might let you get rid of `.getLimitedValue()` because there will be no more type mismatches(?)

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


More information about the cfe-commits mailing list