Fixing Bounds issue with Function Parameters (#582)

Machiry · mattmccutchen-cci · web-flow · commit 07ed3827abb9 · 2021-05-28T13:58:40.000-04:00
* Fixing Bounds issue with Function Parameters

* Update clang/test/3C/func_parm_bounds.c

Co-authored-by: Matt McCutchen (Correct Computation) &lt;matt@correctcomputation.com&gt;

Co-authored-by: Matt McCutchen (Correct Computation) &lt;matt@correctcomputation.com&gt;
diff --git a/clang/include/clang/3C/ABounds.h b/clang/include/clang/3C/ABounds.h
@@ -41,11 +41,16 @@ class ABounds {
 protected:
   ABounds(BoundsKind K) : Kind(K) {}
   void addBoundsUsedKey(BoundsKey);
+  // Get the variable name of the the given bounds key that corresponds
+  // to the given declaration.
+  static std::string getBoundsKeyStr(BoundsKey ,
+                                     AVarBoundsInfo *,
+                                     clang::Decl *);
 
 public:
   virtual ~ABounds() {}
 
-  virtual std::string mkString(AVarBoundsInfo *) = 0;
+  virtual std::string mkString(AVarBoundsInfo *, clang::Decl *D = nullptr) = 0;
   virtual bool areSame(ABounds *, AVarBoundsInfo *) = 0;
   virtual BoundsKey getBKey() = 0;
   virtual ABounds *makeCopy(BoundsKey NK) = 0;
@@ -67,7 +72,7 @@ class CountBound : public ABounds {
 
   virtual ~CountBound() {}
 
-  std::string mkString(AVarBoundsInfo *ABI) override;
+  std::string mkString(AVarBoundsInfo *ABI, clang::Decl *D = nullptr) override;
   bool areSame(ABounds *O, AVarBoundsInfo *ABI) override;
   BoundsKey getBKey() override;
   ABounds *makeCopy(BoundsKey NK) override;
@@ -90,7 +95,7 @@ class ByteBound : public ABounds {
 
   virtual ~ByteBound() {}
 
-  std::string mkString(AVarBoundsInfo *ABI) override;
+  std::string mkString(AVarBoundsInfo *ABI, clang::Decl *D = nullptr) override;
   bool areSame(ABounds *O, AVarBoundsInfo *ABI) override;
   BoundsKey getBKey() override;
   ABounds *makeCopy(BoundsKey NK) override;
@@ -113,7 +118,7 @@ class RangeBound : public ABounds {
 
   virtual ~RangeBound() {}
 
-  std::string mkString(AVarBoundsInfo *ABI) override;
+  std::string mkString(AVarBoundsInfo *ABI, clang::Decl *D = nullptr) override;
   bool areSame(ABounds *O, AVarBoundsInfo *ABI) override;
 
   BoundsKey getBKey() override {
diff --git a/clang/include/clang/3C/AVarBoundsInfo.h b/clang/include/clang/3C/AVarBoundsInfo.h
@@ -255,6 +255,10 @@ class AVarBoundsInfo {
 
   bool areSameProgramVar(BoundsKey B1, BoundsKey B2);
 
+  // Check if the provided BoundsKey is for a function param?
+  // If yes, provide the index of the parameter.
+  bool isFuncParamBoundsKey(BoundsKey BK, unsigned &PIdx);
+
 private:
   friend class AvarBoundsInference;
   friend class CtxSensitiveBoundsKeyHandler;
diff --git a/clang/lib/3C/ABounds.cpp b/clang/lib/3C/ABounds.cpp
@@ -56,10 +56,29 @@ ABounds *ABounds::getBoundsInfo(AVarBoundsInfo *ABInfo, BoundsExpr *BExpr,
   return Ret;
 }
 
-std::string CountBound::mkString(AVarBoundsInfo *ABI) {
-  ProgramVar *PV = ABI->getProgramVar(CountVar);
+std::string ABounds::getBoundsKeyStr(BoundsKey BK,
+                                     AVarBoundsInfo *ABI,
+                                     Decl *D) {
+  ProgramVar *PV = ABI->getProgramVar(BK);
   assert(PV != nullptr && "No Valid program var");
-  return "count(" + PV->mkString() + ")";
+  std::string BKStr = PV->mkString();
+  unsigned PIdx = 0;
+  auto *PVD = dyn_cast_or_null<ParmVarDecl>(D);
+  // Does this belong to a function parameter?
+  if (PVD && ABI->isFuncParamBoundsKey(BK, PIdx)) {
+    // Then get the corresponding parameter in context of the given
+    // Function and get its name.
+    const FunctionDecl *FD = dyn_cast<FunctionDecl>(PVD->getDeclContext());
+    if (FD->getNumParams() > PIdx) {
+      auto *NewPVD = FD->getParamDecl(PIdx);
+      BKStr = NewPVD->getNameAsString();
+    }
+  }
+  return BKStr;
+}
+
+std::string CountBound::mkString(AVarBoundsInfo *ABI, clang::Decl *D) {
+  return "count(" + ABounds::getBoundsKeyStr(CountVar, ABI, D) + ")";
 }
 
 bool CountBound::areSame(ABounds *O, AVarBoundsInfo *ABI) {
@@ -74,10 +93,8 @@ BoundsKey CountBound::getBKey() { return this->CountVar; }
 
 ABounds *CountBound::makeCopy(BoundsKey NK) { return new CountBound(NK); }
 
-std::string ByteBound::mkString(AVarBoundsInfo *ABI) {
-  ProgramVar *PV = ABI->getProgramVar(ByteVar);
-  assert(PV != nullptr && "No Valid program var");
-  return "byte_count(" + PV->mkString() + ")";
+std::string ByteBound::mkString(AVarBoundsInfo *ABI, clang::Decl *D) {
+  return "byte_count(" + ABounds::getBoundsKeyStr(ByteVar, ABI, D) + ")";
 }
 
 bool ByteBound::areSame(ABounds *O, AVarBoundsInfo *ABI) {
@@ -93,11 +110,11 @@ BoundsKey ByteBound::getBKey() { return this->ByteVar; }
 
 ABounds *ByteBound::makeCopy(BoundsKey NK) { return new ByteBound(NK); }
 
-std::string RangeBound::mkString(AVarBoundsInfo *ABI) {
-  ProgramVar *LBVar = ABI->getProgramVar(LB);
-  ProgramVar *UBVar = ABI->getProgramVar(UB);
-  assert(LBVar != nullptr && UBVar != nullptr && "No Valid program var");
-  return "bounds(" + LBVar->mkString() + ", " + UBVar->mkString() + ")";
+
+std::string RangeBound::mkString(AVarBoundsInfo *ABI, clang::Decl *D) {
+  std::string LBStr = ABounds::getBoundsKeyStr(LB, ABI, D);
+  std::string UBStr = ABounds::getBoundsKeyStr(UB, ABI, D);
+  return "bounds(" + LBStr + ", " + UBStr + ")";
 }
 
 bool RangeBound::areSame(ABounds *O, AVarBoundsInfo *ABI) {
diff --git a/clang/lib/3C/AVarBoundsInfo.cpp b/clang/lib/3C/AVarBoundsInfo.cpp
@@ -1403,6 +1403,16 @@ bool AVarBoundsInfo::areSameProgramVar(BoundsKey B1, BoundsKey B2) {
   return B1 == B2;
 }
 
+bool AVarBoundsInfo::isFuncParamBoundsKey(BoundsKey BK, unsigned &PIdx) {
+  auto &ParmBkeyToPSL = ParamDeclVarMap.right();
+  if (ParmBkeyToPSL.find(BK) != ParmBkeyToPSL.end()) {
+    auto &ParmTup = ParmBkeyToPSL.at(BK);
+    PIdx = std::get<3>(ParmTup);
+    return true;
+  }
+  return false;
+}
+
 std::set<BoundsKey>
 AVarBoundsInfo::getCtxSensFieldBoundsKey(Expr *E, ASTContext *C,
                                          ProgramInfo &I) {
diff --git a/clang/lib/3C/RewriteUtils.cpp b/clang/lib/3C/RewriteUtils.cpp
@@ -597,7 +597,7 @@ std::string ArrayBoundsRewriter::getBoundsString(const PVConstraint *PV,
     ABounds *ArrB = ABInfo.getBounds(DK);
     // Only we we have bounds and no pointer arithmetic on the variable.
     if (ArrB != nullptr && !ABInfo.hasPointerArithmetic(DK)) {
-      BString = ArrB->mkString(&ABInfo);
+      BString = ArrB->mkString(&ABInfo, D);
       if (!BString.empty())
         BString = Pfix + BString;
     }
diff --git a/clang/test/3C/func_parm_bounds.c b/clang/test/3C/func_parm_bounds.c
@@ -0,0 +1,49 @@
+/**
+Test for parameter bounds with parameter with in the declaration.
+Issue: https://github.com/correctcomputation/checkedc-clang/issues/573
+**/
+
+// RUN: rm -rf %t*
+// RUN: 3c -base-dir=%S -alltypes %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL" %s
+// RUN: 3c -base-dir=%S -alltypes %s -- | %clang -c -fcheckedc-extension -x c -o %t1.unusedl -
+// RUN: 3c -base-dir=%S %s -- | %clang -c -fcheckedc-extension -x c -o %t2.unused -
+
+#include <stdlib.h>
+
+int gsize;
+int *glob;
+//CHECK_ALL: _Array_ptr<int> glob : count(gsize) = ((void *)0);
+
+int foo(int *c, int z);
+int foo(int *c, int l) {
+  if (0 < l) return c[0];
+  return 0;
+}
+//CHECK_ALL: int foo(_Array_ptr<int> c : count(z), int z);
+//CHECK_ALL: int foo(_Array_ptr<int> c : count(l), int l) {
+
+int consfoo(int *c);
+int consfoo(int *c) {
+  return c[0];
+}
+//CHECK_ALL: int consfoo(_Array_ptr<int> c : count(5));
+//CHECK_ALL: int consfoo(_Array_ptr<int> c : count(5)) {
+
+int globalfoo(int *c);
+int globalfoo(int *c) {
+  return c[0];
+}
+//CHECK_ALL: int globalfoo(_Array_ptr<int> c : count(gsize));
+//CHECK_ALL: int globalfoo(_Array_ptr<int> c : count(gsize)) {
+
+int caller() {
+  int arr[5];
+  gsize = 100;
+  glob = malloc(gsize*sizeof(int));
+  glob[0] = 1;
+  globalfoo(glob);
+  consfoo(arr);
+  return 0;
+}
+//CHECK_ALL: int arr _Checked[5];
+//CHECK_ALL: glob = _Assume_bounds_cast<_Array_ptr<int>>(malloc(gsize*sizeof(int)), byte_count(0));

Original file line number	Diff line number	Diff line change
`@@ -597,7 +597,7 @@ std::string ArrayBoundsRewriter::getBoundsString(const PVConstraint *PV,`
`597`	`597`	`ABounds *ArrB = ABInfo.getBounds(DK);`
`598`	`598`	`// Only we we have bounds and no pointer arithmetic on the variable.`
`599`	`599`	`if (ArrB != nullptr && !ABInfo.hasPointerArithmetic(DK)) {`
`600`		`- BString = ArrB->mkString(&ABInfo);`
	`600`	`+ BString = ArrB->mkString(&ABInfo, D);`
`601`	`601`	`if (!BString.empty())`
`602`	`602`	`BString = Pfix + BString;`
`603`	`603`	`}`