From b4a5b3203999db7b52214831e5a8f0a535b7b6c8 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Wed, 3 Jan 2024 14:53:34 -0800 Subject: [PATCH] [libc] fix -Wcast-function-type via union rather than reinterpret_cast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GCC build is producing the following diagnostic: llvm-project/libc/src/signal/linux/signal_utils.h: In member function ‘__llvm_libc_18_0_0_git::KernelSigaction& __llvm_libc_18_0_0_git::KernelSigaction::operator=(const sigaction&)’: llvm-project/libc/src/signal/linux/signal_utils.h:38:20: warning: cast between incompatible function types from ‘void (*)(int, siginfo_t*, void*)’ to ‘void (*)(int)’ [-Wcast-function-type] 38 | sa_handler = reinterpret_cast(sa.sa_sigaction); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ llvm-project/libc/src/signal/linux/signal_utils.h: In member function ‘__llvm_libc_18_0_0_git::KernelSigaction::operator sigaction() const’: llvm-project/libc/src/signal/linux/signal_utils.h:51:25: warning: cast between incompatible function types from ‘void (*)(int)’ to ‘void (*)(int, siginfo_t*, void*)’ [-Wcast-function-type] 51 | sa.sa_sigaction = reinterpret_cast(sa_handler); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Two issues here: 1. Clang supports -Wcast-function-type, but not as part of the -Wextra group. 2. The existing implementation tried to work around the oddity that is the kernel's struct sigaction != POSIX via reinterpret_cast in a way that's not compatible with -Wcast-function-type. Just use a union which is well defined (and two function pointers are the same size.) Link: https://github.com/llvm/llvm-project/issues/76872 Fixes: https://github.com/llvm/llvm-project/issues/74617 --- libc/src/signal/linux/signal_utils.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libc/src/signal/linux/signal_utils.h b/libc/src/signal/linux/signal_utils.h index ca6fd3aeb1a6e..5e9dd9a5c53ab 100644 --- a/libc/src/signal/linux/signal_utils.h +++ b/libc/src/signal/linux/signal_utils.h @@ -27,15 +27,12 @@ namespace LIBC_NAMESPACE { // handler taking siginfo_t * argument, one can set sa_handler to sa_sigaction // if SA_SIGINFO is set in sa_flags. struct KernelSigaction { - using HandlerType = void(int); - using SiginfoHandlerType = void(int, siginfo_t *, void *); - LIBC_INLINE KernelSigaction &operator=(const struct sigaction &sa) { sa_flags = sa.sa_flags; sa_restorer = sa.sa_restorer; sa_mask = sa.sa_mask; if (sa_flags & SA_SIGINFO) { - sa_handler = reinterpret_cast(sa.sa_sigaction); + sa_sigaction = sa.sa_sigaction; } else { sa_handler = sa.sa_handler; } @@ -48,13 +45,16 @@ struct KernelSigaction { sa.sa_mask = sa_mask; sa.sa_restorer = sa_restorer; if (sa_flags & SA_SIGINFO) - sa.sa_sigaction = reinterpret_cast(sa_handler); + sa.sa_sigaction = sa_sigaction; else sa.sa_handler = sa_handler; return sa; } - HandlerType *sa_handler; + union { + void (*sa_handler)(int); + void (*sa_sigaction)(int, siginfo_t *, void *); + }; unsigned long sa_flags; void (*sa_restorer)(void); // Our public definition of sigset_t matches that of the kernel's definition.