diff --git a/lib/Sema/CheckedCInterop.cpp b/lib/Sema/CheckedCInterop.cpp index 2453e8ef4a7f..9242af6b21dc 100644 --- a/lib/Sema/CheckedCInterop.cpp +++ b/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,14 +215,14 @@ 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 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(), @@ -229,10 +230,10 @@ class TransformFunctionTypeToChecked : 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. diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 735d29545ea4..daee4ae12559 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/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, diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h index 1739f4e544b8..cae851d6754f 100644 --- a/lib/Sema/TreeTransform.h +++ b/lib/Sema/TreeTransform.h @@ -124,6 +124,10 @@ class TreeTransform { llvm::DenseMap TransformedTemporaries; + /// \brief The set of parameters that have been transformed. Used + /// to update positional parameter expression type information. + llvm::SmallVector 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 &PTypes, SmallVectorImpl *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 bool TransformExtendedParameterInfo( FunctionProtoType::ExtProtoInfo &EPI, - SmallVector &ParamTypes, + bool &EPIChanged, + const SmallVector &ParamTypes, SmallVector &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::TransformReturnBoundsAnnotations( template template bool TreeTransform::TransformExtendedParameterInfo( FunctionProtoType::ExtProtoInfo &EPI, - SmallVector &ParamTypes, + bool &EPIChanged, + const SmallVector &ParamTypes, SmallVector &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::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 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::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::TransformFunctionProtoType( // parameters before the return type, since the return type can then refer // to the parameters themselves (via decltype, sizeof, etc.). // - SmallVector ParamTypes; - SmallVector ParamDecls; - SmallVector ParamAnnots; + SmallVector ParamTypes; // New parameter types. + SmallVector ParamDecls; // New parameter declarations. + SmallVector ParamAnnots; // New parameter annotations. Sema::ExtParameterInfoBuilder ExtParamInfos; const FunctionProtoType *T = TL.getTypePtr(); @@ -5457,10 +5500,9 @@ QualType TreeTransform::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::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::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::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::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::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; diff --git a/test/CheckedC/regression-cases/bug_484_rewrite_parameter_uses.c b/test/CheckedC/regression-cases/bug_484_rewrite_parameter_uses.c new file mode 100644 index 000000000000..434a06dff03d --- /dev/null +++ b/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 bsi_callback_fn3; +typedef _Ptr 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(); +} + + + diff --git a/test/CheckedC/static-checking/typechecking.c b/test/CheckedC/static-checking/typechecking.c index 30a59f330450..81a9acb1d402 100644 --- a/test/CheckedC/static-checking/typechecking.c +++ b/test/CheckedC/static-checking/typechecking.c @@ -69,8 +69,10 @@ f304(int i, _Ptr : byte_count(i), _Array_ptr : byte_c // Bounds in a function type reference parameters or locals. void f305(int i) { int j = i; - _Ptr<_Array_ptr(void) : count(i)> p = 0; // expected-error {{out-of-scope variable for bounds}} - _Ptr<_Array_ptr(void) : count(j)> q = 0; // expected-error {{out-of-scope variable for bounds}} + _Ptr<_Array_ptr(void) : count(i)> p = 0; // expected-error {{out-of-scope variable for bounds}} + _Ptr<_Array_ptr(int k) : count(i)> q = 0; ; // expected-error {{out-of-scope variable for bounds}} + _Ptr<_Array_ptr(void) : count(j)> r = 0; // expected-error {{out-of-scope variable for bounds}} + } // Global variable bounds are OK.