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

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

Differential Revision: https://reviews.llvm.org/D141340

diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 3a0f566..08cf45e 100644 (file)
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -16,8 +16,10 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/AST/Expr.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"
@@ -2152,8 +2154,35 @@ 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 (const auto *ASE = dyn_cast<ArraySubscriptExpr>(Operation)) {
+      Loc = ASE->getBase()->getExprLoc();
+      Range = ASE->getBase()->getSourceRange();
+    } else if (const auto *BO = dyn_cast<BinaryOperator>(Operation)) {
+      BinaryOperator::Opcode 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 (const auto *UO = dyn_cast<UnaryOperator>(Operation)) {
+      UnaryOperator::Opcode 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,

diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-source-ranges.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-source-ranges.cpp
new file mode 100644 (file)
index 0000000..35928cc
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-source-ranges.cpp
@@ -0,0 +1,49 @@
+// 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]
+}