BBC: Async-Interrupt-1.25 does not compile anymore since perl 5.31.6

[eserte]

The compilation of MLEHMANN/Async-Interrupt-1.25.tar.gz fails with bleadperl with a error message like this:

Interrupt.xs:209:29: error: too many arguments to function call, expected 1, have 3

Overview of test reports: http://matrix.cpantesters.org/?dist=Async-Interrupt+1.25

A sample report with this compilation failure:

• http://www.cpantesters.org/cpan/report/1fc16e4a-23d4-11ea-ae93-77311f24ea8f (gcc)
• http://www.cpantesters.org/cpan/report/af72acde-2113-11ea-82f8-32321f24ea8f (clang)

[jkeenan]

The compilation of MLEHMANN/Async-Interrupt-1.25.tar.gz fails with bleadperl with a error message like this:

Interrupt.xs:209:29: error: too many arguments to function call, expected 1, have 3

Overview of test reports: http://matrix.cpantesters.org/?dist=Async-Interrupt+1.25

A sample report with this compilation failure:

* http://www.cpantesters.org/cpan/report/1fc16e4a-23d4-11ea-ae93-77311f24ea8f (gcc)

* http://www.cpantesters.org/cpan/report/af72acde-2113-11ea-82f8-32321f24ea8f (clang)

Module has build-time failure. Excerpt from cpanm build.log:

Building and testing Async-Interrupt-1.25
cp Interrupt.pm blib/lib/Async/Interrupt.pm
Running Mkbootstrap for Interrupt ()
chmod 644 "Interrupt.bs"
"/home/jkeenan/testing/threaded_blead/bin/perl" -MExtUtils::Command::MM -e 'cp_nonempty' -- Interrupt.bs blib/arch/auto/Async/Interrupt/Interrupt.bs 644
"/home/jkeenan/testing/threaded_blead/bin/perl" "/home/jkeenan/testing/threaded_blead/lib/perl5/5.31.7/ExtUtils/xsubpp"  -typemap '/home/jkeenan/testing/threaded_blead/lib/perl5/5.31.7/ExtUtils/typemap' -typemap '/home/jkeenan/.cpanm/work/1577414059.16137/Async-Interrupt-1.25/typemap'  Interrupt.xs > Interrupt.xsc
Warning: Aliases 'fileno_r' and 'fileno' have identical values in Interrupt.xs, line 515
mv Interrupt.xsc Interrupt.c
cc -c   -D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2   -DVERSION=\"1.25\" -DXS_VERSION=\"1.25\" -fPIC "-I/home/jkeenan/testing/threaded_blead/lib/perl5/5.31.7/x86_64-linux-thread-multi/CORE"   Interrupt.c
Interrupt.xs: In function ‘async_sighandler’:
Interrupt.xs:209:5: error: too many arguments to function ‘old_sighandler’
     old_sighandler (signum, si, sarg);
     ^~~~~~~~~~~~~~
Interrupt.xs: In function ‘boot_Async__Interrupt’:
Interrupt.xs:264:24: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
         PL_sighandlerp = async_sighandler;
                        ^
In file included from Interrupt.xs:8:0:
schmorp.h: In function ‘s_epipe_signal’:
schmorp.h:450:5: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
     write (epp->fd [1], &counter, (epp->len = 8));
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
schmorp.h: In function ‘s_epipe_drain’:
schmorp.h:463:3: warning: ignoring return value of ‘read’, declared with attribute warn_unused_result [-Wunused-result]
   read (epp->fd [0], buf, sizeof (buf));
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Makefile:331: recipe for target 'Interrupt.o' failed
make: *** [Interrupt.o] Error 1
-> FAIL Installing Async::Interrupt failed.

Bisecting with the following invocation:

perl Porting/bisect.pl \
--start=559a77e6ab \
--end=v5.31.6 \
--module=Async::Interrupt

... points to this commit:

