aaron.ballman updated this revision to Diff 82930. aaron.ballman added a comment.
I've updated this patch to correct the codegen changes. If you'd prefer, I can review/commit in two separate phases, one for the codegen bug and one for the attribute issue, as I think these changes might be reasonably considered separately despite the gating. https://reviews.llvm.org/D28166 Files: include/clang/AST/Type.h include/clang/AST/TypeLoc.h lib/CodeGen/CGCall.cpp lib/CodeGen/CodeGenFunction.h lib/Sema/SemaDecl.cpp test/CodeGen/ms_abi.c test/Sema/knr-def-call.c test/Sema/warn-strict-prototypes.c
Index: test/Sema/warn-strict-prototypes.c =================================================================== --- test/Sema/warn-strict-prototypes.c +++ test/Sema/warn-strict-prototypes.c @@ -60,3 +60,8 @@ // K&R function definition with previous prototype declared is not diagnosed. void foo11(int p, int p2); void foo11(p, p2) int p; int p2; {} + +// PR31020 +void __attribute__((cdecl)) foo12(d) // expected-warning {{this old-style function definition is not preceded by a prototype}} + short d; +{} Index: test/Sema/knr-def-call.c =================================================================== --- test/Sema/knr-def-call.c +++ test/Sema/knr-def-call.c @@ -39,3 +39,9 @@ proto(42.1); // expected-warning{{implicit conversion from 'double' to 'int' changes value from 42.1 to 42}} (&proto)(42.1); // expected-warning{{implicit conversion from 'double' to 'int' changes value from 42.1 to 42}} } + +// PR31020 +void func(short d) __attribute__((cdecl)); // expected-note{{previous declaration is here}} +void __attribute__((cdecl)) func(d) + short d; // expected-warning{{promoted type 'int' of K&R function parameter is not compatible with the parameter type 'short' declared in a previous prototype}} +{} Index: test/CodeGen/ms_abi.c =================================================================== --- test/CodeGen/ms_abi.c +++ test/CodeGen/ms_abi.c @@ -146,3 +146,18 @@ // WIN64: %[[AP_VAL:.*]] = load i8*, i8** %[[AP]] // WIN64-NEXT: store i8* %[[AP_VAL]], i8** %[[AP2:.*]] } + +// Ensure that K&R C function types are not treated the same as a varargs call +// when it comes to the null pointer constant value 0. For a varargs call, this +// should be widened to a pointer-sized type, and for a K&R C call, it should +// remain an int. +void f7(int n) {} +void(*f7_ptr)() = f7; + +void f8(void) { + f7_ptr(0); + // WIN64: %callee.knr.cast = bitcast void (...)* %0 to void (i32)* + // WIN64-NEXT: call void %callee.knr.cast(i32 0) + // FREEBSD: %callee.knr.cast = bitcast void (...)* %0 to void (i32, ...)* + // FREEBSD-NEXT: call void (i32, ...) %callee.knr.cast(i32 0) +} Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -7458,11 +7458,12 @@ // Determine whether the function was written with a // prototype. This true when: // - there is a prototype in the declarator, or - // - the type R of the function is some kind of typedef or other reference - // to a type name (which eventually refers to a function type). + // - the type R of the function is some kind of typedef or other non- + // attributed reference to a type name (which eventually refers to a + // function type). bool HasPrototype = (D.isFunctionDeclarator() && D.getFunctionTypeInfo().hasPrototype) || - (!isa<FunctionType>(R.getTypePtr()) && R->isFunctionProtoType()); + (!R->getAsAdjusted<FunctionType>() && R->isFunctionProtoType()); NewFD = FunctionDecl::Create(SemaRef.Context, DC, D.getLocStart(), NameInfo, R, @@ -11950,7 +11951,7 @@ !LangOpts.CPlusPlus) { TypeSourceInfo *TI = FD->getTypeSourceInfo(); TypeLoc TL = TI->getTypeLoc(); - FunctionTypeLoc FTL = TL.castAs<FunctionTypeLoc>(); + FunctionTypeLoc FTL = TL.getAsAdjusted<FunctionTypeLoc>(); Diag(FTL.getLParenLoc(), diag::warn_strict_prototypes) << 1; } } Index: lib/CodeGen/CodeGenFunction.h =================================================================== --- lib/CodeGen/CodeGenFunction.h +++ lib/CodeGen/CodeGenFunction.h @@ -3567,7 +3567,7 @@ // If we still have any arguments, emit them using the type of the argument. for (auto *A : llvm::make_range(Arg, ArgRange.end())) - ArgTypes.push_back(getVarArgType(A)); + ArgTypes.push_back(getVarArgType(A, CallArgTypeInfo == nullptr)); EmitCallArgs(Args, ArgTypes, ArgRange, CalleeDecl, ParamsToSkip, Order); } @@ -3605,7 +3605,7 @@ void EmitSanitizerStatReport(llvm::SanitizerStatKind SSK); private: - QualType getVarArgType(const Expr *Arg); + QualType getVarArgType(const Expr *Arg, bool KNRFunction); const TargetCodeGenInfo &getTargetHooks() const { return CGM.getTargetCodeGenInfo(); Index: lib/CodeGen/CGCall.cpp =================================================================== --- lib/CodeGen/CGCall.cpp +++ lib/CodeGen/CGCall.cpp @@ -3383,14 +3383,14 @@ args.add(EmitAnyExprToTemp(E), type); } -QualType CodeGenFunction::getVarArgType(const Expr *Arg) { +QualType CodeGenFunction::getVarArgType(const Expr *Arg, bool KNRFunction) { // System headers on Windows define NULL to 0 instead of 0LL on Win64. MSVC // implicitly widens null pointer constants that are arguments to varargs // functions to pointer-sized ints. if (!getTarget().getTriple().isOSWindows()) return Arg->getType(); - if (Arg->getType()->isIntegerType() && + if (!KNRFunction && Arg->getType()->isIntegerType() && getContext().getTypeSize(Arg->getType()) < getContext().getTargetInfo().getPointerWidth(0) && Arg->isNullPointerConstant(getContext(), Index: include/clang/AST/TypeLoc.h =================================================================== --- include/clang/AST/TypeLoc.h +++ include/clang/AST/TypeLoc.h @@ -70,6 +70,26 @@ return t; } + /// \brief Convert to the specified TypeLoc type, returning a null TypeLoc if + /// this TypeLock is not of the desired type. It will consider type + /// adjustments from a type that wad written as a T to another type that is + /// still canonically a T (ignores parens, attributes, elaborated types, etc). + template <typename T> + T getAsAdjusted() const { + TypeLoc Cur = *this; + while (!T::isKind(Cur)) { + if (auto PTL = Cur.getAs<ParenTypeLoc>()) + Cur = PTL.getInnerLoc(); + else if (auto ATL = Cur.getAs<AttributedTypeLoc>()) + Cur = ATL.getModifiedLoc(); + else if (auto ETL = Cur.getAs<ElaboratedTypeLoc>()) + Cur = ETL.getNamedTypeLoc(); + else + break; + } + return Cur.getAs<T>(); + } + /// The kinds of TypeLocs. Equivalent to the Type::TypeClass enum, /// except it also defines a Qualified enum that corresponds to the /// QualifiedLoc class. Index: include/clang/AST/Type.h =================================================================== --- include/clang/AST/Type.h +++ include/clang/AST/Type.h @@ -1875,6 +1875,13 @@ /// immediately following this class. template <typename T> const T *getAs() const; + /// Member-template getAsAdjusted<specific type>. Look through specific kinds + /// of sugar (parens, attributes, etc) for an instance of \<specific type>. + /// This is used when you need to walk over sugar nodes that represent some + /// kind of type adjustment from a type that was written as a \<specific type> + /// to another type that is still canonically a \<specific type>. + template <typename T> const T *getAsAdjusted() const; + /// A variant of getAs<> for array types which silently discards /// qualifiers from the outermost type. const ArrayType *getAsArrayTypeUnsafe() const; @@ -5932,6 +5939,36 @@ return cast<T>(getUnqualifiedDesugaredType()); } +template <typename T> const T *Type::getAsAdjusted() const { + static_assert(!TypeIsArrayType<T>::value, "ArrayType cannot be used with getAsAdjusted!"); + + // If this is directly a T type, return it. + if (const T *Ty = dyn_cast<T>(this)) + return Ty; + + // If the canonical form of this type isn't the right kind, reject it. + if (!isa<T>(CanonicalType)) + return nullptr; + + // Strip off type adjustments that do not modify the underlying nature of the + // type. + const Type *Ty = this; + while (Ty) { + if (const auto *A = dyn_cast<AttributedType>(Ty)) + Ty = A->getModifiedType().getTypePtr(); + else if (const auto *E = dyn_cast<ElaboratedType>(Ty)) + Ty = E->desugar().getTypePtr(); + else if (const auto *P = dyn_cast<ParenType>(Ty)) + Ty = P->desugar().getTypePtr(); + else + break; + } + + // Just because the canonical type is correct does not mean we can use cast<>, + // since we may not have stripped off all the sugar down to the base type. + return dyn_cast<T>(Ty); +} + inline const ArrayType *Type::getAsArrayTypeUnsafe() const { // If this is directly an array type, return it. if (const ArrayType *arr = dyn_cast<ArrayType>(this))
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits