[checked-c-convert] Minor fixes to the checked-c-convert (#679)

Cherry-picked from commit ba778c3

    - Add knowledge of calloc and realloc as allocator functions.
    - Add initializers for variables retyped as checked pointers.
    - Add initializers for structure variables containing members that have been retyped as checked pointers.
    - Avoid deletion of variable arguments.

Author: Machiry

clang/test/CheckedCRewriter/bounds_interface.c

@@ -7,10 +7,10 @@
 
 
 extern void bar(int *q : itype(_Ptr<int>));
-//CHECK: extern void bar(int* q : itype(_Ptr<int>));
+//CHECK: extern void bar(int *q : itype(_Ptr<int>));
 
 extern void bar2(int *q : itype(_Ptr<int>), int *z : itype(_Ptr<int>));
-//CHECK: extern void bar2(int* q : itype(_Ptr<int>), int* z : itype(_Ptr<int>));
+//CHECK: extern void bar2(int *q : itype(_Ptr<int>), int *z : itype(_Ptr<int>));
 
 extern int *baz(void) : itype(_Ptr<int>);
 //CHECK: extern int *baz(void) : itype(_Ptr<int>);

clang/test/CheckedCRewriter/regression_tests.c

@@ -0,0 +1,18 @@
+// Tests for Checked C rewriter tool.
+//
+// Tests checked-c-convert tool for any regressions.
+//
+// RUN: checked-c-convert %s -- | FileCheck -match-full-lines %s
+//
+
+#include <stdlib_checked.h>
+
+int main() {
+
+  char *ptr1 = NULL;
+
+  ptr1 = (char *) calloc(1, sizeof(char));
+
+  return 0;
+}
+//CHECK: _Ptr<char> ptr1 =  NULL;

clang/tools/checked-c-convert/CheckedCConvert.cpp

@@ -78,6 +78,11 @@ cl::opt<bool> enablePropThruIType( "enable-itypeprop",
                                    cl::init(false),
                                    cl::cat(ConvertCategory));
 
+cl::opt<bool> considerAllocUnsafe( "alloc-unsafe",
+                                   cl::desc("Consider the allocators (i.e., malloc/calloc) as unsafe."),
+                                   cl::init(false),
+                                   cl::cat(ConvertCategory));
+
 static cl::opt<std::string>
 BaseDir("base-dir",
   cl::desc("Base directory for the code we're translating"),

clang/tools/checked-c-convert/ConstraintBuilder.cpp

@@ -268,27 +268,32 @@ class FunctionVisitor : public RecursiveASTVisitor<FunctionVisitor> {
             // to a NamedDecl?
             FunctionDecl *calleeDecl =
               dyn_cast<FunctionDecl>(CA->getCalleeDecl());
-            if (calleeDecl && calleeDecl->getName() == "malloc") {
-              // It's a call to malloc. What about the parameter to the call?
-              if (CA->getNumArgs() > 0) {
-                UnaryExprOrTypeTraitExpr *arg =
-                  dyn_cast<UnaryExprOrTypeTraitExpr>(CA->getArg(0));
-                if (arg && arg->isArgumentType()) {
-                  // Check that the argument is a sizeof.
-                  if (arg->getKind() == UETT_SizeOf) {
-                    QualType argTy = arg->getArgumentType();
-                    // argTy should be made a pointer, then compared for
-                    // equality to lhsType and rhsTy.
-                    QualType argPTy = Context->getPointerType(argTy);
-
-                    if (Info.checkStructuralEquality(V, RHSConstraints, argPTy, lhsType) &&
-                        Info.checkStructuralEquality(V, RHSConstraints, argPTy, rhsTy)) {
-                      rulesFired = true;
-                      // At present, I don't think we need to add an
-                      // implication based constraint since this rule only
-                      // fires if there is a cast from a call to malloc.
-                      // Since malloc is an external, there's no point in
-                      // adding constraints to it.
+            if (calleeDecl) {
+              // this is an allocator, should we treat it as safe?
+              if (!considerAllocUnsafe && isFunctionAllocator(calleeDecl->getName()))
+                rulesFired = true;
+              else if (calleeDecl->getName().equals("malloc")) {
+                // It's a call to malloc. What about the parameter to the call?
+                if (CA->getNumArgs() > 0) {
+                  UnaryExprOrTypeTraitExpr *arg =
+                    dyn_cast<UnaryExprOrTypeTraitExpr>(CA->getArg(0));
+                  if (arg && arg->isArgumentType()) {
+                    // Check that the argument is a sizeof.
+                    if (arg->getKind() == UETT_SizeOf) {
+                      QualType argTy = arg->getArgumentType();
+                      // argTy should be made a pointer, then compared for
+                      // equality to lhsType and rhsTy.
+                      QualType argPTy = Context->getPointerType(argTy);
+
+                      if (Info.checkStructuralEquality(V, RHSConstraints, argPTy, lhsType) &&
+                          Info.checkStructuralEquality(V, RHSConstraints, argPTy, rhsTy)) {
+                        rulesFired = true;
+                        // At present, I don't think we need to add an
+                        // implication based constraint since this rule
+                        // only fires if there is a cast from a call to malloc.
+                        // Since malloc is an external, there's no point in
+                        // adding constraints to it.
+                      }
                     }
                   }
                 }

clang/tools/checked-c-convert/ConstraintVariables.cpp

@@ -16,23 +16,15 @@
 
 using namespace clang;
 
-// Helper method to print a Type in a way that can be represented in the source.
-static
-std::string
-tyToStr(const Type *T) {
-  QualType QT(T, 0);
-
-  return QT.getAsString();
-}
-
 PointerVariableConstraint::PointerVariableConstraint(DeclaratorDecl *D,
                                                      ConstraintKey &K, Constraints &CS, const ASTContext &C) :
         PointerVariableConstraint(D->getType(), K, D, D->getName(), CS, C) { }
 
 PointerVariableConstraint::PointerVariableConstraint(const QualType &QT, ConstraintKey &K,
-                                                     DeclaratorDecl *D, std::string N, Constraints &CS, const ASTContext &C) :
+                                                     DeclaratorDecl *D, std::string N, Constraints &CS,
+                                                     const ASTContext &C, bool partOfFunc) :
         ConstraintVariable(ConstraintVariable::PointerVariable,
-                           tyToStr(QT.getTypePtr()),N),FV(nullptr)
+                           tyToStr(QT.getTypePtr()),N), partOFFuncPrototype(partOfFunc), FV(nullptr)
 {
   QualType QTy = QT;
   const Type *Ty = QTy.getTypePtr();
@@ -490,7 +482,7 @@ FunctionVariableConstraint::FunctionVariableConstraint(const Type *Ty,
       }
 
       std::set<ConstraintVariable*> C;
-      C.insert(new PVConstraint(QT, K, tmpD, paramName, CS, Ctx));
+      C.insert(new PVConstraint(QT, K, tmpD, paramName, CS, Ctx, true));
       paramVars.push_back(C);
     }
 
@@ -513,7 +505,7 @@ FunctionVariableConstraint::FunctionVariableConstraint(const Type *Ty,
   // as a type, then we will need the types for all the parameters and the
   // return values
 
-  returnVars.insert(new PVConstraint(returnType, K, D, "", CS, Ctx));
+  returnVars.insert(new PVConstraint(returnType, K, D, "", CS, Ctx, true));
   for ( const auto &V : returnVars) {
     if (PVConstraint *PVC = dyn_cast<PVConstraint>(V)) {
       if (PVC->getFV())

clang/tools/checked-c-convert/ConstraintVariables.h

@@ -157,12 +157,15 @@ class PointerVariableConstraint : public ConstraintVariable {
   // get the qualifier string (e.g., const, etc) for the provided constraint var (targetCvar)
   // into the provided string stream (ss)
   void getQualString(ConstraintKey targetCVar, std::ostringstream &ss);
+  // flag to indicate that this constraint is a part of function prototype
+  // e.g., Parameters or Return
+  bool partOFFuncPrototype;
 public:
   // Constructor for when we know a CVars and a type string.
   PointerVariableConstraint(CVars V, std::string T, std::string Name,
                             FunctionVariableConstraint *F, bool isArr, bool isItype, std::string is) :
           ConstraintVariable(PointerVariable, T, Name)
-          ,vars(V),FV(F),arrPresent(isArr), itypeStr(is) {}
+          ,vars(V),FV(F),arrPresent(isArr), itypeStr(is), partOFFuncPrototype(false) {}
 
   bool getArrPresent() { return arrPresent; }
 
@@ -179,7 +182,9 @@ class PointerVariableConstraint : public ConstraintVariable {
   // Constructor for when we only have a Type. Needs a string name
   // N for the name of the variable that this represents.
   PointerVariableConstraint(const clang::QualType &QT, ConstraintKey &K,
-                            clang::DeclaratorDecl *D, std::string N, Constraints &CS, const clang::ASTContext &C);
+                            clang::DeclaratorDecl *D, std::string N,
+                            Constraints &CS,
+                            const clang::ASTContext &C, bool partOfFunc = false);
 
   const CVars &getCvars() const { return vars; }
 
@@ -201,6 +206,8 @@ class PointerVariableConstraint : public ConstraintVariable {
   // get the highest type assigned to the cvars of this constraint variable
   ConstAtom *getHighestType(Constraints::EnvironmentMap &E);
 
+  bool isPartOfFunctionPrototype() const  { return partOFFuncPrototype; }
+
   bool isLt(const ConstraintVariable &other, ProgramInfo &P) const;
   bool isEq(const ConstraintVariable &other, ProgramInfo &P) const;
   bool isEmpty(void) const { return vars.size() == 0; }

clang/tools/checked-c-convert/PersistentSourceLoc.cpp

@@ -26,21 +26,25 @@ PersistentSourceLoc::mkPSL(const Decl *D, ASTContext &C) {
     SL = C.getSourceManager().getSpellingLoc(PV->getLocation());
   else if (const VarDecl *V = dyn_cast<VarDecl>(D))
     SL = C.getSourceManager().getExpansionLoc(V->getLocation());
-  
+
   return mkPSL(D->getSourceRange(), SL, C);
 }
 
 
 // Create a PersistentSourceLoc for a Stmt.
 PersistentSourceLoc
 PersistentSourceLoc::mkPSL(const Stmt *S, ASTContext &Context) {
-  return mkPSL(S->getSourceRange(), S->getBeginLoc(), Context);
+  return mkPSL(S->getSourceRange(),
+               S->getBeginLoc(),
+               Context);
 }
 
 // Use the PresumedLoc infrastructure to get the absolute file name, expansion
 // line and column number for a SourceLocation and corresponding SourceRange
-PersistentSourceLoc 
-PersistentSourceLoc::mkPSL(clang::SourceRange SR, SourceLocation SL, ASTContext &Context) {
+PersistentSourceLoc
+PersistentSourceLoc::mkPSL(clang::SourceRange SR,
+                           SourceLocation SL,
+                           ASTContext &Context) {
   SourceManager &SM = Context.getSourceManager();
   PresumedLoc PL = SM.getPresumedLoc(SL);
 

clang/tools/checked-c-convert/PersistentSourceLoc.h

@@ -55,16 +55,18 @@ class PersistentSourceLoc {
   void dump() const { print(llvm::errs()); }
 
   static
-    PersistentSourceLoc mkPSL(const clang::Decl *D, clang::ASTContext &Context);
+  PersistentSourceLoc mkPSL(const clang::Decl *D, clang::ASTContext &Context);
 
   static
-    PersistentSourceLoc mkPSL(const clang::Stmt *S, clang::ASTContext &Context);
+  PersistentSourceLoc mkPSL(const clang::Stmt *S, clang::ASTContext &Context);
 
 private:
   // Create a PersistentSourceLoc based on absolute file path
   // from the given SourceRange and SourceLocation.
   static
-    PersistentSourceLoc mkPSL(clang::SourceRange SR, clang::SourceLocation SL, clang::ASTContext &Context);
+  PersistentSourceLoc mkPSL(clang::SourceRange SR,
+                            clang::SourceLocation SL,
+                            clang::ASTContext &Context);
   std::string fileName;
   uint32_t lineNo;
   uint32_t colNo;

clang/tools/checked-c-convert/ProgramInfo.cpp

@@ -220,10 +220,7 @@ bool ProgramInfo::checkStructuralEquality(QualType D, QualType S) {
   if (D == S)
     return true;
 
-  if (D->isPointerType() == S->isPointerType())
-    return true;
-
-  return false;
+  return D->isPointerType() == S->isPointerType();
 }
 
 bool ProgramInfo::isExternOkay(std::string ext) {
@@ -729,6 +726,21 @@ ProgramInfo::getVariableHelper( Expr                            *E,
     T = getVariableHelper(CO->getRHS(), V, C, ifc);
     R.insert(T.begin(), T.end());
     return R;
+  } else if (StringLiteral *exr = dyn_cast<StringLiteral>(E)) {
+    // if this is a string literal. i.e., "foo"
+    // we create a new constraint variable and constrain it to be an Nt_array
+    std::set<ConstraintVariable *> T;
+    CVars V;
+    V.insert(freeKey);
+    CS.getOrCreateVar(freeKey);
+    freeKey++;
+    ConstraintVariable *newC = new PointerVariableConstraint(V, "const char*", exr->getBytes(),
+                                                             nullptr, false, false, "");
+    // constrain the newly created variable to be NTArray.
+    newC->constrainTo(CS, CS.getNTArr());
+    T.insert(newC);
+    return T;
+
   } else {
     return std::set<ConstraintVariable*>();
   }

clang/tools/checked-c-convert/ProgramInfo.h

@@ -109,9 +109,8 @@ class ProgramInfo {
 
   std::set<Decl *> &getIdentifiedArrayVars() {
 #ifdef ARRDEBUG
-    for(auto currD: IdentifiedArrayDecls) {
+    for (auto currD: IdentifiedArrayDecls)
       currD->dump();
-    }
 #endif
     return IdentifiedArrayDecls;
   }