Add -Wcast-calling-convention to warn when casting away calling conve…

…ntions

Summary:
This only warns on casts of the address of a function defined in the
current TU. In this case, the fix is likely to be local and the warning
useful.

Here are some things we could experiment with in the future:
- Fire on declarations as well as definitions
- Limit the warning to non-void function prototypes
- Limit the warning to mismatches of caller and callee cleanup CCs

This warning is currently off by default while we study its usefulness.

Reviewers: thakis, rtrieu

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D17348

llvm-svn: 269116

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -6621,7 +6621,13 @@ def warn_cast_pointer_from_sel : Warning<
 def warn_function_def_in_objc_container : Warning<
   "function definition inside an Objective-C container is deprecated">,
   InGroup<FunctionDefInObjCContainer>;
-
+
+def warn_cast_calling_conv : Warning<
+  "cast between incompatible calling conventions '%0' and '%1'; "
+  "calls through this pointer may abort at runtime">,
+  InGroup<DiagGroup<"cast-calling-convention">>;
+def note_change_calling_conv_fixit : Note<
+  "consider defining %0 with the '%1' calling convention">;
 def warn_bad_function_cast : Warning<
   "cast from function call of type %0 to non-matching type %1">,
   InGroup<BadFunctionCast>, DefaultIgnore;

clang/lib/Sema/SemaCast.cpp

@@ -22,6 +22,7 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/Initialization.h"
 #include "llvm/ADT/SmallVector.h"
 #include <set>
@@ -1726,6 +1727,89 @@ static void DiagnoseCastOfObjCSEL(Sema &Self, const ExprResult &SrcExpr,
     }
 }
 
+/// Diagnose casts that change the calling convention of a pointer to a function
+/// defined in the current TU.
+static void DiagnoseCallingConvCast(Sema &Self, const ExprResult &SrcExpr,
+                                    QualType DstType, SourceRange OpRange) {
+  // Check if this cast would change the calling convention of a function
+  // pointer type.
+  QualType SrcType = SrcExpr.get()->getType();
+  if (Self.Context.hasSameType(SrcType, DstType) ||
+      !SrcType->isFunctionPointerType() || !DstType->isFunctionPointerType())
+    return;
+  const auto *SrcFTy =
+      SrcType->castAs<PointerType>()->getPointeeType()->castAs<FunctionType>();
+  const auto *DstFTy =
+      DstType->castAs<PointerType>()->getPointeeType()->castAs<FunctionType>();
+  CallingConv SrcCC = SrcFTy->getCallConv();
+  CallingConv DstCC = DstFTy->getCallConv();
+  if (SrcCC == DstCC)
+    return;
+
+  // We have a calling convention cast. Check if the source is a pointer to a
+  // known, specific function that has already been defined.
+  Expr *Src = SrcExpr.get()->IgnoreParenImpCasts();
+  if (auto *UO = dyn_cast<UnaryOperator>(Src))
+    if (UO->getOpcode() == UO_AddrOf)
+      Src = UO->getSubExpr()->IgnoreParenImpCasts();
+  auto *DRE = dyn_cast<DeclRefExpr>(Src);
+  if (!DRE)
+    return;
+  auto *FD = dyn_cast<FunctionDecl>(DRE->getDecl());
+  const FunctionDecl *Definition;
+  if (!FD || !FD->hasBody(Definition))
+    return;
+
+  // The source expression is a pointer to a known function defined in this TU.
+  // Diagnose this cast, as it is probably bad.
+  StringRef SrcCCName = FunctionType::getNameForCallConv(SrcCC);
+  StringRef DstCCName = FunctionType::getNameForCallConv(DstCC);
+  Self.Diag(OpRange.getBegin(), diag::warn_cast_calling_conv)
+      << SrcCCName << DstCCName << OpRange;
+
+  // The checks above are cheaper than checking if the diagnostic is enabled.
+  // However, it's worth checking if the warning is enabled before we construct
+  // a fixit.
+  if (Self.Diags.isIgnored(diag::warn_cast_calling_conv, OpRange.getBegin()))
+    return;
+
+  // Try to suggest a fixit to change the calling convention of the function
+  // whose address was taken. Try to use the latest macro for the convention.
+  // For example, users probably want to write "WINAPI" instead of "__stdcall"
+  // to match the Windows header declarations.
+  SourceLocation NameLoc = Definition->getNameInfo().getLoc();
+  Preprocessor &PP = Self.getPreprocessor();
+  SmallVector<TokenValue, 6> AttrTokens;
+  SmallString<64> CCAttrText;
+  llvm::raw_svector_ostream OS(CCAttrText);
+  if (Self.getLangOpts().MicrosoftExt) {
+    // __stdcall or __vectorcall
+    OS << "__" << DstCCName;
+    IdentifierInfo *II = PP.getIdentifierInfo(OS.str());
+    AttrTokens.push_back(II->isKeyword(Self.getLangOpts())
+                             ? TokenValue(II->getTokenID())
+                             : TokenValue(II));
+  } else {
+    // __attribute__((stdcall)) or __attribute__((vectorcall))
+    OS << "__attribute__((" << DstCCName << "))";
+    AttrTokens.push_back(tok::kw___attribute);
+    AttrTokens.push_back(tok::l_paren);
+    AttrTokens.push_back(tok::l_paren);
+    IdentifierInfo *II = PP.getIdentifierInfo(DstCCName);
+    AttrTokens.push_back(II->isKeyword(Self.getLangOpts())
+                             ? TokenValue(II->getTokenID())
+                             : TokenValue(II));
+    AttrTokens.push_back(tok::r_paren);
+    AttrTokens.push_back(tok::r_paren);
+  }
+  StringRef AttrSpelling = PP.getLastMacroWithSpelling(NameLoc, AttrTokens);
+  if (!AttrSpelling.empty())
+    CCAttrText = AttrSpelling;
+  OS << ' ';
+  Self.Diag(NameLoc, diag::note_change_calling_conv_fixit)
+      << FD << DstCCName << FixItHint::CreateInsertion(NameLoc, CCAttrText);
+}
+
 static void checkIntToPointerCast(bool CStyle, SourceLocation Loc,
                                   const Expr *SrcExpr, QualType DestType,
                                   Sema &Self) {
@@ -2042,7 +2126,9 @@ static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
   }
   if (CStyle)
     DiagnoseCastOfObjCSEL(Self, SrcExpr, DestType);
