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

Reply via email to