[libc-commits] [libc] [libc] fix -Wcast-function-type via union rather than reinterpret_cast (PR #76875)

[via libc-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

<details>
<summary>Changes</summary>

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<HandlerType *>(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<SiginfoHandlerType *>(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


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


1 Files Affected:

- (modified) libc/src/signal/linux/signal_utils.h (+6-6) 


``````````diff
diff --git a/libc/src/signal/linux/signal_utils.h b/libc/src/signal/linux/signal_utils.h
index ca6fd3aeb1a6ed..5e9dd9a5c53ab0 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<HandlerType *>(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<SiginfoHandlerType *>(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.

``````````

</details>


https://github.com/llvm/llvm-project/pull/76875