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

eserte · 2019-12-26T22:16:23Z

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:

The text was updated successfully, but these errors were encountered:

jkeenan · 2019-12-27T02:41:36Z

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 <[email protected]>
Date:   Tue Nov 12 10:47:30 2019 +0000
Commit:     David Mitchell <[email protected]>
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 · 2020-01-02T15:31:41Z

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 <[email protected]> AuthorDate: Mon Nov 18 10:20:54 2019 +0000 Commit: David Mitchell <[email protected]> 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 · 2020-01-02T23:17:57Z

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 · 2020-01-03T09:42:46Z

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 · 2020-01-04T23:49:30Z

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 · 2020-01-05T00:04:37Z

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 · 2020-01-05T19:15:14Z

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 · 2020-02-17T19:38:11Z

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

iabyn · 2020-02-18T09:46:26Z

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

xsawyerx · 2020-04-01T09:40:00Z

I cannot ask Dave to write such a patch if he is not interested in it. He experienced enough to step back and not get involved. I understand and support it.

@tonycoz, do you think there's enough information here for you to try and come up with a patch for it? Todd will be handling the communication with the author.

xsawyerx · 2020-04-01T09:42:47Z

In any case, we still need to determine if this is a blocker. I think it should stay a blocker and forces us to revert if we cannot resolve it one way or the other. Considering we haven't reached out to the author yet, we need to do that first. (Hopefully with a patch.)

Leont · 2020-04-01T21:28:12Z

The following patch fixes the issue (tested on 5.24.0 and 5.31.10):

From 2fc0da67fb02cf578081bb0efe8bf76ec5190582 Mon Sep 17 00:00:00 2001
From: Leon Timmermans <[email protected]>
Date: Wed, 1 Apr 2020 23:26:53 +0200
Subject: [PATCH] Fix signal handler chaining on 5.31.6+

This was broken by core commit c99d8cd6ea985084aa149f8f2b39217d6fc9068f
---
 Interrupt.xs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git Interrupt.xs Interrupt.xs
index a177b2e..d99fd6c 100644
--- Interrupt.xs
+++ Interrupt.xs
@@ -200,7 +200,7 @@ handle_asyncs (void)
     }
 }
 
-#if HAS_SA_SIGINFO
+#if HAS_SA_SIGINFO && !PERL_VERSION_ATLEAST(5, 31, 6)
 static Signal_t async_sighandler (int signum, siginfo_t *si, void *sarg)
 {
   if (signum == 9)
-- 
2.26.0

jkeenan · 2020-04-01T23:00:46Z

The following patch fixes the issue (tested on 5.24.0 and 5.31.10):

From 2fc0da67fb02cf578081bb0efe8bf76ec5190582 Mon Sep 17 00:00:00 2001
From: Leon Timmermans <[email protected]>
Date: Wed, 1 Apr 2020 23:26:53 +0200
Subject: [PATCH] Fix signal handler chaining on 5.31.6+

This was broken by core commit c99d8cd6ea985084aa149f8f2b39217d6fc9068f
---
 Interrupt.xs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git Interrupt.xs Interrupt.xs
index a177b2e..d99fd6c 100644
--- Interrupt.xs
+++ Interrupt.xs
@@ -200,7 +200,7 @@ handle_asyncs (void)
     }
 }
 
