[-Wunsafe-buffer-usage] Use relevant source locations for warnings

[jkorous]

Summary

This way we highlight a particular unsafe subexpression by providing more accurate source location than begin of an entire statement.

[aaron.ballman]

clang/lib/Sema/AnalysisBasedWarnings.cpp

2156

2159

2160

Please spell this type out and drop the top-level const.

2171–2172

Same here as above.

[jkorous]

Addressed the comments about use of auto in declarations.

[aaron.ballman]

LGTM

clang/lib/Sema/AnalysisBasedWarnings.cpp

 //===----------------------------------------------------------------------===//
 
 #include "clang/Sema/AnalysisBasedWarnings.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h"
 #include "clang/Analysis/Analyses/CalledOnceCheck.h"
 #include "clang/Analysis/Analyses/Consumed.h"

 
 class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
   Sema &S;
 
 public:
   UnsafeBufferUsageReporter(Sema &S) : S(S) {}
 
   void handleUnsafeOperation(const Stmt *Operation) override {
-    S.Diag(Operation->getBeginLoc(), diag::warn_unsafe_buffer_expression)
-        << Operation->getSourceRange();
+    SourceLocation Loc;
+    SourceRange Range;
+    if (auto ASE = dyn_cast<ArraySubscriptExpr>(Operation)) {
+      Loc = ASE->getBase()->getExprLoc();
+      Range = ASE->getBase()->getSourceRange();
+    } else if (auto BO = dyn_cast<BinaryOperator>(Operation)) {
+      const auto Op = BO->getOpcode();
+      if (Op == BO_Add || Op == BO_AddAssign || Op == BO_Sub ||
+          Op == BO_SubAssign) {
+        if (BO->getRHS()->getType()->isIntegerType()) {
+          Loc = BO->getLHS()->getExprLoc();
+          Range = BO->getLHS()->getSourceRange();
+        } else {
+          Loc = BO->getRHS()->getExprLoc();
+          Range = BO->getRHS()->getSourceRange();
+        }
+      }
+    } else if (auto UO = dyn_cast<UnaryOperator>(Operation)) {
+      const auto Op = UO->getOpcode();
+      if (Op == UO_PreInc || Op == UO_PreDec || Op == UO_PostInc ||
+          Op == UO_PostDec) {
+        Loc = UO->getSubExpr()->getExprLoc();
+        Range = UO->getSubExpr()->getSourceRange();
+      }
+    } else {
+      Loc = Operation->getBeginLoc();
+      Range = Operation->getSourceRange();
+    }
+    S.Diag(Loc, diag::warn_unsafe_buffer_expression) << Range;
   }
 
   void handleFixableVariable(const VarDecl *Variable,
                              FixItList &&Fixes) override {
     const auto &D =
         S.Diag(Variable->getBeginLoc(), diag::warn_unsafe_buffer_variable);
     D << Variable << Variable->getSourceRange();
     for (const auto &F: Fixes)

clang/test/SemaCXX/warn-unsafe-buffer-usage-source-ranges.cpp

+// RUN: %clang_cc1 -Wno-everything -Wunsafe-buffer-usage -fdiagnostics-print-source-range-info %s 2>&1 | FileCheck %s
+
+void foo(int i) {
+  int * ptr;
+
+
+  ptr++;
+  // CHECK-DAG: {7:3-7:6}{{.*}}[-Wunsafe-buffer-usage]
+  ptr--;
+  // CHECK-DAG: {9:3-9:6}{{.*}}[-Wunsafe-buffer-usage]
+  ++ptr;
+  // CHECK-DAG: {11:5-11:8}{{.*}}[-Wunsafe-buffer-usage]
+  --ptr;
+  // CHECK-DAG: {13:5-13:8}{{.*}}[-Wunsafe-buffer-usage]
+
+
+  ptr + 1;
+  // CHECK-DAG: {17:3-17:6}{{.*}}[-Wunsafe-buffer-usage]
+  2 + ptr;
+  // CHECK-DAG: {19:7-19:10}{{.*}}[-Wunsafe-buffer-usage]
+  ptr + i;
+  // CHECK-DAG: {21:3-21:6}{{.*}}[-Wunsafe-buffer-usage]
+  i + ptr;
+  // CHECK-DAG: {23:7-23:10}{{.*}}[-Wunsafe-buffer-usage]
+
+
+  ptr - 3;
+  // CHECK-DAG: {27:3-27:6}{{.*}}[-Wunsafe-buffer-usage]
+  ptr - i;
+  // CHECK-DAG: {29:3-29:6}{{.*}}[-Wunsafe-buffer-usage]
+
+
+  ptr += 4;
+  // CHECK-DAG: {33:3-33:6}{{.*}}[-Wunsafe-buffer-usage]
+  ptr += i;
+  // CHECK-DAG: {35:3-35:6}{{.*}}[-Wunsafe-buffer-usage]
+
+
+  ptr -= 5;
+  // CHECK-DAG: {39:3-39:6}{{.*}}[-Wunsafe-buffer-usage]
+  ptr -= i;
+  // CHECK-DAG: {41:3-41:6}{{.*}}[-Wunsafe-buffer-usage]
+
+
+  ptr[5];
+  // CHECK-DAG: {45:3-45:6}{{.*}}[-Wunsafe-buffer-usage]
+  5[ptr];
+  // CHECK-DAG: {47:5-47:8}{{.*}}[-Wunsafe-buffer-usage]
+}