Silence -Wcast-function-type warnings on idiomatic Windows code (#135660)

On Windows, GetProcAddress() is the API used to dynamically load
function pointers (similar to dlsym on Linux). This API returns a
function pointer (a typedef named FARPROC), which means that casting
from the call to the eventual correct type is technically a function
type mismatch on the cast. However, because this is idiomatic code on
Windows, we should accept it unless -Wcast-function-type-strict is
passed.

This was brought up in post-commit review feedback on
llvm/llvm-project#86131

Author: AaronBallman

clang/docs/ReleaseNotes.rst

@@ -358,6 +358,16 @@ Improvements to Clang's diagnostics
 
 - Now correctly diagnose a tentative definition of an array with static
   storage duration in pedantic mode in C. (#GH50661)
+- No longer diagnosing idiomatic function pointer casts on Windows under
+  ``-Wcast-function-type-mismatch`` (which is enabled by ``-Wextra``). Clang
+  would previously warn on this construct, but will no longer do so on Windows:
+
+  .. code-block:: c
+
+    typedef void (WINAPI *PGNSI)(LPSYSTEM_INFO);
+    HMODULE Lib = LoadLibrary("kernel32");
+    PGNSI FnPtr = (PGNSI)GetProcAddress(Lib, "GetNativeSystemInfo");
+
 
 - An error is now emitted when a ``musttail`` call is made to a function marked with the ``not_tail_called`` attribute. (#GH133509).
 

clang/lib/Sema/SemaCast.cpp

@@ -1153,10 +1153,33 @@ static unsigned int checkCastFunctionType(Sema &Self, const ExprResult &SrcExpr,
     return false;
   };
 
+  auto IsFarProc = [](const FunctionType *T) {
+    // The definition of FARPROC depends on the platform in terms of its return
+    // type, which could be int, or long long, etc. We'll look for a source
+    // signature for: <integer type> (*)() and call that "close enough" to
+    // FARPROC to be sufficient to silence the diagnostic. This is similar to
+    // how we allow casts between function pointers and void * for supporting
+    // dlsym.
+    // Note: we could check for __stdcall on the function pointer as well, but
+    // that seems like splitting hairs.
+    if (!T->getReturnType()->isIntegerType())
+      return false;
+    if (const auto *PT = T->getAs<FunctionProtoType>())
+      return !PT->isVariadic() && PT->getNumParams() == 0;
+    return true;
+  };
+
   // Skip if either function type is void(*)(void)
   if (IsVoidVoid(SrcFTy) || IsVoidVoid(DstFTy))
     return 0;
 
+  // On Windows, GetProcAddress() returns a FARPROC, which is a typedef for a
+  // function pointer type (with no prototype, in C). We don't want to diagnose
+  // this case so we don't diagnose idiomatic code on Windows.
+  if (Self.getASTContext().getTargetInfo().getTriple().isOSWindows() &&
+      IsFarProc(SrcFTy))
+    return 0;
+
   // Check return type.
   if (!argTypeIsABIEquivalent(SrcFTy->getReturnType(), DstFTy->getReturnType(),
                               Self.Context))

clang/test/Sema/warn-cast-function-type-win.c

@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 %s -triple x86_64-windows -fsyntax-only -Wcast-function-type -Wno-cast-function-type-strict -verify=windows
+// RUN: %clang_cc1 %s -triple x86_64-windows -fsyntax-only -Wcast-function-type -Wno-cast-function-type-strict -x c++ -verify=windows
+// RUN: %clang_cc1 %s -triple x86_64-pc-linux -fsyntax-only -Wcast-function-type -Wno-cast-function-type-strict -verify=linux
+// RUN: %clang_cc1 %s -triple x86_64-pc-linux -fsyntax-only -Wcast-function-type -Wno-cast-function-type-strict -x c++ -verify=linux,linux-cpp
+// RUN: %clang_cc1 %s -triple x86_64-windows -fsyntax-only -Wcast-function-type -Wcast-function-type-strict -x c++ -verify=strict
+// windows-no-diagnostics
+
+// On Windows targets, this is expected to compile fine, and on non-Windows
+// targets, this should diagnose the mismatch. This is to allow for idiomatic
+// use of GetProcAddress, similar to what we do for dlsym. On non-Windows
+// targets, this should be diagnosed.
+typedef int (*FARPROC1)();
+typedef unsigned long long (*FARPROC2)();
+
+FARPROC1 GetProcAddress1(void);
+FARPROC2 GetProcAddress2(void);
+
+typedef int (*test1_type)(int);
+typedef float(*test2_type)();
+
+void test(void) {
+  // This does not diagnose on Linux in C mode because FARPROC1 has a matching
+  // return type to test1_type, but FARPROC1 has no prototype and so checking
+  // is disabled for further compatibility issues. In C++ mode, all functions
+  // have a prototype and so the check happens.
+  test1_type t1 = (test1_type)GetProcAddress1();
+  // linux-cpp-warning@-1 {{cast from 'FARPROC1' (aka 'int (*)()') to 'test1_type' (aka 'int (*)(int)') converts to incompatible function type}}
+  // strict-warning@-2 {{cast from 'FARPROC1' (aka 'int (*)()') to 'test1_type' (aka 'int (*)(int)') converts to incompatible function type}}
+  
+  // This case is diagnosed in both C and C++ modes on Linux because the return
+  // type of FARPROC2 does not match the return type of test2_type.
+  test2_type t2 = (test2_type)GetProcAddress2();
+  // linux-warning@-1 {{cast from 'FARPROC2' (aka 'unsigned long long (*)()') to 'test2_type' (aka 'float (*)()') converts to incompatible function type}}
+  // strict-warning@-2 {{cast from 'FARPROC2' (aka 'unsigned long long (*)()') to 'test2_type' (aka 'float (*)()') converts to incompatible function type}}
+}
+