-#if HAS_SA_SIGINFO
+#if HAS_SA_SIGINFO && !PERL_VERSION_ATLEAST(5, 31, 6)
 static Signal_t async_sighandler (int signum, siginfo_t *si, void *sarg)
 {
   if (signum == 9)
-- 
2.26.0

With @Leont's patch, I was able to install Async::Interrupt on unthreaded builds on Linux for the following production versions and also on a post-5.31.10 blead.

  513  perlbrew use perl-5.12.5
  522  perlbrew use perl-5.14.4
  529  perlbrew use perl-5.16.3
  532  perlbrew use perl-5.18.4
  538  perlbrew use perl-5.20.3
  542  perlbrew use perl-5.22.3
  546  perlbrew use perl-5.24.4
  550  perlbrew use perl-5.26.2
  554  perlbrew use perl-5.28.0
  559  perlbrew use perl-5.30.0

With that patch I was also able to install Async::Interrupt against a threaded post-5.31.10 build on FreeBSD-11.

Thank you very much.
Jim Keenan

jkeenan · 2020-04-01T23:47:36Z

The following patch fixes the issue (tested on 5.24.0 and 5.31.10):

From 2fc0da67fb02cf578081bb0efe8bf76ec5190582 Mon Sep 17 00:00:00 2001
From: Leon Timmermans <[email protected]>
Date: Wed, 1 Apr 2020 23:26:53 +0200
Subject: [PATCH] Fix signal handler chaining on 5.31.6+

This was broken by core commit c99d8cd6ea985084aa149f8f2b39217d6fc9068f
---
 Interrupt.xs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git Interrupt.xs Interrupt.xs
index a177b2e..d99fd6c 100644
--- Interrupt.xs
+++ Interrupt.xs
@@ -200,7 +200,7 @@ handle_asyncs (void)
     }
 }
 
-#if HAS_SA_SIGINFO
+#if HAS_SA_SIGINFO && !PERL_VERSION_ATLEAST(5, 31, 6)
 static Signal_t async_sighandler (int signum, siginfo_t *si, void *sarg)
 {
   if (signum == 9)
-- 
2.26.0

With @Leont's patch, I was able to install Async::Interrupt on unthreaded builds on Linux for the following production versions and also on a post-5.31.10 blead.

  513  perlbrew use perl-5.12.5
  522  perlbrew use perl-5.14.4
  529  perlbrew use perl-5.16.3
  532  perlbrew use perl-5.18.4
  538  perlbrew use perl-5.20.3
  542  perlbrew use perl-5.22.3
  546  perlbrew use perl-5.24.4
  550  perlbrew use perl-5.26.2
  554  perlbrew use perl-5.28.0
  559  perlbrew use perl-5.30.0

With that patch I was also able to install Async::Interrupt against a threaded post-5.31.10 build on FreeBSD-11.

Thank you very much.
Jim Keenan

Also was able to install Async::Interrupt on NetBSD-8.0 and OpenBSD-6.6. Example:

[openbsd66: blead] $ uname -mrs
OpenBSD 6.6 amd64
[openbsd66: blead] $ ./bin/perl -v | head -2 | tail -1 
This is perl 5, version 32, subversion 0 (v5.32.0 (v5.31.10-26-g45f87e6586)) built for OpenBSD.amd64-openbsd-thread-multi
[openbsd66: blead] $ ./bin/perl -MAsync::Interrupt -E 'say q|hello world|;'
hello world

khwilliamson · 2020-04-02T15:24:57Z

Is there a reason not to apply this?

jkeenan · 2020-04-02T15:56:48Z

Is there a reason not to apply this?

It has to be applied upstream on CPAN. It's not patching blead, nor does Async-Interrupt ship with core.

toddr · 2020-04-02T16:07:34Z

The following patch fixes the issue (tested on 5.24.0 and 5.31.10):

From 2fc0da67fb02cf578081bb0efe8bf76ec5190582 Mon Sep 17 00:00:00 2001
From: Leon Timmermans <[email protected]>
Date: Wed, 1 Apr 2020 23:26:53 +0200
Subject: [PATCH] Fix signal handler chaining on 5.31.6+

This was broken by core commit c99d8cd6ea985084aa149f8f2b39217d6fc9068f
---
 Interrupt.xs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git Interrupt.xs Interrupt.xs
index a177b2e..d99fd6c 100644
--- Interrupt.xs
+++ Interrupt.xs
@@ -200,7 +200,7 @@ handle_asyncs (void)
     }
 }
 
