https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/135660
>From 1e9fce88cc5e1d4f75e7126a394ec8f9440b8d63 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Mon, 14 Apr 2025 15:21:04 -0400 Subject: [PATCH 1/2] Silence -Wcast-function-type warnings on idiomatic Windows code 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 https://github.com/llvm/llvm-project/pull/86131 --- clang/docs/ReleaseNotes.rst | 10 ++++++ clang/lib/Sema/SemaCast.cpp | 23 +++++++++++++ clang/test/Sema/warn-cast-function-type-win.c | 33 +++++++++++++++++++ 3 files changed, 66 insertions(+) create mode 100644 clang/test/Sema/warn-cast-function-type-win.c 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}} +} + >From 4b25eec38aa68ad2380dda6251114cebb1ed63c1 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Tue, 15 Apr 2025 07:18:34 -0400 Subject: [PATCH 2/2] Update based on review feedback --- clang/test/Sema/warn-cast-function-type-win.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/clang/test/Sema/warn-cast-function-type-win.c b/clang/test/Sema/warn-cast-function-type-win.c index 16ee777b22c49..4e7ba33b258d8 100644 --- a/clang/test/Sema/warn-cast-function-type-win.c +++ b/clang/test/Sema/warn-cast-function-type-win.c @@ -23,11 +23,14 @@ void test(void) { // 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}} + 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 {{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}} + 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}} } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits