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

[Nick Desaulniers via libc-commits]

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

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


>From b4a5b3203999db7b52214831e5a8f0a535b7b6c8 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
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<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
---
 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 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.