r269116 - Add -Wcast-calling-convention to warn when casting away calling conventions

Reid Kleckner via cfe-commits cfe-commits at lists.llvm.org
Tue May 10 14:00:03 PDT 2016


Author: rnk
Date: Tue May 10 16:00:03 2016
New Revision: 269116

URL: http://llvm.org/viewvc/llvm-project?rev=269116&view=rev
Log:
Add -Wcast-calling-convention to warn when casting away calling conventions

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

Added:
    cfe/trunk/test/Sema/callingconv-cast.c
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaCast.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=269116&r1=269115&r2=269116&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 10 16:00:03 2016
@@ -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;

Modified: cfe/trunk/lib/Sema/SemaCast.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=269116&r1=269115&r2=269116&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCast.cpp Tue May 10 16:00:03 2016
@@ -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 &
     }
 }
 
+/// 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(
   }
   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())

Added: cfe/trunk/test/Sema/callingconv-cast.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/callingconv-cast.c?rev=269116&view=auto
==============================================================================
--- cfe/trunk/test/Sema/callingconv-cast.c (added)
+++ cfe/trunk/test/Sema/callingconv-cast.c Tue May 10 16:00:03 2016
@@ -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 at +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 at +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 at +1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  take_callback((callback_t)mismatched);
+
+  // expected-warning at +1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  callback_t callback = (callback_t)mismatched; // warns
+  (void)callback;
+
+  // expected-warning at +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 at +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)) "




More information about the cfe-commits mailing list