-#if HAS_SA_SIGINFO
+#if HAS_SA_SIGINFO && !PERL_VERSION_ATLEAST(5, 31, 6)
 static Signal_t async_sighandler (int signum, siginfo_t *si, void *sarg)
 {
   if (signum == 9)
-- 
2.26.0

Looks simple. Does everything work above 5.31.6? what are we disabling? Was he overriding a core perl function there?

Leont · 2020-04-03T16:20:43Z

Does everything work above 5.31.6?

It appears so.

what are we disabling?

Two arguments that are only used for unsafe signals, not for safe signals.

Was he overriding a core perl function there?

Yes, the handler for safe signals.

xsawyerx · 2020-04-04T23:45:00Z

@toddr Can you submit this to the author?

toddr · 2020-04-05T00:09:03Z

yep

xsawyerx · 2020-04-15T18:10:16Z

@toddr I cannot find a ticket for this, nor is there a new version of Async-Interrupt. Did you reach out to the author with it?

toddr · 2020-04-21T03:31:09Z

@toddr I cannot find a ticket for this, nor is there a new version of Async-Interrupt. Did you reach out to the author with it?

Marc doesn't do RT. However It appears to have already been reported to him here in January by Andreas. http://lists.schmorp.de/pipermail/anyevent/2020q1/000893.html

toddr · 2020-04-21T03:55:08Z

I have sent him an email. Let's give it a few days for a response.

xsawyerx · 2020-04-23T05:13:39Z

And a lovely response in return, finding an opportunity to bash p5p. sigh

I'm keeping this issue open for observation, but it is no longer a blocker for 5.32.0.

toddr · 2020-04-27T15:16:55Z

Marc has released the patch provided by @Leont. cpantesters already shows it is passing on 5.31.10. I think we're all done here. Closing case.

toddr · 2020-04-27T15:17:57Z

Tests: http://matrix.cpantesters.org/?dist=Async-Interrupt%201.26;os=linux;perl=5.31.10;reports=1
The release: https://metacpan.org/release/Async-Interrupt

eserte added BBC Blead Breaks CPAN - changes in blead broke a cpan module(s) Needs Triage affects-5.31 labels Dec 26, 2019

jkeenan removed the Needs Triage label Dec 27, 2019

toddr added this to the 5.32.0 milestone Feb 17, 2020

toddr self-assigned this Apr 16, 2020

xsawyerx removed this from the 5.32.0 milestone Apr 23, 2020

toddr closed this as completed Apr 27, 2020

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

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

Comments

eserte commented Dec 26, 2019

jkeenan commented Dec 27, 2019

Uh oh!

iabyn commented Jan 2, 2020 via email

Uh oh!

jkeenan commented Jan 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iabyn commented Jan 3, 2020 via email

Uh oh!

Leont commented Jan 4, 2020

Uh oh!

Leont commented Jan 5, 2020

Uh oh!

khwilliamson commented Jan 5, 2020 via email

Uh oh!

toddr commented Feb 17, 2020

Uh oh!

iabyn commented Feb 18, 2020 via email

Uh oh!

xsawyerx commented Apr 1, 2020

Uh oh!

xsawyerx commented Apr 1, 2020

Uh oh!

Leont commented Apr 1, 2020

Uh oh!

jkeenan commented Apr 1, 2020

Uh oh!

jkeenan commented Apr 1, 2020

Uh oh!

khwilliamson commented Apr 2, 2020

Uh oh!

jkeenan commented Apr 2, 2020

Uh oh!

toddr commented Apr 2, 2020

Uh oh!

Leont commented Apr 3, 2020

Uh oh!

xsawyerx commented Apr 4, 2020

Uh oh!

toddr commented Apr 5, 2020

Uh oh!

xsawyerx commented Apr 15, 2020

Uh oh!

toddr commented Apr 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

toddr commented Apr 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xsawyerx commented Apr 23, 2020

Uh oh!

toddr commented Apr 27, 2020

Uh oh!

toddr commented Apr 27, 2020

Uh oh!

jkeenan commented Jan 2, 2020 •

edited

Loading

toddr commented Apr 21, 2020 •

edited

Loading

toddr commented Apr 21, 2020 •

edited

Loading