Generate itype string from AST when extracting from source fails (#693)

Fixes an assertion failure (#594) when
attempting to extract the string representation of an itype expression from the
original source code if the itype expression is inside a macro. The string
representation for itypes in macros is now re-generated from the AST.

The fix in its current form leads to some unnecessary expansion of macros. This
is discussed in #694.

Author: john-h-kastner

clang/lib/3C/ConstraintVariables.cpp

@@ -280,7 +280,16 @@ PointerVariableConstraint::PointerVariableConstraint(
         SourceRange R = ITE->getSourceRange();
         if (R.isValid()) {
           ItypeStr = getSourceText(R, C);
-          assert(ItypeStr.size() > 0);
+        }
+
+        // ITE->isCompilerGenerated will be true when an itype expression is
+        // implied by a bounds expression on the declaration. When the itype
+        // expression is not generated by the compiler, but we failed to extract
+        // its string representation from the source, build the itype string
+        // from the AST.
+        if (!ITE->isCompilerGenerated() && ItypeStr.empty()) {
+          assert(!InteropType.getAsString().empty());
+          ItypeStr = "itype(" + InteropType.getAsString() + ")";
         }
       }
     }

clang/test/3C/macro_itype.c

@@ -0,0 +1,34 @@
+// RUN: rm -rf %t*
+// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s
+// RUN: 3c -base-dir=%S -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s
+// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null -
+// RUN: 3c -base-dir=%S -alltypes -output-dir=%t.checked %s --
+// RUN: 3c -base-dir=%t.checked -alltypes %t.checked/macro_itype.c -- | diff %t.checked/macro_itype.c -
+
+// Example encountered while converting libjpeg. This triggered an assertion
+// fail because the ItypeStr extracted from the source was empty.
+#define macro0 int *a : itype(_Ptr<int>);
+struct s { macro0 };
+//CHECK: struct s { macro0 };
+
+// Example from issue correctcomputation/checkedc-clang#594.
+#define PARAM_DECL_WITH_ITYPE int *p : itype(_Ptr<int>)
+void foo(PARAM_DECL_WITH_ITYPE);
+//CHECK: void foo(PARAM_DECL_WITH_ITYPE);
+
+// Just removing the assertion that failed on the above example caused this to
+// rewrite incorrectly. The ItypeStr would be left empty, so first parameter
+// would be rewritten to `int *b` even though the rewriter intended to give it
+// an itype. If the parameter was then passed a checked pointer, there would be
+// a Checked C compiler error. Ideally, 3C wouldn't need to change the
+// declaration of `b` at all (see issue correctcomputation/checkedc-clang#694).
+#define macro1 : itype(_Ptr<int>)
+void fn(int *b macro1, int *c) {
+//CHECK: void fn(int *b : itype(_Ptr<int>), _Ptr<int> c) {
+  b = 1;
+}
+void caller() {
+  int *e;
+  //CHECK: _Ptr<int> e = ((void *)0);
+  fn(e, 0);
+}

clang/test/3C/params_in_macro.c

@@ -11,14 +11,11 @@
 
 typedef double mydouble;
 
-// TODO: FunctionDeclBuilder::buildDeclVar should be able to handle an itype
-// here, but currently the PointerConstraintVariable constructor asserts when it
-// fails to retrieve the original source of the itype declaration.
 #define parms1 volatile mydouble d, void (*f)(void)
-#define parms2 int *const y : count(7), _Ptr<char> z
+#define parms2 int *const y : count(7), _Ptr<char> z, int *zz : itype(_Ptr<int>)
 
 void test(parms1, int *x, parms2) {}
-// CHECK: void test(volatile mydouble d, void (*f)(void), _Ptr<int> x, int *const y : count(7), _Ptr<char> z) {}
+// CHECK: void test(volatile mydouble d, void (*f)(void), _Ptr<int> x, int *const y : count(7), _Ptr<char> z, int *zz : itype(_Ptr<int>)) {}
 
 // Before the bug fix, we got:
-// void test(, , _Ptr<int> x, , ) {}
+// void test(, , _Ptr<int> x, , , ) {}