[llvm-branch-commits] [clang-tools-extra] e097724 - [clang-tidy] Fix crash on "reference-to-array" parameters in 'bugprone-easily-swappable-parameters'

[Tom Stellard via llvm-branch-commits]

Author: Whisperity
Date: 2021-08-05T10:16:24-07:00
New Revision: e097724cb9efbc3a068767ecf264897e97b2e109

URL: https://github.com/llvm/llvm-project/commit/e097724cb9efbc3a068767ecf264897e97b2e109
DIFF: https://github.com/llvm/llvm-project/commit/e097724cb9efbc3a068767ecf264897e97b2e109.diff

LOG: [clang-tidy] Fix crash on "reference-to-array" parameters in 'bugprone-easily-swappable-parameters'

An otherwise unexercised code path related to trying to model
"array-to-pointer decay" resulted in a null pointer dereference crash
when parameters of type "reference to array" were encountered.

Fixes crash report http://bugs.llvm.org/show_bug.cgi?id=50995.

Reviewed By: aaron.ballman

Differential Revision: http://reviews.llvm.org/D106946

(cherry picked from commit 21832121e112d97f1e197b35959867f3a99226ee)

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
    clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
    clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
index afcdca226911d..e4500706b4b2e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
@@ -962,11 +962,8 @@ approximateStandardConversionSequence(const TheCheck &Check, QualType From,
   // LValue->RValue is irrelevant for the check, because it is a thing to be
   // done at a call site, and will be performed if need be performed.
 
-  // Array->Ptr decay.
-  if (const auto *ArrayT = dyn_cast<ArrayType>(From)) {
-    LLVM_DEBUG(llvm::dbgs() << "--- approximateStdConv. Array->Ptr decayed.\n");
-    WorkType = ArrayT->getPointeeType();
-  }
+  // Array->Pointer decay is handled by the main method in desugaring
+  // the parameter's DecayedType as "useless sugar".
 
   // Function->Pointer conversions are also irrelevant, because a
   // "FunctionType" cannot be the type of a parameter variable, so this

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
index c1a72d687b135..ba7aa44a8731a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
@@ -26,6 +26,50 @@ void arrayAndPtr2(int *IP, int IA[8]) { arrayAndPtr2(IA, IP); }
 
 void arrayAndElement(int I, int IA[]) {} // NO-WARN.
 
+typedef int Point2D[2];
+typedef int Point3D[3];
+
+void arrays1(Point2D P2D, Point3D P3D) {} // In reality this is (int*, int*).
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 2 adjacent parameters of 'arrays1' of similar type ('int *') are
+// CHECK-MESSAGES: :[[@LINE-2]]:22: note: the first parameter in the range is 'P2D'
+// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'P3D'
+
+void crefToArrayTypedef1(int I, const Point2D &P) {}
+// NO-WARN.
+
+void crefToArrayTypedef2(int *IA, const Point2D &P) {}
+// NO-WARN.
+
+void crefToArrayTypedef3(int P1[2], const Point2D &P) {}
+// NO-WARN.
+
+void crefToArrayTypedefBoth1(const Point2D &VecDescartes, const Point3D &VecThreeD) {}
+// NO-WARN: Distinct types.
+
+template <int N, int M>
+void templatedArrayRef(int (&Array1)[N], int (&Array2)[M]) {}
+// NO-WARN: Distinct template types in the primary template.
+
+void templatedArrayRefTest() {
+  int Foo[12], Bar[12];
+  templatedArrayRef(Foo, Bar);
+
+  int Baz[12], Quux[42];
+  templatedArrayRef(Baz, Quux);
+
+  // NO-WARN: Implicit instantiations are not checked.
+}
+
+template <>
+void templatedArrayRef(int (&Array1)[8], int (&Array2)[8]) { templatedArrayRef(Array2, Array1); }
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 2 adjacent parameters of 'templatedArrayRef<8, 8>' of similar type ('int (&)[8]') are
+// CHECK-MESSAGES: :[[@LINE-2]]:30: note: the first parameter in the range is 'Array1'
+// CHECK-MESSAGES: :[[@LINE-3]]:48: note: the last parameter in the range is 'Array2'
+
+template <>
+void templatedArrayRef(int (&Array1)[16], int (&Array2)[24]) {}
+// NO-WARN: Not the same type.
+
 void numericConversion1(int I, double D) { numericConversion1(D, I); }
 // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 2 adjacent parameters of 'numericConversion1' of convertible types are easily swapped by mistake [bugprone-easily-swappable-parameters]
 // CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
index aeb43e986d537..c061ad7958316 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
@@ -244,6 +244,26 @@ void referenceThroughTypedef(int I, ICRTy Builtin, MyIntCRTy MyInt) {}
 // CHECK-MESSAGES: :[[@LINE-6]]:52: note: 'int' and 'MyIntCRTy' parameters accept and bind the same kind of values
 // CHECK-MESSAGES: :[[@LINE-7]]:37: note: after resolving type aliases, the common type of 'ICRTy' and 'MyIntCRTy' is 'const int &'
 
+typedef int Point2D[2];
+typedef int Point3D[3];
+
+void arrays1(Point2D P2D, Point3D P3D) {} // In reality this is (int*, int*).
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 2 adjacent parameters of 'arrays1' of similar type ('int *') are
+// CHECK-MESSAGES: :[[@LINE-2]]:22: note: the first parameter in the range is 'P2D'
+// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'P3D'
+
+void crefToArrayTypedef1(int I, const Point2D &P) {}
+// NO-WARN.
+
+void crefToArrayTypedef2(int *IA, const Point2D &P) {}
+// NO-WARN.
+
+void crefToArrayTypedef3(int P1[2], const Point2D &P) {}
+// NO-WARN.
+
+void crefToArrayTypedefBoth1(const Point2D &VecDescartes, const Point3D &VecThreeD) {}
+// NO-WARN: Distinct types and no conversion because of &.
+
 short const typedef int unsigned Eldritch;
 typedef const unsigned short Holy;
 
@@ -300,6 +320,30 @@ void templateAndAliasTemplate(Pair<int, int> P, TwoOf<int> I) {}
 // CHECK-MESSAGES: :[[@LINE-2]]:46: note: the first parameter in the range is 'P'
 // CHECK-MESSAGES: :[[@LINE-3]]:60: note: the last parameter in the range is 'I'
 
+template <int N, int M>
+void templatedArrayRef(int (&Array1)[N], int (&Array2)[M]) {}
+// NO-WARN: Distinct template types in the primary template.
+
+void templatedArrayRefTest() {
+  int Foo[12], Bar[12];
+  templatedArrayRef(Foo, Bar);
+
+  int Baz[12], Quux[42];
+  templatedArrayRef(Baz, Quux);
+
+  // NO-WARN: Implicit instantiations are not checked.
+}
+
+template <>
+void templatedArrayRef(int (&Array1)[8], int (&Array2)[8]) { templatedArrayRef(Array2, Array1); }
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 2 adjacent parameters of 'templatedArrayRef<8, 8>' of similar type ('int (&)[8]') are
+// CHECK-MESSAGES: :[[@LINE-2]]:30: note: the first parameter in the range is 'Array1'
+// CHECK-MESSAGES: :[[@LINE-3]]:48: note: the last parameter in the range is 'Array2'
+
+template <>
+void templatedArrayRef(int (&Array1)[16], int (&Array2)[24]) {}
+// NO-WARN: Not the same type.
+
 template <typename T>
 struct Vector {
   typedef T element_type;