-
+
+  DiagnoseCallingConvCast(Self, SrcExpr, DestType, OpRange);
+
   // Not casting away constness, so the only remaining check is for compatible
   // pointer categories.
 
@@ -2461,6 +2547,7 @@ void CastOperation::CheckCStyleCast() {
   }
 
   DiagnoseCastOfObjCSEL(Self, SrcExpr, DestType);
+  DiagnoseCallingConvCast(Self, SrcExpr, DestType, OpRange);
   DiagnoseBadFunctionCast(Self, SrcExpr, DestType);
   Kind = Self.PrepareScalarCast(SrcExpr, DestType);
   if (SrcExpr.isInvalid())

clang/test/Sema/callingconv-cast.c

@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -Wcast-calling-convention -DMSVC -Wno-pointer-bool-conversion -verify -x c %s
+// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -Wcast-calling-convention -DMSVC -Wno-pointer-bool-conversion -verify -x c++ %s
+// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -Wcast-calling-convention -DMSVC -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s --check-prefix=MSFIXIT
+// RUN: %clang_cc1 -triple i686-pc-windows-gnu -Wcast-calling-convention -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s --check-prefix=GNUFIXIT
+
+// expected-note@+1 {{consider defining 'mismatched_before_winapi' with the 'stdcall' calling convention}}
+void mismatched_before_winapi(int x) {}
+
+#ifdef MSVC
+#define WINAPI __stdcall
+#else
+#define WINAPI __attribute__((stdcall))
+#endif
+
+// expected-note@+1 3 {{consider defining 'mismatched' with the 'stdcall' calling convention}}
+void mismatched(int x) {}
+
+typedef void (WINAPI *callback_t)(int);
+void take_callback(callback_t callback);
+
+int main() {
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  take_callback((callback_t)mismatched);
+
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  callback_t callback = (callback_t)mismatched; // warns
+  (void)callback;
+
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  callback = (callback_t)&mismatched; // warns
+
+  // No warning, just to show we don't drill through other kinds of unary operators.
+  callback = (callback_t)!mismatched;
+
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  callback = (callback_t)&mismatched_before_winapi; // warns
+
+  // Probably a bug, but we don't warn.
+  void (*callback2)(int) = mismatched;
+  take_callback((callback_t)callback2);
+
+  // Another way to suppress the warning.
+  take_callback((callback_t)(void*)mismatched);
+}
+
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{7:6-7:6}:"__stdcall "
+
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{7:6-7:6}:"__attribute__((stdcall)) "