Improve TreeTransform for positional parameter expressions.

lib/Sema/CheckedCInterop.cpp

@@ -82,6 +82,7 @@ class TransformFunctionTypeToChecked :
 public:
   TransformFunctionTypeToChecked(Sema &SemaRef) : BaseTransform(SemaRef) {}
 
+  bool IsInstantiation() { return false; }
 
   QualType TransformFunctionProtoType(TypeLocBuilder &TLB,
                                       FunctionProtoTypeLoc TL) {
@@ -214,25 +215,25 @@ class TransformFunctionTypeToChecked :
     Sema::ExtParameterInfoBuilder ExtParamInfos;
     // ParamAnnotsStorage is pre-allocated storage that is used when updating EPI
     // in TransformExtendedParameterInfo.  Its lifetime must last until the end of
-    // the lifetimie of EPI.
+    // the lifetime of EPI.
     SmallVector<BoundsAnnotations, 4> ParamAnnotsStorage;
 
     QualType ResultType = getDerived().TransformType(TLB, ResultLoc);
     if (ResultType.isNull())
       return QualType();
 
-    // Places transformed data ParamTypes, ParamDecls, and ExtParamInfos.
+    // Places transformed data in ParamTypes, ParamDecls, and ExtParamInfos.
     if (getDerived().TransformFunctionTypeParams(
       TL.getBeginLoc(), TL.getParams(),
       CurrentParamTypes.begin(),
       T->getExtParameterInfosOrNull(),
       ParamTypes, &ParamDecls, ExtParamInfos))
       return QualType();
 
-    if (getDerived().TransformExtendedParameterInfo(EPI, ParamTypes, ParamAnnotsStorage,
+    if (getDerived().TransformExtendedParameterInfo(EPI, EPIChanged, ParamTypes,
+                                                    ParamAnnotsStorage,
                                                     ExtParamInfos, TL,
-                                                    TransformExceptionSpec,
-                                                    EPIChanged))
+                                                    TransformExceptionSpec))
       return QualType();
 
     // Rebuild the type if something changed.

lib/Sema/SemaExpr.cpp

@@ -1715,7 +1715,7 @@ Sema::BuildDeclRefExpr(ValueDecl *D, QualType Ty, ExprValueKind VK,
   return BuildDeclRefExpr(D, Ty, VK, NameInfo, SS);
 }
 
-/// BuildDeclRefExpr - Build an expression that references aB
+/// BuildDeclRefExpr - Build an expression that references a
 /// declaration that does not require a closure capture.
 ExprResult
 Sema::BuildDeclRefExpr(ValueDecl *D, QualType Ty, ExprValueKind VK,

lib/Sema/TreeTransform.h

@@ -124,6 +124,10 @@ class TreeTransform {
   llvm::DenseMap<CHKCBindTemporaryExpr *, CHKCBindTemporaryExpr *>
     TransformedTemporaries;
 
+  /// \brief The set of parameters that have been transformed.  Used
+  /// to update positional parameter expression type information.
+  llvm::SmallVector<QualType, 16> TransformedPositionalParameters;
+
 public:
   /// \brief Initializes a new tree transformer.
   TreeTransform(Sema &SemaRef) : SemaRef(SemaRef) { }
@@ -154,6 +158,12 @@ class TreeTransform {
   /// statement node appears at most once in its containing declaration.
   bool AlwaysRebuild() { return SemaRef.ArgumentPackSubstitutionIndex != -1; }
 
+  /// \brief Whether this use of TreeTransform is being used for template
+  /// instantiation.
+  ///
+  /// Subclasses may override this function when it isn't.
+  bool IsInstantiation() { return true; }
+
   /// \brief Returns the location of the entity being transformed, if that
   /// information was not available elsewhere in the AST.
   ///
@@ -654,12 +664,12 @@ class TreeTransform {
   /// The result vectors should be kept in sync; null entries in the
   /// variables vector are acceptable.
   ///
-  /// Inputs: Params, ParamTypes, and ParamInfos are the inputs to the
-  /// method.
+  /// Inputs: Params, ParamTypes, and ParamInfos.
   ///
-  /// Output: PTypes, PVars, and PInfos are the outputs of the method.
-  /// The updated parameter types, param var declarations, and PInfo
-  /// are stored in the method.
+  /// Outputs: PTypes, PVars, and PInfos:
+  /// - The updated parameter types are stored in PTypes.
+  /// - The updated parameter variable declarations are stored in PVars.
+  /// - The updated extend parameter info is stored in PInfos.
   ///
   /// For correctness, the inputs and outputs shoudl be disjoint data
   /// structures.
@@ -672,6 +682,8 @@ class TreeTransform {
       SmallVectorImpl<QualType> &PTypes, SmallVectorImpl<ParmVarDecl *> *PVars,
       Sema::ExtParameterInfoBuilder &PInfos);
 
+  /// Transform the bounds annotation Annot, updating Annot. Set Changed to
+  /// true if Annot changed.
   bool TransformBoundsAnnotations(BoundsAnnotations &Annot, bool &Changed);
 
   /// \brief Transform return bounds annotations.  We provide a separate method for
@@ -681,19 +693,36 @@ class TreeTransform {
   /// \brief Transform the extended parameter information for
   /// a function prototype.
   ///
-  /// Updates EPI with the transformed information.  Sets
-  /// EPIChanged to true if something changed, false otherwise.
+  /// \param EPI: The current extended parameter information. Updated
+  /// with transformed information.
   ///
-  /// Return true on error.
+  /// \param EPIChanged: set to true if something changed, left
+  /// unchanged otherwise.
+  ///
+  /// \param ParamTypes: the new parameter types.  Not changed.
+  ///
+
+  /// \param ParamListAnnots: pre-allocated for holding parameter list
+  /// annotations. Modified by method if there are parameter bounds
+  /// annotations.
+  ///
+  /// \param ExtParamInfo: external parameter info builder. May be modified
+  /// if the number of parameters has changed.
+  ///
+  /// \param TL: the existing type location information (before transformation)
+  ///
+  /// \param TransformExceptionSpec: transforms exception specification.
+  ///
+  /// Return true on error, false on success.
   template<typename Fn>
   bool TransformExtendedParameterInfo(
     FunctionProtoType::ExtProtoInfo &EPI,
-    SmallVector<QualType, 4> &ParamTypes,
+    bool &EPIChanged,
+    const SmallVector<QualType, 4> &ParamTypes,
     SmallVector<BoundsAnnotations, 4> &ParamListAnnots,
     Sema::ExtParameterInfoBuilder &ExtParamInfos,
-    FunctionProtoTypeLoc &TL,
-    Fn TransformExceptionSpec,
-    bool &EPIChanged);
+    const FunctionProtoTypeLoc &TL,
+    Fn TransformExceptionSpec);
 
   /// \brief Transforms a single function-type parameter.  Return null
   /// on error.
@@ -5331,12 +5360,12 @@ bool TreeTransform<Derived>::TransformReturnBoundsAnnotations(
 template <typename Derived> template<typename Fn>
 bool TreeTransform<Derived>::TransformExtendedParameterInfo(
   FunctionProtoType::ExtProtoInfo &EPI,
-  SmallVector<QualType, 4> &ParamTypes,
+  bool &EPIChanged,
+  const SmallVector<QualType, 4> &ParamTypes,
   SmallVector<BoundsAnnotations, 4> &ParamListAnnots,
   Sema::ExtParameterInfoBuilder &ExtParamInfos,
-  FunctionProtoTypeLoc &TL,
-  Fn TransformExceptionSpec,
-  bool &EPIChanged) {
+  const FunctionProtoTypeLoc &TL,
+  Fn TransformExceptionSpec) {
   EPIChanged = false;
   if (TransformExceptionSpec(EPI.ExceptionSpec, EPIChanged))
     return false;
@@ -5356,6 +5385,16 @@ bool TreeTransform<Derived>::TransformExtendedParameterInfo(
     EPI.ExtParameterInfos = nullptr;
   }
 
+  // Return now if there are no bounds annotations to process.
+  if (EPI.ReturnAnnots.IsEmpty() && !EPI.ParamAnnots)
+    return false;
+
+  // Set up transformed type information, for positional parameters (if there
+  // are any).
+  llvm::SmallVector<QualType, 16> ExistingInfo(TransformedPositionalParameters);
+  TransformedPositionalParameters.assign(ParamTypes.begin(),
+                                         ParamTypes.end());
+
   // Handle bounds annotations for return
   if (getDerived().TransformReturnBoundsAnnotations(EPI.ReturnAnnots, EPIChanged))
     return true;
@@ -5377,12 +5416,16 @@ bool TreeTransform<Derived>::TransformExtendedParameterInfo(
       }
       ParamListAnnots.push_back(ParamAnnotations);
     }
-
     if (ParamListAnnotsChanged) {
       EPIChanged = true;
       EPI.ParamAnnots = ParamListAnnots.data();
     }
   }
+
+  // Restore the transformed parameter information.
+  TransformedPositionalParameters.assign(ExistingInfo.begin(),
+                                         ExistingInfo.end());
+
   return false;
 }
 
@@ -5412,9 +5455,9 @@ QualType TreeTransform<Derived>::TransformFunctionProtoType(
   // parameters before the return type,  since the return type can then refer
   // to the parameters themselves (via decltype, sizeof, etc.).
   //
-  SmallVector<QualType, 4> ParamTypes;
-  SmallVector<ParmVarDecl*, 4> ParamDecls;
-  SmallVector<BoundsAnnotations, 4> ParamAnnots;
+  SmallVector<QualType, 4> ParamTypes;           // New parameter types.
+  SmallVector<ParmVarDecl*, 4> ParamDecls;       // New parameter declarations.
+  SmallVector<BoundsAnnotations, 4> ParamAnnots; // New parameter annotations.
   Sema::ExtParameterInfoBuilder ExtParamInfos;
   const FunctionProtoType *T = TL.getTypePtr();
 
@@ -5457,10 +5500,9 @@ QualType TreeTransform<Derived>::TransformFunctionProtoType(
 
   FunctionProtoType::ExtProtoInfo EPI = T->getExtProtoInfo();
   bool EPIChanged = false;
-  if (getDerived().TransformExtendedParameterInfo(EPI, ParamTypes, ParamAnnots,
+  if (getDerived().TransformExtendedParameterInfo(EPI, EPIChanged, ParamTypes, ParamAnnots,
                                                   ExtParamInfos, TL,
-                                                  TransformExceptionSpec,
-                                                  EPIChanged))
+                                                  TransformExceptionSpec))
     return QualType();
 
   QualType Result = TL.getType();
@@ -9101,8 +9143,8 @@ TreeTransform<Derived>::TransformDeclRefExpr(DeclRefExpr *E) {
       !E->hasExplicitTemplateArgs()) {
 
     // Mark it referenced in the new context regardless.
-    // FIXME: this is a bit instantiation-specific.
-    SemaRef.MarkDeclRefReferenced(E);
+    if (getDerived().IsInstantiation())
+      SemaRef.MarkDeclRefReferenced(E);
 
     return E;
   }
@@ -11006,7 +11048,8 @@ TreeTransform<Derived>::TransformCXXConstructExpr(CXXConstructExpr *E) {
       !ArgumentChanged) {
     // Mark the constructor as referenced.
     // FIXME: Instantiation-specific
-    SemaRef.MarkFunctionReferenced(E->getLocStart(), Constructor);
+    if (getDerived().IsInstantiation())
+      SemaRef.MarkFunctionReferenced(E->getLocStart(), Constructor);
     return E;
   }
 
@@ -11038,7 +11081,8 @@ ExprResult TreeTransform<Derived>::TransformCXXInheritedCtorInitExpr(
       Constructor == E->getConstructor()) {
     // Mark the constructor as referenced.
     // FIXME: Instantiation-specific
-    SemaRef.MarkFunctionReferenced(E->getLocStart(), Constructor);
+    if (getDerived().IsInstantiation())
+      SemaRef.MarkFunctionReferenced(E->getLocStart(), Constructor);
     return E;
   }
 
@@ -11116,7 +11160,8 @@ TreeTransform<Derived>::TransformCXXTemporaryObjectExpr(
       Constructor == E->getConstructor() &&
       !ArgumentChanged) {
     // FIXME: Instantiation-specific
-    SemaRef.MarkFunctionReferenced(E->getLocStart(), Constructor);
+    if (getDerived().IsInstantiation())
+      SemaRef.MarkFunctionReferenced(E->getLocStart(), Constructor);
     return SemaRef.MaybeBindToTemporary(E);
   }
 
@@ -12565,7 +12610,9 @@ ExprResult
 TreeTransform<Derived>::TransformPositionalParameterExpr(
   PositionalParameterExpr *E) {
   unsigned Index = E->getIndex();
-  QualType QT = getDerived().TransformType(E->getType());
+  assert(Index < TransformedPositionalParameters.size());
+  QualType QT = TransformedPositionalParameters[Index];
+
   if (!getDerived().AlwaysRebuild() && QT == E->getType())
     return E;
 

test/CheckedC/regression-cases/bug_484_rewrite_parameter_uses.c

@@ -0,0 +1,34 @@
+//
+// This is a regression test case for
+// https://github.com/Microsoft/checkedc-clang/issues/484
+//
+// In checked scopes, we rewrite function types with bounds-safe interfaces to
+// be fully checked. We need to rewrite declarations of parameters and uses of
+// parameters that have bounds-safe interfaces.
+//
+// The following example illustrates the problem, if we don't do the rewrite.
+// We define 3 C typedefs:
+// 1. The first one defines an unchecked function pointer with a bounds-safe
+// interface on a parameter.
+// 2. The second one defines the bounds-safe interface type for the first
+// typedef's function pointer type.
+// 3. The third one defines a fully checked version.
+//
+// If we don't rewrite the uses of parameters, the function types
+// don't match and we get a type checking error.
+// RUN: %clang -Wno-check-bounds-decls-checked-scope -c -o %t %s
+
+#pragma CHECKED_SCOPE ON
+
+typedef int (*callback_fn3)(int *a : count(n), int n);
+typedef  _Ptr<int (int *a : bounds(a, a + n), int n)> bsi_callback_fn3;
+typedef  _Ptr<int (_Array_ptr<int> a : bounds(a, a + n), int n)> checked_callback_fn3;
+
+_Checked callback_fn3 return_function_pointer(void) : itype(bsi_callback_fn3);
+
+_Checked void test_function_pointer_return(void) {
+   checked_callback_fn3 fn = return_function_pointer();
+}
+
+
+

test/CheckedC/static-checking/typechecking.c

@@ -69,8 +69,10 @@ f304(int i, _Ptr<int(_Array_ptr<void> : byte_count(i), _Array_ptr<void> : byte_c
                                                                                                // Bounds in a function type reference parameters or locals.
 void f305(int i) {
   int j = i;
-  _Ptr<_Array_ptr<int>(void) : count(i)> p = 0; // expected-error {{out-of-scope variable for bounds}}
-  _Ptr<_Array_ptr<int>(void) : count(j)> q = 0; // expected-error {{out-of-scope variable for bounds}}
+  _Ptr<_Array_ptr<int>(void) : count(i)> p = 0;    // expected-error {{out-of-scope variable for bounds}}
+  _Ptr<_Array_ptr<int>(int k) : count(i)> q = 0; ; // expected-error {{out-of-scope variable for bounds}}
+  _Ptr<_Array_ptr<int>(void) : count(j)> r = 0;    // expected-error {{out-of-scope variable for bounds}}
+
 }
 
 // Global variable bounds are OK.