[Support] Silence warnings when retrieving exported functions #97905

aganea · 2024-07-06T16:33:12Z

Since functions exported from DLLs are type-erased, before this patch I was seeing the new Clang 19 warning -Wcast-function-type-mismatch.

This happens when building LLVM on Windows.

Following discussion in 593f708#commitcomment-143905744

… function Since functions exported from DLLs are type-erased, before this patch I was seeing the new Clang 19 warning `-Wcast-function-type-mismatch`. This happens when building LLVM on Windows.

llvmbot · 2024-07-06T16:33:40Z

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-llvm-support

Author: Alexandre Ganea (aganea)

Changes

Since functions exported from DLLs are type-erased, before this patch I was seeing the new Clang 19 warning -Wcast-function-type-mismatch.

This happens when building LLVM on Windows.

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

3 Files Affected:

(modified) llvm/include/llvm/Support/Windows/WindowsSupport.h (+6)
(modified) llvm/lib/Support/Windows/Process.inc (+2-1)
(modified) llvm/lib/Support/Windows/Signals.inc (+12-10)

diff --git a/llvm/include/llvm/Support/Windows/WindowsSupport.h b/llvm/include/llvm/Support/Windows/WindowsSupport.h
index d3aacd14b2097..1f4bd71aec49f 100644
--- a/llvm/include/llvm/Support/Windows/WindowsSupport.h
+++ b/llvm/include/llvm/Support/Windows/WindowsSupport.h
@@ -245,6 +245,12 @@ std::error_code GetCommandLineArguments(SmallVectorImpl<const char *> &Args,
 std::error_code widenPath(const Twine &Path8, SmallVectorImpl<wchar_t> &Path16,
                           size_t MaxPathLen = MAX_PATH);
 
+/// Dynamically retrieve an exported function from a DLL, while casting it to a
+/// known type. This avoids -Wcast-function-type-mismatch.
+template <typename T> T funcFromModule(HMODULE Mod, StringRef Name) {
+  return (T)(void *)::GetProcAddress(Mod, Name.data());
+}
+
 } // end namespace windows
 } // end namespace sys
 } // end namespace llvm.
