Silence -Wcast-function-type warnings on idiomatic Windows code

[AaronBallman]

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 #86131

[llvmbot]

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

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 #86131

---

Full diff: https://github.com/llvm/llvm-project/pull/135660.diff

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+10)
• (modified) clang/lib/Sema/SemaCast.cpp (+23)
• (added) clang/test/Sema/warn-cast-function-type-win.c (+33)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 840a52a693c7a..0fa764293842d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -355,6 +355,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");
+
 
 Improvements to Clang's time-trace
 ----------------------------------
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 2824dfce1572c..1591075ff05d8 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/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))
diff --git a/clang/test/Sema/warn-cast-function-type-win.c b/clang/test/Sema/warn-cast-function-type-win.c
new file mode 100644
index 0000000000000..16ee777b22c49
--- /dev/null
+++ b/clang/test/Sema/warn-cast-function-type-win.c
@@ -0,0 +1,33 @@
+// 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 {{cast from 'FARPROC1' (aka 'int (*)()') to 'test1_type' (aka 'int (*)(int)') converts to incompatible function type}} \
+                                                    strict-warning {{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 {{cast from 'FARPROC2' (aka 'unsigned long long (*)()') to 'test2_type' (aka 'float (*)()') converts to incompatible function type}} \
+                                                    strict-warning {{cast from 'FARPROC2' (aka 'unsigned long long (*)()') to 'test2_type' (aka 'float (*)()') converts to incompatible function type}}
+}
+

[cor3ntin]

LGTM.
Or should I say, "Thanks, I hate it".

Do you know how likely it is WG14 come up with a good solution for "arbitrary function pointer" ?

[rnk]

I agree this is the practical thing to do.

[AaronBallman]

LGTM. Or should I say, "Thanks, I hate it".

Do you know how likely it is WG14 come up with a good solution for "arbitrary function pointer" ?

The last we heard on this topic was https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2230.htm which was discussed in Brno in 2018 and the following is from my notes: We had consensus that the overall idea of a generic function pointer type is a good one. We took a straw poll to determine whether this should be a core language feature: F:14/O:0/A:5, which is strong consensus for a language feature rather than library option.

So we need an updated paper (the author of that paper retired from the committee).

[AaronBallman]

/cherry-pick d0e4af8

[llvmbot]

Failed to cherry-pick: d0e4af8

https://github.com/llvm/llvm-project/actions/runs/14473318832

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

[tstellar]

@AaronBallman Were you able to manually cherry-pick this one?

[AaronBallman]

@AaronBallman Were you able to manually cherry-pick this one?

Yup! That was handled in #135798 thank you!