c99d8cd6ea985084aa149f8f2b39217d6fc9068f is the first bad commit
commit c99d8cd6ea985084aa149f8f2b39217d6fc9068f
Author: David Mitchell <davem@iabyn.com>
Date:   Tue Nov 12 10:47:30 2019 +0000
Commit:     David Mitchell <davem@iabyn.com>
CommitDate: Mon Nov 18 09:34:40 2019 +0000

    declare perl core's sig handler as 1-arg
    
    First some background:
    
    UNIXy OSes support two types of signal handler function:
    
        Signal_t handler1(int sig);
        Signal_t handler3(int sig, siginfo_t *info, void *uap);
    
    The original one-argument handler was set using the signal(2) system
    call. The newer sigaction(2) system call allows either a 1-arg or
    3-arg handler to be specified:
    
        act.sa_handler = handler1;
        sigaction(sig, act, NULL);
    
        act.sa_sigaction = handler3;
        act.sa_sa_flags |= SA_SIGINFO;
        sigaction(sig, act, NULL);
    
    The current behaviour in perl core is that, in the presence of
    HAS_SIGACTION and SA_SIGINFO, the signal handler type and function are
    both declared as 3-arg, but perl still tells the kernel that the
    supplied signal handler function takes one arg. This means that whenever
    the kernel calls the handler, args 2 and 3 are whatever garbage the OS
    and architecture cause them to happen to be.
    
    Note that POSIX.xs *does* allow a 3-arg signal handler to be specified
    by passing the SA_SIGINFO flag, and a couple of tests check for this.
    
    Recently, gcc-8 has (quite reasonably) been warning that we're passing
    around 3-arg function pointers where a 1-arg function pointer is
    expected.
    
    After the groundwork laid down by the previous commits in this branch,
    this commit flips things over so that the perl core now declares its
    handlers and handler type as being 1-arg, thus reflecting the reality
    that the core has actually being using 1-arg handlers.
    
    This makes a whole bunch of compiler noise like this go away:
    
        perl.h:2825:51: warning: cast between incompatible function types
        from ‘__sighandler_t’ {aka ‘void (*)(int)’} to ‘void (*)(int,
        siginfo_t *, void *)’ {aka ‘void (*)(int,  struct <anonymous> *,
        void *)’} [-Wcast-function-type]
    
    In theory this should make no functional difference. It might cause
    3rd-party XS modules to emit compiler warnings now if they are relying
    on things like Sighandler_t to represent a 3-arg function - this commit
    flips it to being 1-arg.

@iabyn, can you take a look?

Thank you very much.
Jim Keenan

[iabyn]

On Thu, Dec 26, 2019 at 06:41:36PM -0800, James E Keenan wrote: > The compilation of MLEHMANN/Async-Interrupt-1.25.tar.gz fails with bleadperl with a error message like this: > > ``` > Interrupt.xs:209:29: error: too many arguments to function call, expected 1, have 3

This will almost certainly be a side-effect of my merge commit shown below. It turned a broken mess into a slightly less broken mess, and as a side-effect, affects how many args low-level (i.e. OS) signal handlers need to be declared and/or called with. Async-Interrupt will have to be updated to handle the new reality. commit 116e19a Merge: 42d0f81 b466a94 Author: David Mitchell <davem@iabyn.com> AuthorDate: Mon Nov 18 10:20:54 2019 +0000 Commit: David Mitchell <davem@iabyn.com> CommitDate: Mon Nov 18 10:20:54 2019 +0000 [MERGE] fixup perl's OS signal handling First some background: UNIXy OSes support two types of signal handler function: Signal_t handler1(int sig); Signal_t handler3(int sig, siginfo_t *info, void *uap); The original one-argument handler was set using the signal(2) system call. The newer sigaction(2) system call allows either a 1-arg or 3-arg handler to be specified: act.sa_handler = handler1; sigaction(sig, act, NULL); act.sa_sigaction = handler3; act.sa_sa_flags |= SA_SIGINFO; sigaction(sig, act, NULL); The current behaviour in perl core is that, in the presence of HAS_SIGACTION and SA_SIGINFO, the signal handler type and function are both declared as 3-arg, but perl still tells the kernel that the supplied signal handler function takes one arg. This means that whenever the kernel calls the handler, args 2 and 3 are whatever garbage the OS and architecture cause them to happen to be. Note that POSIX.xs *does* allow a 3-arg signal handler to be specified by passing the SA_SIGINFO flag, and a couple of tests check for this. Recently, gcc-8 has (quite reasonably) been warning that we're passing around 3-arg function pointers where a 1-arg function pointer is expected. This branch fixes things up by declaring the signal handler type as 1-arg to match reality. It also introduces a new type, Siginfo_t, which is a wrapper for those platforms which have siginfo_t, and a dummy implementation for those which don't. In addition it adds new explicit handler functions with a known number of args, e.g. Perl_sighandler1() and Perl_sighandler3(), which can be used by POSIX.xs when it wants to use a specific handler type. It also adds a new function, Perl_perly_sighandler(), which is called by the underlying OS signal handler to do the actual perl-level call to $SIG{FOO}, and which has an extra arg to indicate whether it is being called via a safe signal path or not.

…

-- Overhead, without any fuss, the stars were going out. -- Arthur C Clarke

[jkeenan]

On Thu, Dec 26, 2019 at 06:41:36PM -0800, James E Keenan wrote: > The compilation of MLEHMANN/Async-Interrupt-1.25.tar.gz fails with bleadperl with a error message like this: > > ``` > Interrupt.xs:209:29: error: too many arguments to function call, expected 1, have 3
This will almost certainly be a side-effect of my merge commit shown below. It turned a broken mess into a slightly less broken mess, and as a side-effect, affects how many args low-level (i.e. OS) signal handlers need to be declared and/or called with. Async-Interrupt will have to be updated to handle the new reality.