diff --git a/llvm/lib/Support/Windows/Process.inc b/llvm/lib/Support/Windows/Process.inc
index 34d294b232c32..4f6a41e23904e 100644
--- a/llvm/lib/Support/Windows/Process.inc
+++ b/llvm/lib/Support/Windows/Process.inc
@@ -482,7 +482,8 @@ static RTL_OSVERSIONINFOEXW GetWindowsVer() {
     HMODULE hMod = ::GetModuleHandleW(L"ntdll.dll");
     assert(hMod);
 
-    auto getVer = (RtlGetVersionPtr)::GetProcAddress(hMod, "RtlGetVersion");
+    auto getVer = llvm::sys::windows::funcFromModule<RtlGetVersionPtr>(
+        hMod, "RtlGetVersion");
     assert(getVer);
 
     RTL_OSVERSIONINFOEXW info{};
diff --git a/llvm/lib/Support/Windows/Signals.inc b/llvm/lib/Support/Windows/Signals.inc
index 29ebf7c696e04..432b2e9b8dfd7 100644
--- a/llvm/lib/Support/Windows/Signals.inc
+++ b/llvm/lib/Support/Windows/Signals.inc
@@ -168,25 +168,27 @@ static bool isDebugHelpInitialized() {
 }
 
 static bool load64BitDebugHelp(void) {
+  using namespace llvm::sys::windows;
+
   HMODULE hLib =
       ::LoadLibraryExA("Dbghelp.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
   if (hLib) {
     fMiniDumpWriteDump =
-        (fpMiniDumpWriteDump)::GetProcAddress(hLib, "MiniDumpWriteDump");
-    fStackWalk64 = (fpStackWalk64)::GetProcAddress(hLib, "StackWalk64");
+        funcFromModule<fpMiniDumpWriteDump>(hLib, "MiniDumpWriteDump");
+    fStackWalk64 = funcFromModule<fpStackWalk64>(hLib, "StackWalk64");
     fSymGetModuleBase64 =
-        (fpSymGetModuleBase64)::GetProcAddress(hLib, "SymGetModuleBase64");
+        funcFromModule<fpSymGetModuleBase64>(hLib, "SymGetModuleBase64");
     fSymGetSymFromAddr64 =
-        (fpSymGetSymFromAddr64)::GetProcAddress(hLib, "SymGetSymFromAddr64");
+        funcFromModule<fpSymGetSymFromAddr64>(hLib, "SymGetSymFromAddr64");
     fSymGetLineFromAddr64 =
-        (fpSymGetLineFromAddr64)::GetProcAddress(hLib, "SymGetLineFromAddr64");
+        funcFromModule<fpSymGetLineFromAddr64>(hLib, "SymGetLineFromAddr64");
     fSymGetModuleInfo64 =
-        (fpSymGetModuleInfo64)::GetProcAddress(hLib, "SymGetModuleInfo64");
-    fSymFunctionTableAccess64 = (fpSymFunctionTableAccess64)::GetProcAddress(
+        funcFromModule<fpSymGetModuleInfo64>(hLib, "SymGetModuleInfo64");
+    fSymFunctionTableAccess64 = funcFromModule<fpSymFunctionTableAccess64>(
         hLib, "SymFunctionTableAccess64");
-    fSymSetOptions = (fpSymSetOptions)::GetProcAddress(hLib, "SymSetOptions");
-    fSymInitialize = (fpSymInitialize)::GetProcAddress(hLib, "SymInitialize");
-    fEnumerateLoadedModules = (fpEnumerateLoadedModules)::GetProcAddress(
+    fSymSetOptions = funcFromModule<fpSymSetOptions>(hLib, "SymSetOptions");
+    fSymInitialize = funcFromModule<fpSymInitialize>(hLib, "SymInitialize");
+    fEnumerateLoadedModules = funcFromModule<fpEnumerateLoadedModules>(
         hLib, "EnumerateLoadedModules64");
   }
   return isDebugHelpInitialized();

compnerd · 2024-07-06T17:18:08Z

llvm/include/llvm/Support/Windows/WindowsSupport.h

+template <typename T> T funcFromModule(HMODULE Mod, StringRef Name) {
+  return (T)(void *)::GetProcAddress(Mod, Name.data());
+}


What about something like:

template <typename ReturnType, typename... Arguments> auto funcFromModule(HMODULE hMod, StringRef Name) -> ReturnType ((*)(Arguments...)) { return reinterpret_cast<ReturnType (*)(Arguments...)>(::GetProcAddress(hMod, Name.data()); }

You can't destructure a C function pointer like this, can you? This needs std::remove_pointer_t<> on the calling site?

aganea · 2024-07-07T13:29:46Z

I reverted back to C-casting to a void *. I think having a helper function with all the template wizardy has very low value in terms of cognitive load.

compnerd

This is confusing to me. I think that we should declare typedefs for the function pointer type. I doubt that anyone wants to build on 32-bit at this point, but the calling convention differences on Windows really do require that we create typedefs for the function pointer type so that we can account for the calling convention. As an example:

typedef (NTSTATUS)(*RtlGetVersionTy)(PRTL_OSVERSIONINFOW);

mstorsjo · 2024-07-07T20:14:58Z

I doubt that anyone wants to build on 32-bit at this point

FWIW, I continuously build and test LLVM (both running LLVM itself on, and generating code for) on 32 bit Windows targets, both i386 and arm.

compnerd · 2024-07-07T20:17:32Z

FWIW, I continuously build and test LLVM (both running LLVM itself on, and generating code for) on 32 bit Windows targets, both i386 and arm.

In that case, I think that the suggestion is more a requirement - we cannot risk losing thee calling convention attribute on the function pointer.

aganea · 2024-07-30T14:09:36Z

This is confusing to me. I think that we should declare typedefs for the function pointer type. I doubt that anyone wants to build on 32-bit at this point, but the calling convention differences on Windows really do require that we create typedefs for the function pointer type so that we can account for the calling convention. As an example:
typedef (NTSTATUS)(*RtlGetVersionTy)(PRTL_OSVERSIONINFOW);

We do have typedefs with proper calling convention for all functions retrieved with GetProcAddress, for example see: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/Windows/Signals.inc#L124
The problem is that GetProcAddress returns a FARPROC value but we need to cast it to another pointer type.

What would be the way forward here? Are we fine with the (void*) cast?

compnerd

I see, the void* is an intermediate cast to drop the FARPROC. That seems fine to me. The one final piece - the function is type erased irrespective of windows or not and DLL exported or not. Symbols on ELF and MachO libraries will also be "type erased".

aganea · 2024-07-30T15:06:44Z

Thanks @compnerd! I will apply the same receipe for other places where this occurs in the codebase (compiler-rt for instance)

llvm-ci · 2024-07-30T23:36:12Z

LLVM Buildbot has detected a new failure on builder clang-armv8-quick running on linaro-clang-armv8-quick while building llvm at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/154/builds/2148

Here is the relevant piece of the build log for the reference:

Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'ClangPseudo :: crash/backslashes.c' FAILED ********************
Exit Code: 134

Command Output (stderr):
--
RUN: at line 2: clang-pseudo -source=/home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang-tools-extra/pseudo/test/crash/backslashes.c -print-tokens
+ clang-pseudo -source=/home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang-tools-extra/pseudo/test/crash/backslashes.c -print-tokens
clang-pseudo: ../llvm/clang-tools-extra/pseudo/lib/cxx/CXX.cpp:437: auto clang::pseudo::cxx::getLanguage()::(anonymous class)::operator()() const: Assertion `Diags.empty()' failed.
#0 0x007d61d0 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/tcwg-buildbot/worker/clang-armv8-quick/stage1/bin/clang-pseudo+0x5e1d0)
#1 0x007d3f38 llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/clang-armv8-quick/stage1/bin/clang-pseudo+0x5bf38)
#2 0x007d6c3c SignalHandler(int) Signals.cpp:0:0
#3 0xf7c56530 __default_sa_restorer /build/glibc-tftl1u/glibc-2.31/signal/../sysdeps/unix/sysv/linux/arm/sigrestorer.S:67:0
#4 0xf7c467e6 /build/glibc-tftl1u/glibc-2.31/csu/../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:47:0
#5 0xf7c557fe raise /build/glibc-tftl1u/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:28:1
/home/tcwg-buildbot/worker/clang-armv8-quick/stage1/tools/clang/tools/extra/pseudo/test/crash/Output/backslashes.c.script: line 1: 3193434 Aborted                 clang-pseudo -source=/home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang-tools-extra/pseudo/test/crash/backslashes.c -print-tokens

--

********************

aganea · 2024-07-31T00:10:05Z

/cherry-pick 39e192b

…7905) Since functions exported from DLLs are type-erased, before this patch I was seeing the new Clang 19 warning `-Wcast-function-type-mismatch`. This happens when building LLVM on Windows. Following discussion in llvm@593f708#commitcomment-143905744 (cherry picked from commit 39e192b)

llvmbot · 2024-07-31T00:14:59Z

/pull-request #101266

…7905) Since functions exported from DLLs are type-erased, before this patch I was seeing the new Clang 19 warning `-Wcast-function-type-mismatch`. This happens when building LLVM on Windows. Following discussion in llvm@593f708#commitcomment-143905744 (cherry picked from commit 39e192b)

This fixes a few of these warnings, when building with Clang ToT on Windows: ``` [622/7618] Building CXX object projects\compiler-rt\lib\sanitizer_common\CMakeFiles\RTSanitizerCommonSymbolizer.x86_64.dir\sanitizer_symbolizer_win.cpp.obj C:\src\git\llvm-project\compiler-rt\lib\sanitizer_common\sanitizer_symbolizer_win.cpp(74,3): warning: cast from 'FARPROC' (aka 'long long (*)()') to 'decltype(::StackWalk64) *' (aka 'int (*)(unsigned long, void *, void *, _tagSTACKFRAME64 *, void *, int (*)(void *, unsigned long long, void *, unsigned long, unsigned long *), void *(*)(void *, unsigned long long), unsigned long long (*)(void *, unsigned long long), unsigned long long (*)(void *, void *, _tagADDRESS64 *))') converts to incompatible function type [-Wcast-function-type-mismatch] ``` This is similar to #97905

This fixes the following warning, when building with Clang ToT on Windows: ``` [6668/7618] Building CXX object tools\lldb\source\Plugins\Process\Windows\Common\CMakeFiles\lldbPluginProcessWindowsCommon.dir\TargetThreadWindows.cpp.obj C:\src\git\llvm-project\lldb\source\Plugins\Process\Windows\Common\TargetThreadWindows.cpp(182,22): warning: cast from 'FARPROC' (aka 'long long (*)()') to 'GetThreadDescriptionFunctionPtr' (aka 'long (*)(void *, wchar_t **)') converts to incompatible function type [-Wcast-function-type-mismatch] ``` This is similar to: #97905

[Support][Windows] Add utility function for retrieving a DLL-exported…

ea06f09

… function Since functions exported from DLLs are type-erased, before this patch I was seeing the new Clang 19 warning `-Wcast-function-type-mismatch`. This happens when building LLVM on Windows.

aganea requested review from tru, compnerd, mstorsjo, nikic and zmodem July 6, 2024 16:33

llvmbot added platform:windows llvm:support labels Jul 6, 2024

compnerd reviewed Jul 6, 2024

View reviewed changes

Revert back to casting the function to a void*

0006144

Forgot one void * cast.

b572260

compnerd reviewed Jul 7, 2024

View reviewed changes

aganea changed the title ~~[Support][Windows] Add utility function for retrieving a DLL-exported function~~ [Support][Windows] Silence warnings when retrieving DLL-exported functions Jul 30, 2024

compnerd approved these changes Jul 30, 2024

View reviewed changes

aganea changed the title ~~[Support][Windows] Silence warnings when retrieving DLL-exported functions~~ [Support] Silence warnings when retrieving exported functions Jul 30, 2024

aganea merged commit 39e192b into llvm:main Jul 30, 2024
7 checks passed

aganea deleted the fix_getprocaddress_warning branch July 30, 2024 23:06

aganea added this to the LLVM 19.X Release milestone Jul 31, 2024

philnik777 mentioned this pull request Aug 2, 2024

[Clang] Add a release note deprecating __is_nullptr #101638

Merged

mgabka mentioned this pull request Aug 22, 2024

Add release note about ABI mgabka/llvm-project#6

Open

thewtex mentioned this pull request Feb 10, 2025

llvmorg 19.1.5 libcxxabi pthread lib name #126605

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Support] Silence warnings when retrieving exported functions #97905

[Support] Silence warnings when retrieving exported functions #97905

aganea commented Jul 6, 2024 •

edited

Loading

llvmbot commented Jul 6, 2024 •

edited

Loading

compnerd Jul 6, 2024

aganea Jul 7, 2024

aganea commented Jul 7, 2024 •

edited

Loading

compnerd left a comment •

edited

Loading

mstorsjo commented Jul 7, 2024 •

edited

Loading

compnerd commented Jul 7, 2024

aganea commented Jul 30, 2024

compnerd left a comment

aganea commented Jul 30, 2024

llvm-ci commented Jul 30, 2024

aganea commented Jul 31, 2024

llvmbot commented Jul 31, 2024

[Support] Silence warnings when retrieving exported functions #97905

[Support] Silence warnings when retrieving exported functions #97905

Conversation

aganea commented Jul 6, 2024 • edited Loading

llvmbot commented Jul 6, 2024 • edited Loading

compnerd Jul 6, 2024

Choose a reason for hiding this comment

aganea Jul 7, 2024

Choose a reason for hiding this comment

aganea commented Jul 7, 2024 • edited Loading

compnerd left a comment • edited Loading

Choose a reason for hiding this comment

mstorsjo commented Jul 7, 2024 • edited Loading

compnerd commented Jul 7, 2024

aganea commented Jul 30, 2024

compnerd left a comment

Choose a reason for hiding this comment

aganea commented Jul 30, 2024

llvm-ci commented Jul 30, 2024

aganea commented Jul 31, 2024

llvmbot commented Jul 31, 2024

aganea commented Jul 6, 2024 •

edited

Loading

llvmbot commented Jul 6, 2024 •

edited

Loading

aganea commented Jul 7, 2024 •

edited

Loading

compnerd left a comment •

edited

Loading

mstorsjo commented Jul 7, 2024 •

edited

Loading