Fix array bounds heuristics on itype parameters (#654)

The internal constraint variable for function parameters was used to checked the
solved pointer type when adding array bounds heuristics.  This made itype
parameters look like wild pointers instead of array or nt array pointers, so
the heuristics were not applied. Heuristics are now applied based on the
external parameter, which solves to a checked type for itype parameters.

This resolves the remaining issue in #487

Author: john-h-kastner

clang/lib/3C/ArrayBoundsInferenceConsumer.cpp

@@ -118,16 +118,20 @@ static bool needArrayBounds(Expr *E, ProgramInfo &Info, ASTContext *C) {
   return false;
 }
 
+static bool
+needArrayBounds(ConstraintVariable *CV, ProgramInfo &Info, bool IsNtArr) {
+  const auto &E = Info.getConstraints().getVariables();
+  if (IsNtArr)
+    return needNTArrayBounds(CV, E);
+  return needArrayBounds(CV, E);
+}
+
 static bool needArrayBounds(Decl *D, ProgramInfo &Info, ASTContext *C,
                             bool IsNtArr) {
-  const auto &E = Info.getConstraints().getVariables();
   CVarOption CVar = Info.getVariable(D, C);
   if (CVar.hasValue()) {
     ConstraintVariable &CV = CVar.getValue();
-    if ((!IsNtArr && needArrayBounds(&CV, E)) ||
-        (IsNtArr && needNTArrayBounds(&CV, E)))
-      return true;
-    return false;
+    return needArrayBounds(&CV, Info, IsNtArr);
   }
   return false;
 }
@@ -440,6 +444,11 @@ bool GlobalABVisitor::VisitFunctionDecl(FunctionDecl *FD) {
       std::map<unsigned, std::pair<std::string, BoundsKey>> ParamNtArrays;
       std::map<unsigned, std::pair<std::string, BoundsKey>> LengthParams;
 
+      // The FVConstraint is needed to access the external parameter
+      // ConstraintVariables. External CVs are required because these
+      // are there variables can solve to a checked array type when the
+      // parameter has an itype while the internals will solve to WILD.
+      FVConstraint *FV = Info.getFuncConstraint(FD, Context);
       for (unsigned I = 0; I < FT->getNumParams(); I++) {
         ParmVarDecl *PVD = FD->getParamDecl(I);
         BoundsKey PK;
@@ -451,14 +460,14 @@ bool GlobalABVisitor::VisitFunctionDecl(FunctionDecl *FD) {
           // Here, we are using heuristics. So we only use heuristics when
           // there are no bounds already computed.
           if (!ABInfo.getBounds(PK)) {
-            if (needArrayBounds(PVD, Info, Context, true)) {
-              // Is this an NTArray?
+            PVConstraint *ParamCV = FV->getExternalParam(I);
+            const EnvironmentMap &Env = Info.getConstraints().getVariables();
+            // Is this an NTArray?
+            if (needNTArrayBounds(ParamCV, Env))
               ParamNtArrays[I] = PVal;
-            }
-            if (needArrayBounds(PVD, Info, Context, false)) {
-              // Is this an array?
+            // Is this an array?
+            if (needArrayBounds(ParamCV, Env))
               ParamArrays[I] = PVal;
-            }
           }
 
           // If this is a length field?

clang/test/3C/arrboundsheuristicsitype.c

@@ -0,0 +1,35 @@
+// RUN: rm -rf %t*
+// RUN: 3c -base-dir=%S -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK,CHECK_NOALL" %s
+// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK,CHECK_ALL" %s
+// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | %clang -c -x c -o /dev/null -
+// RUN: 3c -base-dir=%S -alltypes -output-dir=%t.checked %s --
+// RUN: 3c -base-dir=%t.checked -alltypes %t.checked/arrboundsheuristicsitype.c -- | diff %t.checked/arrboundsheuristicsitype.c -
+
+// Verify that array bounds heuristics are correctly applied to parameters with
+// itypes.
+
+void foo(void *);
+
+void test0(int *a, int len) {
+//CHECK_ALL: void test0(int *a : itype(_Array_ptr<int>) count(len), int len) {
+//CHECK_NOALL: void test0(int *a : itype(_Ptr<int>), int len) {
+  foo(a);
+  (void) a[len - 1];
+}
+
+void test1(int *a : itype(_Array_ptr<int>), int len) {
+//CHECK_ALL: void test1(int *a : itype(_Array_ptr<int>) count(len), int len) {
+//CHECK_NOALL: void test1(int *a : itype(_Ptr<int>), int len) {
+  foo(a);
+  (void) a[len - 1];
+}
+
+#include<stdlib.h>
+
+void test2(int *a, int len) {
+//CHECK_ALL: void test2(int *a : itype(_Array_ptr<int>) count(len), int len) {
+//CHECK_NOALL: void test2(int *a : itype(_Ptr<int>), int len) {
+  a = malloc(sizeof(int)*len);
+  foo(a);
+  (void) a[len - 1];
+}