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

Ziqing Luo via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 29 16:37:16 PDT 2024


================
@@ -420,6 +420,118 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
   return false;
 }
 
+class MaxValueEval : public RecursiveASTVisitor<MaxValueEval> {
+
+  std::vector<llvm::APInt> val;
+  ASTContext &Context;
+  llvm::APInt Max;
+  unsigned bit_width;
+
+public:
+  typedef RecursiveASTVisitor<MaxValueEval> VisitorBase;
+
+  explicit MaxValueEval(ASTContext &Ctx, const Expr *exp) : Context(Ctx) {
+    bit_width = Ctx.getIntWidth(exp->getType());
+    Max = llvm::APInt::getSignedMaxValue(bit_width);
+    val.clear();
+  }
+
+  bool findMatch(Expr *exp) {
+    TraverseStmt(exp);
+    return true;
+  }
+
+  bool VisitDeclRefExpr(DeclRefExpr *dre) {
+    val.push_back(Max);
+    return false;
+  }
+
+  bool VisitArraySubscriptExpr(ArraySubscriptExpr *E) {
+    val.push_back(Max);
+    return false;
+  }
+
+  bool EvaluateExpression(Expr *exp) {
+    Expr::EvalResult EVResult;
+    if (exp->EvaluateAsInt(EVResult, Context)) {
+      llvm::APSInt Result = EVResult.Val.getInt();
+      val.push_back(Result);
+      return true;
+    }
+    return false;
+  }
+
+  bool VisitBinaryOperator(BinaryOperator *E) {
+
+    if (EvaluateExpression(E)) {
+      return false;
+    } else {
+      TraverseStmt(E->getLHS());
+      llvm::APInt LHS = val.back();
----------------
ziqingluo-90 wrote:

I think there is no guarantee of `val.size() > 0` here.  
This expression will break it: `(unsigned long)"x" << 1`

I also tried `s.f << 1`.  It doesn't crash because the DeclRefExpr `s` gets visited and hence `val` is populated. But I guess such behavior is not what you intended, because `val` should represent the approximation of `s.f` not `s`.

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


More information about the cfe-commits mailing list