Rewrite types in explict cast expressions

Author: john-h-kastner

clang/include/clang/CConv/ConstraintResolver.h

@@ -70,6 +70,7 @@ class ConstraintResolver {
   bool hasPersistentConstraints(clang::Expr *E);
   CVarSet getPersistentConstraints(clang::Expr *E);
   void storePersistentConstraints(clang::Expr *E, CVarSet &Vars);
+  PVConstraint *getRewritablePVConstraint(Expr *E);
 };
 
 #endif // _CONSTRAINTRESOLVER_H

clang/include/clang/CConv/ProgramInfo.h

@@ -110,6 +110,8 @@ class ProgramInfo {
   bool hasTypeParamBindings(CallExpr *CE, ASTContext *C);
   CallTypeParamBindingsT &getTypeParamBindings(CallExpr *CE, ASTContext *C);
 
+  void constrainWildIfMacro(CVarSet &S, SourceLocation Location);
+
 private:
   // List of all constraint variables, indexed by their location in the source.
   // This information persists across invocations of the constraint analysis
@@ -175,8 +177,6 @@ class ProgramInfo {
   //   or global)
   std::set<FVConstraint *> *getFuncFVConstraints(FunctionDecl *FD,
                                                  ASTContext *C);
-  void constrainWildIfMacro(CVarSet S,
-                            SourceLocation Location);
 };
 
 #endif

clang/lib/CConv/CastPlacement.cpp

@@ -26,7 +26,7 @@ using namespace clang;
 
 bool CastPlacementVisitor::VisitCallExpr(CallExpr *CE) {
   Decl *D = CE->getCalleeDecl();
-  if (D) {
+  if (Rewriter::isRewritable(CE->getExprLoc()) && D) {
     PersistentSourceLoc PL = PersistentSourceLoc::mkPSL(CE, *Context);
     if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
       // Get the constraint variable for the function.

clang/lib/CConv/ConstraintResolver.cpp

@@ -214,7 +214,10 @@ CVarSet
         return PVConstraintFromType(TypE);
       }
       // NULL
-    } else if (isNULLExpression(E, *Context)) {
+      // Special handling for casts of null is required to enable rewriting
+      // statements such as int *x = (int*) 0. If this was handled as a
+      // normal null expression, the cast would never be visited.
+    } else if (!isa<ExplicitCastExpr>(E) && isNULLExpression(E, *Context)) {
       return EmptyCSet;
       // Implicit cast, e.g., T* from T[] or int (*)(int) from int (int),
       //   but also weird int->int * conversions (and back)
@@ -254,14 +257,29 @@ CVarSet
       CVarSet Ret = EmptyCSet;
       if (ExplicitCastExpr *ECE = dyn_cast<ExplicitCastExpr>(E)) {
         assert(ECE->getType() == TypE);
-        // Is cast internally safe? Return WILD if not
         Expr *TmpE = ECE->getSubExpr();
-        if (TypE->isPointerType() && !isCastSafe(TypE, TmpE->getType()))
+        // Is cast internally safe? Return WILD if not.
+        // If the cast is NULL, it will otherwise seem invalid, but we want to
+        // handle it as usual so the type in the cast can be rewritten.
+        if (!isNULLExpression(ECE, *Context) && TypE->isPointerType()
+            && !isCastSafe(TypE, TmpE->getType())) {
           Ret = getInvalidCastPVCons(E);
           // NB: Expression ECE itself handled in
           // ConstraintBuilder::FunctionVisitor
-        else
-          Ret = getExprConstraintVars(TmpE);
+        } else {
+          CVarSet Vars = getExprConstraintVars(TmpE);
+          // PVConstraint introduced for explicit cast so they can be rewritten.
+          // Pretty much the same idea as CompoundLiteralExpr.
+          PVConstraint *P = getRewritablePVConstraint(ECE);
+          Ret = {P};
+          // ConstraintVars for TmpE when ECE is NULL will be WILD, so
+          // constraining GEQ these vars would be the cast always be WILD.
+          if (!isNULLExpression(ECE, *Context)) {
+            PersistentSourceLoc PL = PersistentSourceLoc::mkPSL(ECE, *Context);
+            constrainConsVarGeq(Ret, Vars, Info.getConstraints(), &PL,
+                                Same_to_Same, false, &Info);
+          }
+        }
       }
         // x = y, x+y, x+=y, etc.
       else if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
@@ -520,9 +538,7 @@ CVarSet
         CVarSet T;
         CVarSet Vars = getExprConstraintVars(CLE->getInitializer());
 
-        PVConstraint *P =
-            new PVConstraint(CLE->getType(), nullptr, CLE->getStmtClassName(),
-                             Info, *Context, nullptr);
+        PVConstraint *P = getRewritablePVConstraint(CLE);
         T = {P};
 
         PersistentSourceLoc PL = PersistentSourceLoc::mkPSL(CLE, *Context);
@@ -581,11 +597,12 @@ CVarSet
   return Persist;
 }
 
+
 void ConstraintResolver::storePersistentConstraints(clang::Expr *E,
                                                     CVarSet &Vars) {
-  auto PSL = PersistentSourceLoc::mkPSL(E, *Context);
   // Store only if the PSL is valid.
-  if (PSL.valid()) {
+  auto PSL = PersistentSourceLoc::mkPSL(E, *Context);
+  if (PSL.valid() && Rewriter::isRewritable(E->getBeginLoc())){
     CVarSet &Persist = Info.getPersistentConstraintVars(E, Context);
     Persist.insert(Vars.begin(), Vars.end());
   }
@@ -679,6 +696,18 @@ CVarSet ConstraintResolver::getBaseVarPVConstraint(DeclRefExpr *Decl) {
   return Ret;
 }
 
+// Construct a PVConstraint for an expression that can safely be used when
+// rewriting the expression later on. This is done by making the constraint WILD
+// if the expression is inside a macro.
+PVConstraint *ConstraintResolver::getRewritablePVConstraint(Expr *E) {
+  PVConstraint *P = new PVConstraint(E->getType(), nullptr,
+                                     E->getStmtClassName(), Info, *Context,
+                                     nullptr);
+  CVarSet Tmp = {P};
+  Info.constrainWildIfMacro(Tmp, E->getExprLoc());
+  return P;
+}
+
 bool
 ConstraintResolver::containsValidCons(CVarSet &CVs) {
   bool RetVal = false;

clang/lib/CConv/ProgramInfo.cpp

@@ -641,7 +641,7 @@ CVarSet
 // If it was, we should constrain it to top. This is sad. Hopefully,
 // someday, the Rewriter will become less lame and let us re-write stuff
 // in macros.
-void ProgramInfo::constrainWildIfMacro(CVarSet S,
+void ProgramInfo::constrainWildIfMacro(CVarSet &S,
                                        SourceLocation Location) {
   std::string Rsn = "Pointer in Macro declaration.";
   if (!Rewriter::isRewritable(Location))

clang/lib/CConv/RewriteUtils.cpp

@@ -783,45 +783,52 @@ static void emit(Rewriter &R, ASTContext &C, std::set<FileID> &Files,
 
 
 
-// Visit every array compound literal expression (i.e., (int*[2]){&a, &b}) and
-// replace the type name with a new type based on constraint solution.
-class CompoundLiteralRewriter
-    : public clang::RecursiveASTVisitor<CompoundLiteralRewriter> {
+// Rewrites types that inside other expressions. This includes cast expression
+// and compound literal expressions.
+class TypeExprRewriter
+    : public clang::RecursiveASTVisitor<TypeExprRewriter> {
 public:
-  explicit CompoundLiteralRewriter(ASTContext *C, ProgramInfo &I, Rewriter &R)
+  explicit TypeExprRewriter(ASTContext *C, ProgramInfo &I, Rewriter &R)
       : Context(C), Info(I) , Writer(R) {}
 
   bool VisitCompoundLiteralExpr(CompoundLiteralExpr *CLE) {
-    // When an compound literal was visited in constraint generation, a
-    // constraint variable for it was stored in program info.  There should be
-    // either zero or one of these.
-    CVarSet
-        CVSingleton = Info.getPersistentConstraintVars(CLE, Context);
+    SourceRange TypeSrcRange(CLE->getBeginLoc().getLocWithOffset(1),
+         CLE->getTypeSourceInfo()->getTypeLoc().getEndLoc());
+    rewriteType(CLE, TypeSrcRange);
+    return true;
+  }
+
+  bool VisitCStyleCastExpr(CStyleCastExpr *ECE) {
+    SourceRange TypeSrcRange
+        (ECE->getBeginLoc().getLocWithOffset(1),
+          ECE->getTypeInfoAsWritten()->getTypeLoc().getEndLoc());
+    rewriteType(ECE, TypeSrcRange);
+    return true;
+  }
+
+private:
+  ASTContext *Context;
+  ProgramInfo &Info;
+  Rewriter &Writer;
+
+  void rewriteType(Expr *E, SourceRange &Range) {
+    CVarSet CVSingleton = Info.getPersistentConstraintVars(E, Context);
     if (CVSingleton.empty())
-      return true;
+      return;
     ConstraintVariable *CV = getOnly(CVSingleton);
 
     // Only rewrite if the type has changed.
-    if(CV->anyChanges(Info.getConstraints().getVariables())){
+    if (CV->anyChanges(Info.getConstraints().getVariables())){
       // The constraint variable is able to tell us what the new type string
       // should be.
-      std::string NewType = CV->mkString(Info.getConstraints().getVariables(),false);
+      std::string
+          NewType = CV->mkString(Info.getConstraints().getVariables(), false);
 
       // Replace the original type with this new one
-      SourceRange *TypeSrcRange =
-          new SourceRange(CLE->getBeginLoc().getLocWithOffset(1),
-                          CLE->getTypeSourceInfo()->getTypeLoc().getEndLoc());
-
-      if(canRewrite(Writer, *TypeSrcRange))
-        Writer.ReplaceText(*TypeSrcRange, NewType);
+      if (canRewrite(Writer, Range))
+        Writer.ReplaceText(Range, NewType);
     }
-    return true;
   }
-
-private:
-  ASTContext *Context;
-  ProgramInfo &Info;
-  Rewriter &Writer;
 };
 
 // Adds type parameters to calls to alloc functions.
@@ -973,7 +980,7 @@ void RewriteConsumer::HandleTranslationUnit(ASTContext &Context) {
   CheckedRegionFinder CRF(&Context, R, Info, seen, nodeMap);
   CheckedRegionAdder CRA(&Context, R, nodeMap);
   CastPlacementVisitor ECPV(&Context, Info, R);
-  CompoundLiteralRewriter CLR(&Context, Info, R);
+  TypeExprRewriter TER(&Context, Info, R);
   TypeArgumentAdder TPA(&Context, Info, R);
   for (auto &D : TUD->decls()) {
     V.TraverseDecl(D);
@@ -986,7 +993,7 @@ void RewriteConsumer::HandleTranslationUnit(ASTContext &Context) {
     }
 
     GVG.addGlobalDecl(dyn_cast<VarDecl>(D));
-    CLR.TraverseDecl(D);
+    TER.TraverseDecl(D);
     TPA.TraverseDecl(D);
   }
 

clang/test/CheckedCRewriter/alloc_type_param.c

@@ -19,11 +19,11 @@ void foo() {
   // Explicit casts work fine also
 
   int *d = (int*) malloc(sizeof(int));
-  // CHECK: _Ptr<int> d = (int*) malloc<int>(sizeof(int));
+  // CHECK: _Ptr<int> d = (_Ptr<int> ) malloc<int>(sizeof(int));
   int *e = (int*) calloc(1, sizeof(int));
-  // CHECK: _Ptr<int> e = (int*) calloc<int>(1, sizeof(int));
+  // CHECK: _Ptr<int> e = (_Ptr<int> ) calloc<int>(1, sizeof(int));
   int *f = (int*) realloc(d, sizeof(int));
-  // CHECK: _Ptr<int> f = (int*) realloc<int>(d, sizeof(int));
+  // CHECK: _Ptr<int> f = (_Ptr<int> ) realloc<int>(d, sizeof(int));
 }
 
 // Allocating pointers to pointers

clang/test/CheckedCRewriter/allocator.c

@@ -48,4 +48,4 @@ void add_some_stuff(listhead *hd) {
   return;
 }
 //CHECK: void add_some_stuff(_Ptr<listhead>  hd) {
-//CHECK-NEXT: _Ptr<listelt>  l1 = (listelt *) malloc<listelt>(sizeof(listelt));
+//CHECK-NEXT: _Ptr<listelt>  l1 = (_Ptr<listelt> ) malloc<listelt>(sizeof(listelt));

clang/test/CheckedCRewriter/b10_allsafepointerstruct.c

@@ -51,7 +51,7 @@ struct p *foo() {
   x->y = &ex2;
   y->y = &ex1;
   struct p *z = (struct p *) sus(x, y);
-	//CHECK: _Ptr<struct p> z =  (struct p *) sus(x, y);
+	//CHECK: _Ptr<struct p> z =  (_Ptr<struct p> ) sus(x, y);
   return z;
 }
 
@@ -67,6 +67,6 @@ struct p *bar() {
   x->y = &ex2;
   y->y = &ex1;
   struct p *z = (struct p *) sus(x, y);
-	//CHECK: _Ptr<struct p> z =  (struct p *) sus(x, y);
+	//CHECK: _Ptr<struct p> z =  (_Ptr<struct p> ) sus(x, y);
   return z;
-}
+}

clang/test/CheckedCRewriter/b16_callerpointerstruct.c

@@ -54,7 +54,7 @@ struct p *foo() {
   x->y = &ex2;
   y->y = &ex1;
   struct p *z = (struct p *) sus(x, y);
-	//CHECK_NOALL: _Ptr<struct p> z =  (struct p *) sus(x, y);
+	//CHECK_NOALL: _Ptr<struct p> z =  (_Ptr<struct p> ) sus(x, y);
 	//CHECK_ALL:   struct p *z = (struct p *) sus(x, y);
   return z;
 }
@@ -74,4 +74,4 @@ struct p *bar() {
 	//CHECK: struct p *z = (struct p *) sus(x, y);
   z += 2;
   return z;
-}
+}