Initial fix for #255

Author: john-h-kastner

clang/include/clang/CConv/RewriteUtils.h

@@ -28,7 +28,7 @@ class DeclReplacement {
 
   std::string getReplacement() const { return Replacement; }
 
-  virtual SourceRange getSourceRange() const {
+  virtual SourceRange getSourceRange(SourceManager &SM) const {
     return getDecl()->getSourceRange();
   }
 
@@ -93,10 +93,13 @@ class FunctionDeclReplacement :
            && (RewriteReturn || RewriteParams));
   }
 
-  SourceRange getSourceRange() const override {
-    FunctionTypeLoc TypeLoc =
-        getBaseTypeLoc(Decl->getTypeSourceInfo()->getTypeLoc())
-        .getAs<clang::FunctionTypeLoc>();
+  SourceRange getSourceRange(SourceManager &SM) const override {
+    TypeSourceInfo *TSInfo = Decl->getTypeSourceInfo();
+    if (!TSInfo)
+      return SourceRange(Decl->getBeginLoc(),
+                         getFunctionDeclarationEnd(Decl, SM));
+    FunctionTypeLoc TypeLoc = getBaseTypeLoc(TSInfo->getTypeLoc())
+                             .getAs<clang::FunctionTypeLoc>();
 
     assert("FunctionDecl doesn't have function type?" && !TypeLoc.isNull());
 

clang/lib/CConv/ConstraintVariables.cpp

@@ -375,7 +375,8 @@ PointerVariableConstraint::PointerVariableConstraint(const QualType &QT,
         // inside a macro. Not sure how to handle this, so fall back to tyToStr.
         if (BaseType.empty())
           FoundMatchingType = false;
-      }
+      } else
+        FoundMatchingType = false;
     }
   }
   // Fall back to rebuilding the base type based on type passed to constructor

clang/lib/CConv/DeclRewriter.cpp

@@ -136,10 +136,6 @@ void DeclRewriter::rewrite(RSet &ToRewrite) {
       errs() << "with " << N->getReplacement() << "\n";
     }
 
-    // Get a FullSourceLoc for the start location and add it to the
-    // list of file ID's we've touched.
-    SourceRange SR = N->getDecl()->getSourceRange();
-
     // Exact rewriting procedure depends on declaration type
     if (auto *PVR = dyn_cast<ParmVarDeclReplacement>(N)) {
       assert(N->getStatement() == nullptr);
@@ -337,7 +333,7 @@ void DeclRewriter::rewriteFunctionDecl(FunctionDeclReplacement *N) {
   //       Additionally, a source range can be (mis) identified as
   //       spanning multiple files. We don't know how to re-write that,
   //       so don't.
-  SourceRange SR = N->getSourceRange();
+  SourceRange SR = N->getSourceRange(A.getSourceManager());
   if (canRewrite(R, SR)) {
     R.ReplaceText(SR, N->getReplacement());
   } else {

clang/lib/CConv/PersistentSourceLoc.cpp

@@ -23,9 +23,9 @@ PersistentSourceLoc::mkPSL(const Decl *D, ASTContext &C) {
   SourceLocation SL = D->getLocation();
 
   if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) 
-    SL = C.getSourceManager().getSpellingLoc(FD->getLocation());
+    SL = C.getSourceManager().getExpansionLoc(FD->getLocation());
   else if (const ParmVarDecl *PV = dyn_cast<ParmVarDecl>(D)) 
-    SL = C.getSourceManager().getSpellingLoc(PV->getLocation());
+    SL = C.getSourceManager().getExpansionLoc(PV->getLocation());
   else if (const VarDecl *V = dyn_cast<VarDecl>(D))
     SL = C.getSourceManager().getExpansionLoc(V->getLocation());
 

clang/lib/CConv/ProgramInfo.cpp

@@ -530,10 +530,13 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D,
   PersistentSourceLoc PLoc = PersistentSourceLoc::mkPSL(D, *astContext);
   assert(PLoc.valid());
 
-  // We only add a PVConstraint or an FVConstraint if the set at
-  // Variables[PLoc] does not contain one already. TODO: Explain why would this happen
+  // We only add a PVConstraint if the set at Variables[PLoc] does not contain
+  // one already. Two variables can have the same source locations when they are
+  // declared inside the same macro expansion. Functions are exempt from this
+  // check because they need to be added to the Extern/Static function maps
+  // regardless if they are inside a macro expansion.
   CVarSet &S = Variables[PLoc];
-  if (S.size()) return;
+  if (S.size() && !isa<FunctionDecl>(D)) return;
 
   if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
     // Function Decls have FVConstraints.

clang/lib/CConv/RewriteUtils.cpp

@@ -31,7 +31,7 @@ SourceLocation DComp::getDeclBegin(DeclReplacement *D) const {
 }
 
 SourceRange DComp::getReplacementSourceRange(DeclReplacement *D) const {
-  SourceRange Range = D->getSourceRange();
+  SourceRange Range = D->getSourceRange(SM);
 
   // Also take into account whether or not there is a multi-statement
   // decl, because the generated ranges will overlap.
@@ -227,19 +227,28 @@ class TypeExprRewriter
     CVarSet CVSingleton = Info.getPersistentConstraintVars(E, Context);
     if (CVSingleton.empty())
       return;
-    ConstraintVariable *CV = getOnly(CVSingleton);
-
-    // Only rewrite if the type has changed.
-    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);
-
-      // Replace the original type with this new one
-      if (canRewrite(Writer, Range))
-        Writer.ReplaceText(Range, NewType);
-    }
+
+    // Macros wil sometimes cause a single expression to have multiple
+    // constraint variables. These should have been constrained to wild, so
+    // there shouldn't be any rewriting required.
+    EnvironmentMap &Vars = Info.getConstraints().getVariables();
+    assert(CVSingleton.size() == 1 || llvm::none_of(CVSingleton,
+      [&Vars](ConstraintVariable *CV) {
+        return CV->anyChanges(Vars);
+      }));
+
+    for (auto *CV : CVSingleton)
+      // Only rewrite if the type has changed.
+      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);
+
+        // Replace the original type with this new one
+        if (canRewrite(Writer, Range))
+          Writer.ReplaceText(Range, NewType);
+      }
   }
 };
 

