Fixing the allocator issue, #8

Author: Machiry

tools/checked-c-convert/CheckedCConvert.cpp

@@ -85,6 +85,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"),

tools/checked-c-convert/ConstraintBuilder.cpp

@@ -285,27 +285,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 && isFunctionAllocator(calleeDecl->getName())) {
+              // this is an allocator, should we treat it as safe?
+              if(!considerAllocUnsafe) {
+                rulesFired = true;
+              } else {
+                // It's a call to allocator. 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.
+                      }
                     }
                   }
                 }

tools/checked-c-convert/Utils.cpp

@@ -181,6 +181,12 @@ bool functionHasVarArgs(clang::FunctionDecl *FD) {
   return false;
 }
 
+bool isFunctionAllocator(std::string funcName) {
+  return llvm::StringSwitch<bool>(funcName)
+    .Cases("malloc", "calloc", "realloc", true)
+    .Default(false);
+}
+
 float getTimeSpentInSeconds(clock_t startTime) {
   return float(clock() - startTime)/CLOCKS_PER_SEC;
 }

tools/checked-c-convert/Utils.h

@@ -29,6 +29,7 @@ extern llvm::cl::opt<bool> DumpIntermediate;
 extern llvm::cl::opt<bool> handleVARARGS;
 extern llvm::cl::opt<bool> mergeMultipleFuncDecls;
 extern llvm::cl::opt<bool> enablePropThruIType;
+extern llvm::cl::opt<bool> considerAllocUnsafe;
 
 const clang::Type *getNextTy(const clang::Type *Ty);
 
@@ -54,5 +55,8 @@ float getTimeSpentInSeconds(clock_t startTime);
 // check if the function has varargs i.e., foo(<named_arg>,...)
 bool functionHasVarArgs(clang::FunctionDecl *FD);
 
+// check if the function is a allocator.
+bool isFunctionAllocator(std::string funcName);
+
 clang::SourceLocation getFunctionDeclarationEnd(clang::FunctionDecl *FD, clang::SourceManager &S);
 #endif

tools/checked-c-convert/functests/regression/consider_allocs_safe.c

@@ -0,0 +1,10 @@
+#include <stdlib_checked.h>
+
+int main() {
+
+  char *ptr1 = NULL;
+
+  ptr1 = (char *) calloc(1, sizeof(char));
+
+  return 0;
+}

tools/checked-c-convert/functests/regression/consider_allocs_safe.expected.c

@@ -0,0 +1,10 @@
+#include <stdlib_checked.h>
+
+int main() {
+
+  _Ptr<char> ptr1 =  NULL;
+
+  ptr1 = (char *) calloc(1, sizeof(char));
+
+  return 0;
+}