Do you think this patch for the upstream module would suffice?

--- Interrupt.xs.orig	2020-01-02 18:09:14.713390791 -0500
+++ Interrupt.xs	2020-01-02 18:12:18.846461882 -0500
@@ -201,12 +201,12 @@
 }
 
 #if HAS_SA_SIGINFO
-static Signal_t async_sighandler (int signum, siginfo_t *si, void *sarg)
+static Signal_t async_sighandler (int signum)
 {
   if (signum == 9)
     handle_asyncs ();
   else
-    old_sighandler (signum, si, sarg);
+    old_sighandler (signum);
 }
 #else
 static Signal_t async_sighandler (int signum)

It passes all tests -- but the test coverage for that distro is not high. (And it would need guards for Perl versions.)

Thank you very much.
Jim Keenan

[iabyn]

On Thu, Jan 02, 2020 at 03:17:58PM -0800, James E Keenan wrote: > On Thu, Dec 26, 2019 at 06:41:36PM -0800, James E Keenan wrote: > The compilation of MLEHMANN/Async-Interrupt-1.25.tar.gz fails with bleadperl with a error message like this: > > ``` > Interrupt.xs:209:29: error: too many arguments to function call, expected 1, have 3 > This will almost certainly be a side-effect of my merge commit shown below. It turned a broken mess into a slightly less broken mess, and as a side-effect, affects how many args low-level (i.e. OS) signal handlers need to be declared and/or called with. Async-Interrupt will have to be updated to handle the new reality. Do you think this patch for the upstream module would suffice?

I don't know. Signals are hard; making them portable is harder. And any change would need to work against older versions of perl. But since this is a Marc Lehmann module, I'm not going to devote any effort to fix it. Even submitting correct fixes to him is likely to be rewarded with a stream of invective, which I can do without.

…

-- Counsellor Troi states something other than the blindingly obvious. -- Things That Never Happen in "Star Trek" #16

[Leont]

Do you think this patch for the upstream module would suffice?

It looks to me like it would break on perls before this change. It's probably a better idea to make it use the #else on perls after c99d8cd

[Leont]

But since this is a Marc Lehmann module, I'm not going to devote any
effort to fix it. Even submitting correct fixes to him is likely to be
rewarded with a stream of invective, which I can do without.

I don't think that making comments like these will make that situation any better

[khwilliamson]

On 1/4/20 5:04 PM, Leon Timmermans wrote: But since this is a Marc Lehmann module, I'm not going to devote any effort to fix it. Even submitting correct fixes to him is likely to be rewarded with a stream of invective, which I can do without. I don't think that making comments like these will make that situation any better

I don't think that makeing comments like those would make that situation any worse either

[toddr]

If someone wants to give me the patch, I'll be happy to let him vent at me.

[iabyn]

On Mon, Feb 17, 2020 at 11:38:12AM -0800, Todd Rinaldo wrote: If someone wants to give me the patch, I'll be happy to let him vent at me.

I have no desire to write such a patch.

…

-- This is a great day for France! -- Nixon at Charles De Gaulle's funeral