clang/test/CheckedCRewriter/definedType.c

@@ -110,3 +110,31 @@ baz (*(*lua_test3));
 typedef int *StkId;
 void lua_test4(StkId *x) {}
 // CHECK: void lua_test4(_Ptr<_Ptr<int>> x) _Checked {}
+
+// Things declared inside macros should be WILD unless we start doing something extremely clever
+
+#define declare_function(x) int *foo##x(int *a) {return a; } int *bar##x(int *a) { return a; }
+
+declare_function(1)
+
+#define declare_var(x) int * x##1; int ** x##2; int *** x##3;
+
+declare_var(y)
+
+void test() {
+  int *x = 0;
+  int *y = 0;
+  int *a = foo1(x);
+  int *b = bar1(y);
+  int *c = y1;
+  int **d = y2;
+  int ***e = y3;
+}
+// CHECK: void test() {
+// CHECK: int *x = 0;
+// CHECK: int *y = 0;
+// CHECK: int *a = foo1(x);
+// CHECK: int *b = bar1(y);
+// CHECK: int *c = y1;
+// CHECK: int **d = y2;
+// CHECK: int ***e = y3;

clang/test/CheckedCRewriter/macroConcat.c

@@ -0,0 +1,21 @@
+// RUN: cconv-standalone -addcr -alltypes %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s
+// RUN: cconv-standalone -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s
+// RUN: cconv-standalone -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null -
+// RUN: cconv-standalone -alltypes -output-postfix=checked %s
+// RUN: cconv-standalone -alltypes %S/macroConcat.checked.c -- | count 0
+// RUN: rm %S/macroConcat.checked.c
+
+#define c(g) void FOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO##g () 
+c(BAR0); c(BAR1); c(BAR2); c(BAR3); c(BAR4); c(BAR5); c(BAR6); c(BAR7);
+c(BAR8); c(BAR9); c(BAR10); c(BAR11); c(BAR12); c(BAR13); c(BAR14); c(BAR15); 
+c(BAR16); c(BAR17); c(BAR18); c(BAR19); c(BAR20); c(BAR21); c(BAR22); c(BAR23);
+c(BAR24); c(BAR25); c(BAR26); c(BAR27); c(BAR28); c(BAR29); c(BAR30); c(BAR31);
+c(BAR32); c(BAR33); c(BAR34); c(BAR35); c(BAR36); c(BAR37); c(BAR38);
+
+#define u(d) void add##d(g) {}
+// CHECK: #define u(d) void add##d(g) {}
+u(2);
+// CHECK: u(2);
+
+int *a = 0;
+// CHECK: _Ptr<int> a = 0;