Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SIGUNUSED removal in glibc 2.26 changes PL_sig_name / SIG_SIZE #16152

Closed
p5pRT opened this issue Sep 16, 2017 · 11 comments
Closed

SIGUNUSED removal in glibc 2.26 changes PL_sig_name / SIG_SIZE #16152

p5pRT opened this issue Sep 16, 2017 · 11 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 16, 2017

Migrated from rt.perl.org#132105 (status was 'resolved')

Searchable as RT132105$

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2017

From @ntyni

As discussed in
https://bugs.debian.org/875927
https://bugs.launchpad.net/ubuntu/+source/libanyevent-perl/+bug/1717367

a change in glibc 2.26, presumably
https://patchwork.sourceware.org/patch/20800/
causes perl to change its ABI when rebuilt against the new glibc.
This is a serious problem for distributions that need to manage ABI
changes carefully.

The problem seems to be that Perl's global array PL_sig_name[] and
the SIG_SIZE constant ($Config{sig_name}, $Config{sig_size} on the
perl language side) change due to the removal of the (long?) obsolete
SIGUNUSED constant in glibc. In this case, the Async​::Interrupt XS module
compiles in SIG_SIZE and walks PL_sig_name[] at run time based on that.
When Perl is rebuilt against the new glibc but Async​::Interrupt is not,
the latter does not know that the array has become smaller and accesses
it out of bounds.

It looks to me like Perl builds on glibc could pad PL_sig_name[] on the
maint-* branches when SIGUNUSED is missing (indicating glibc >= 2.26)
to ensure ABI compatibility. Would that be feasible?

For bleadperl / 5.27, where there are no ABI guarantees, it seems
appropriate to start filtering out SIGUNUSED altogether regardless of
the glibc version, given its apparent obsolete status.

Similar code seems to be in IO​::AIO, Coro and Ev, see s_signum()
in schmorp.h.

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.26.0:

Configured by Debian Project at Tue Sep 12 15:07:07 UTC 2017.

Summary of my perl5 (revision 5 version 26 subversion 0) configuration:
   
  Platform:
    osname=linux
    osvers=4.9.0
    archname=x86_64-linux-gnu-thread-multi
    uname='linux localhost 4.9.0 #1 smp debian 4.9.0 x86_64 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dcc=x86_64-linux-gnu-gcc -Dcpp=x86_64-linux-gnu-cpp -Dld=x86_64-linux-gnu-gcc -Dccflags=-DDEBIAN -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/perl-uoUNjt/perl-5.26.0=. -fstack-protector-strong -Wformat -Werror=format-security -Dldflags= -Wl,-z,relro -Dlddlflags=-shared -Wl,-z,relro -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.26 -Darchlib=/usr/lib/x86_64-linux-gnu/perl/5.26 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/x86_64-linux-gnu/perl5/5.26 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.26.0 -Dsitearch=/usr/local/lib/x86_64-linux-gnu/perl/5.26.0 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Duse64bitint -Dman1ext=1 -Dman3ext=3perl
-Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -Ui_libutil -Uversiononly -DDEBUGGING=-g -Doptimize=-O2 -dEs -Duseshrplib -Dlibperl=libperl.so.5.26.0'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='x86_64-linux-gnu-gcc'
    ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    optimize='-O2 -g'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include'
    ccversion=''
    gccversion='7.2.0'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='x86_64-linux-gnu-gcc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/7/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=libc-2.24.so
    so=so
    useshrplib=true
    libperl=libperl.so.5.26
    gnulibc_version='2.24'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -L/usr/local/lib -fstack-protector-strong'

Locally applied patches:
    DEBPKG:debian/cpan_definstalldirs - Provide a sensible INSTALLDIRS default for modules installed from CPAN.
    DEBPKG:debian/db_file_ver - https://bugs.debian.org/340047 Remove overly restrictive DB_File version check.
    DEBPKG:debian/doc_info - Replace generic man(1) instructions with Debian-specific information.
    DEBPKG:debian/enc2xs_inc - https://bugs.debian.org/290336 Tweak enc2xs to follow symlinks and ignore missing @INC directories.
    DEBPKG:debian/errno_ver - https://bugs.debian.org/343351 Remove Errno version check due to upgrade problems with long-running processes.
    DEBPKG:debian/libperl_embed_doc - https://bugs.debian.org/186778 Note that libperl-dev package is required for embedded linking
    DEBPKG:fixes/respect_umask - Respect umask during installation
    DEBPKG:debian/writable_site_dirs - Set umask approproately for site install directories
    DEBPKG:debian/extutils_set_libperl_path - EU:MM: set location of libperl.a under /usr/lib
    DEBPKG:debian/no_packlist_perllocal - Don't install .packlist or perllocal.pod for perl or vendor
    DEBPKG:debian/fakeroot - Postpone LD_LIBRARY_PATH evaluation to the binary targets.
    DEBPKG:debian/instmodsh_doc - Debian policy doesn't install .packlist files for core or vendor.
    DEBPKG:debian/ld_run_path - Remove standard libs from LD_RUN_PATH as per Debian policy.
    DEBPKG:debian/libnet_config_path - Set location of libnet.cfg to /etc/perl/Net as /usr may not be writable.
    DEBPKG:debian/mod_paths - Tweak @INC ordering for Debian
    DEBPKG:debian/prune_libs - https://bugs.debian.org/128355 Prune the list of libraries wanted to what we actually need.
    DEBPKG:debian/perlivp - https://bugs.debian.org/510895 Make perlivp skip include directories in /usr/local
    DEBPKG:debian/deprecate-with-apt - https://bugs.debian.org/747628 Point users to Debian packages of deprecated core modules
    DEBPKG:debian/squelch-locale-warnings - https://bugs.debian.org/508764 Squelch locale warnings in Debian package maintainer scripts
    DEBPKG:debian/skip-upstream-git-tests - Skip tests specific to the upstream Git repository
    DEBPKG:debian/patchlevel - https://bugs.debian.org/567489 List packaged patches for 5.26.0-8 in patchlevel.h
    DEBPKG:fixes/document_makemaker_ccflags - https://bugs.debian.org/628522 [rt.cpan.org #68613] Document that CCFLAGS should include $Config{ccflags}
    DEBPKG:debian/find_html2text - https://bugs.debian.org/640479 Configure CPAN::Distribution with correct name of html2text
    DEBPKG:debian/perl5db-x-terminal-emulator.patch - https://bugs.debian.org/668490 Invoke x-terminal-emulator rather than xterm in perl5db.pl
    DEBPKG:debian/cpan-missing-site-dirs - https://bugs.debian.org/688842 Fix CPAN::FirstTime defaults with nonexisting site dirs if a parent is writable
    DEBPKG:fixes/memoize_storable_nstore - [rt.cpan.org #77790] https://bugs.debian.org/587650 Memoize::Storable: respect 'nstore' option not respected
    DEBPKG:debian/regen-skip - Skip a regeneration check in unrelated git repositories
    DEBPKG:debian/makemaker-pasthru - https://bugs.debian.org/758471 Pass LD settings through to subdirectories
    DEBPKG:debian/makemaker-manext - https://bugs.debian.org/247370 Make EU::MakeMaker honour MANnEXT settings in generated manpage headers
    DEBPKG:debian/kfreebsd-softupdates - https://bugs.debian.org/796798 Work around Debian Bug#796798
    DEBPKG:fixes/autodie-scope - https://bugs.debian.org/798096 Fix a scoping issue with "no autodie" and the "system" sub
    DEBPKG:fixes/memoize-pod - [rt.cpan.org #89441] Fix POD errors in Memoize
    DEBPKG:debian/hurd-softupdates - https://bugs.debian.org/822735 Fix t/op/stat.t failures on hurd
    DEBPKG:fixes/math_complex_doc_great_circle - https://bugs.debian.org/697567 [rt.cpan.org #114104] Math::Trig: clarify definition of great_circle_midpoint
    DEBPKG:fixes/math_complex_doc_see_also - https://bugs.debian.org/697568 [rt.cpan.org #114105] Math::Trig: add missing SEE ALSO
    DEBPKG:fixes/math_complex_doc_angle_units - https://bugs.debian.org/731505 [rt.cpan.org #114106] Math::Trig: document angle units
    DEBPKG:fixes/cpan_web_link - https://bugs.debian.org/367291 CPAN: Add link to main CPAN web site
    DEBPKG:fixes/time_piece_doc - https://bugs.debian.org/817925 Time::Piece: Improve documentation for add_months and add_years
    DEBPKG:fixes/extutils_makemaker_reproducible - https://bugs.debian.org/835815 https://bugs.debian.org/834190 Make perllocal.pod files reproducible
    DEBPKG:fixes/file_path_hurd_errno - File-Path: Fix test failure in Hurd due to hard-coded ENOENT
    DEBPKG:debian/hppa_op_optimize_workaround - https://bugs.debian.org/838613 Temporarily lower the optimization of op.c on hppa due to gcc-6 problems
    DEBPKG:debian/installman-utf8 - https://bugs.debian.org/840211 Generate man pages with UTF-8 characters
    DEBPKG:fixes/file_path_chmod_race - https://bugs.debian.org/863870 [rt.cpan.org #121951] Prevent directory chmod race attack.
    DEBPKG:fixes/extutils_file_path_compat - Correct the order of tests of chmod(). (#294)
    DEBPKG:fixes/getopt-long-2 - [rt.cpan.org #120300] Withdraw part of commit 5d9947fb445327c7299d8beb009d609bc70066c0, which tries to implement more GNU getopt_long campatibility. GNU
    DEBPKG:fixes/getopt-long-3 - provide a default value for optional arguments
    DEBPKG:fixes/getopt-long-4 - https://bugs.debian.org/864544 [rt.cpan.org #122068] Fix issue #122068.
    DEBPKG:fixes/fbm-instr-crash - [bb152a4] [perl #131575] don't call Perl_fbm_instr() with negative length
    DEBPKG:fixes/test-builder-reset - https://bugs.debian.org/865894 Reset inside subtest maintains parent
    DEBPKG:debian/CVE-2016-1238/base-pm-amends-pt2 - [a77da41] Limit dotless-INC effect on base.pm with guard:
    DEBPKG:debian/hppa_opmini_optimize_workaround - https://bugs.debian.org/869122 Lower the optimization level of opmini.c on hppa
    DEBPKG:debian/sh4_op_optimize_workaround - https://bugs.debian.org/869373 Also lower the optimization level of op.c and opmini.c on sh4
    DEBPKG:fixes/json-pp-example - [rt.cpan.org #92793] https://bugs.debian.org/871837 fix RT-92793: bug in SYNOPSIS
    DEBPKG:debian/customized - Update customized.dat for files patched in Debian
    DEBPKG:fixes/CVE-2017-12837 - https://bugs.debian.org/875596 [perl #131582] [66288bb] regcomp [perl #131582]
    DEBPKG:fixes/CVE-2017-12883 - https://bugs.debian.org/875597 [perl #131598] [2692dda] PATCH: [perl #131598]


@INC for perl 5.26.0:
    /etc/perl
    /usr/local/lib/x86_64-linux-gnu/perl/5.26.0
    /usr/local/share/perl/5.26.0
    /usr/lib/x86_64-linux-gnu/perl5/5.26
    /usr/share/perl5
    /usr/lib/x86_64-linux-gnu/perl/5.26
    /usr/share/perl/5.26
    /usr/local/lib/site_perl
    /usr/lib/x86_64-linux-gnu/perl-base


Environment for perl 5.26.0:
    HOME=/home/niko
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LC_CTYPE=fi_FI.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/niko/bin:/home/niko/bin:/home/niko/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games:/sbin:/usr/sbin:/sbin:/usr/sbin
    PERL_BADLANG (unset)
    SHELL=/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2017

From @jkeenan

On Sat, 16 Sep 2017 18​:01​:56 GMT, ntyni@​debian.org wrote​:

This is a bug report for perl from Niko Tyni <ntyni@​debian.org>,
generated with the help of perlbug 1.40 running under perl 5.26.0.

-----------------------------------------------------------------
[Please describe your issue here]

As discussed in
https://bugs.debian.org/875927
https://bugs.launchpad.net/ubuntu/+source/libanyevent-
perl/+bug/1717367

a change in glibc 2.26, presumably
https://patchwork.sourceware.org/patch/20800/
causes perl to change its ABI when rebuilt against the new glibc.
This is a serious problem for distributions that need to manage ABI
changes carefully.

The problem seems to be that Perl's global array PL_sig_name[] and
the SIG_SIZE constant ($Config{sig_name}, $Config{sig_size} on the
perl language side) change due to the removal of the (long?) obsolete
SIGUNUSED constant in glibc. In this case, the Async​::Interrupt XS
module
compiles in SIG_SIZE and walks PL_sig_name[] at run time based on
that.
When Perl is rebuilt against the new glibc but Async​::Interrupt is
not,
the latter does not know that the array has become smaller and
accesses
it out of bounds.

Async​::Interrupt is maintained upstream on CPAN; it is not maintained by Perl 5 Porters.

Can you be a bit more specific as to what changes you want in the core distribution?

It looks to me like Perl builds on glibc could pad PL_sig_name[] on
the
maint-* branches when SIGUNUSED is missing (indicating glibc >= 2.26)
to ensure ABI compatibility. Would that be feasible?

For bleadperl / 5.27, where there are no ABI guarantees, it seems
appropriate to start filtering out SIGUNUSED altogether regardless of
the glibc version, given its apparent obsolete status.

From 'man glibc' on Linux I read​:

#####
By far the most widely used C library on Linux is the GNU C Library ⟨http​://www.gnu.org/software
/libc/⟩, often referred to as glibc. This is the C library that is nowadays used in all major Linux distributions.
#####

So, is this problems specific to Linux?

Similar code seems to be in IO​::AIO, Coro and Ev, see s_signum()
in schmorp.h.

Those are also maintained upstream on CPAN.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2017

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Sep 17, 2017

From @ntyni

On Sat, Sep 16, 2017 at 01​:26​:24PM -0700, James E Keenan via RT wrote​:

On Sat, 16 Sep 2017 18​:01​:56 GMT, ntyni@​debian.org wrote​:

The problem seems to be that Perl's global array PL_sig_name[] and
the SIG_SIZE constant ($Config{sig_name}, $Config{sig_size} on the
perl language side) change due to the removal of the (long?) obsolete
SIGUNUSED constant in glibc. In this case, the Async​::Interrupt XS
module compiles in SIG_SIZE and walks PL_sig_name[] at run time based on
that. When Perl is rebuilt against the new glibc but Async​::Interrupt is
not, the latter does not know that the array has become smaller and
accesses it out of bounds.

Async​::Interrupt is maintained upstream on CPAN; it is not maintained by Perl 5 Porters.

Of course.

Can you be a bit more specific as to what changes you want in the core distribution?

I want stable Perl releases to have the same binary interface whether
built with glibc 2.25 or 2.26. Currently this is not the case.
That leads to segmentation faults when certain CPAN modules are built
against one binary interface of Perl but run on the other.

I suggest that this could be fixed in maint-5.* by adding a placeholder
for the removed SIGUNUSED entry in PL_sig_name[] on glibc >= 2.26.

I also suggest that bleadperl could ignore SIGUNUSED on glibc < 2.26,
which would eliminate the issue for future stable releases from 5.28
onward.

I can look at implementing these if this is considered feasible and
desirable.

I suppose an alternative to the above approach would be to declare that
Async​::Interrupt et al. are fiddling with Perl internals in a way they
shouldn't, and that those internals are not covered by ABI stability
guarantees.

I would not be very happy with such an outcome, as it would cause pain
for us downstream. We would then have to push for getting those CPAN
modules fixed (which may be challenging), or artificially make them have
very strict dependencies on the Perl versions they were built with
(which does not scale.)

#####
By far the most widely used C library on Linux is the GNU C Library ⟨http​://www.gnu.org/software
/libc/⟩, often referred to as glibc. This is the C library that is nowadays used in all major Linux distributions.
#####

So, is this problems specific to Linux?

Essentially yes, although there are a some other minority platforms such
us GNU/Hurd and GNU/kFreeBSD that have glibc with a non-linux kernel
and are probably affected too.

--
Niko Tyni ntyni@​debian.org

@p5pRT
Copy link
Author

p5pRT commented Sep 17, 2017

From @Leont

On Sun, Sep 17, 2017 at 10​:25 AM, Niko Tyni <ntyni@​debian.org> wrote​:

I want stable Perl releases to have the same binary interface whether
built with glibc 2.25 or 2.26. Currently this is not the case.
That leads to segmentation faults when certain CPAN modules are built
against one binary interface of Perl but run on the other.

This is one of those cases where not having described much of our code
bites. For that matter, even the way core itself uses SIG_SIZE is a fairly
confused.

I suggest that this could be fixed in maint-5.* by adding a placeholder
for the removed SIGUNUSED entry in PL_sig_name[] on glibc >= 2.26.

I also suggest that bleadperl could ignore SIGUNUSED on glibc < 2.26,
which would eliminate the issue for future stable releases from 5.28
onward.

I can look at implementing these if this is considered feasible and
desirable.

I suppose an alternative to the above approach would be to declare that
Async​::Interrupt et al. are fiddling with Perl internals in a way they
shouldn't, and that those internals are not covered by ABI stability
guarantees.

I would not be very happy with such an outcome, as it would cause pain
for us downstream. We would then have to push for getting those CPAN
modules fixed (which may be challenging), or artificially make them have
very strict dependencies on the Perl versions they were built with
(which does not scale.)

The patch to the modules would be little more than removing code; they'd
just have to use the whichsig (perl 1.0) /whichsig_sv (perl 5.16) API
instead of reimplementing it. Alternatively s_signame could check for the
end of PL_sig_name by checking for a null-pointer instead of assuming it
has SIG_SIZE elements. Unlike the other solutions that will prevent this
kind of situation from happening again.

Leon

@p5pRT
Copy link
Author

p5pRT commented Sep 18, 2017

From @tonycoz

On Sat, 16 Sep 2017 11​:01​:56 -0700, ntyni@​debian.org wrote​:

As discussed in
https://bugs.debian.org/875927
https://bugs.launchpad.net/ubuntu/+source/libanyevent-
perl/+bug/1717367

a change in glibc 2.26, presumably
https://patchwork.sourceware.org/patch/20800/
causes perl to change its ABI when rebuilt against the new glibc.
This is a serious problem for distributions that need to manage ABI
changes carefully.

The problem seems to be that Perl's global array PL_sig_name[] and
the SIG_SIZE constant ($Config{sig_name}, $Config{sig_size} on the
perl language side) change due to the removal of the (long?) obsolete
SIGUNUSED constant in glibc. In this case, the Async​::Interrupt XS
module
compiles in SIG_SIZE and walks PL_sig_name[] at run time based on
that.
When Perl is rebuilt against the new glibc but Async​::Interrupt is
not,
the latter does not know that the array has become smaller and
accesses
it out of bounds.

It looks to me like Perl builds on glibc could pad PL_sig_name[] on
the
maint-* branches when SIGUNUSED is missing (indicating glibc >= 2.26)
to ensure ABI compatibility. Would that be feasible?

The check would need to be more complex than that - other platforms may not provide SIGUNUSED (darwin doesn't)

I think workaround for this belongs downstream - in Debian/Ubuntu - the incompatibility was introduced by a change in the platform.

Building with something like​:

./Configure -des -Dusedevel -Dsig_name_init='"ZERO", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "BUS", "FPE", "KILL", "USR1", "SEGV", "USR2", "PIPE", "ALRM", "TERM", "STKFLT", "CHLD", "CONT", "STOP", "TSTP", "TTIN", "TTOU", "URG", "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "IO", "PWR", "SYS", "NUM32", "NUM33", "RTMIN", "NUM35", "NUM36", "NUM37", "NUM38", "NUM39", "NUM40", "NUM41", "NUM42", "NUM43", "NUM44", "NUM45", "NUM46", "NUM47", "NUM48", "NUM49", "NUM50", "NUM51", "NUM52", "NUM53", "NUM54", "NUM55", "NUM56", "NUM57", "NUM58", "NUM59", "NUM60", "NUM61", "NUM62", "NUM63", "RTMAX", "IOT", "CLD", "POLL", "UNUSED", 0' -Dsig_num_init='0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 6, 17, 29, 31, 0'

(and probably sig_name and sig_count too) though ugly should retain binary compatibility.

For bleadperl / 5.27, where there are no ABI guarantees, it seems
appropriate to start filtering out SIGUNUSED altogether regardless of
the glibc version, given its apparent obsolete status.

The real fix for blead would probably be to make SIG_SIZE a variable, or for code that iterates over sig_num[] to terminate on the 0 at the end.

Similar code seems to be in IO​::AIO, Coro and Ev, see s_signum()
in schmorp.h.

s_signum() itself has a bug​:

$ perl -MAsync​::Interrupt -le 'print Async​::Interrupt​::sig2num("POLL")'
67

(Debian packaged x86_64 perl and module)

Tony

@p5pRT
Copy link
Author

p5pRT commented Sep 18, 2017

From @ntyni

On Sun, Sep 17, 2017 at 08​:47​:43PM -0700, Tony Cook via RT wrote​:

On Sat, 16 Sep 2017 11​:01​:56 -0700, ntyni@​debian.org wrote​:

It looks to me like Perl builds on glibc could pad PL_sig_name[] on
the
maint-* branches when SIGUNUSED is missing (indicating glibc >= 2.26)
to ensure ABI compatibility. Would that be feasible?

The check would need to be more complex than that - other platforms may not provide SIGUNUSED (darwin doesn't)

Sure. Sorry for cutting corners there.

I think workaround for this belongs downstream - in Debian/Ubuntu - the incompatibility was introduced by a change in the platform.

Okay. I can see the point. I note that Fedora etc. are probably going
to be affected too, though they may have a lower threshold for just
rebuilding the world to solve it.

./Configure -des -Dusedevel -Dsig_name_init='"ZERO", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "BUS", "FPE", "KILL", "USR1", "SEGV", "USR2", "PIPE", "ALRM", "TERM", "STKFLT", "CHLD", "CONT", "STOP", "TSTP", "TTIN", "TTOU", "URG", "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "IO", "PWR", "SYS", "NUM32", "NUM33", "RTMIN", "NUM35", "NUM36", "NUM37", "NUM38", "NUM39", "NUM40", "NUM41", "NUM42", "NUM43", "NUM44", "NUM45", "NUM46", "NUM47", "NUM48", "NUM49", "NUM50", "NUM51", "NUM52", "NUM53", "NUM54", "NUM55", "NUM56", "NUM57", "NUM58", "NUM59", "NUM60", "NUM61", "NUM62", "NUM63", "RTMAX", "IOT", "CLD", "POLL", "UNUSED", 0' -Dsig_num_init='0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 6, 17, 29, 31, 0'

Urgh, but sure. We'll look at the options on the Debian side.

For bleadperl / 5.27, where there are no ABI guarantees, it seems
appropriate to start filtering out SIGUNUSED altogether regardless of
the glibc version, given its apparent obsolete status.

The real fix for blead would probably be to make SIG_SIZE a variable, or for code that iterates over sig_num[] to terminate on the 0 at the end.

Making SIG_SIZE a variable on the Perl core side works for me and would
avoid this in the future.

Fixing the 'consumers' would be ideal of course. Is there something
that could/should be done on the Perl core side to push this? Compiler
warnings, hiding some of the symbols with '#ifdef PERL_CORE', just
documentation?

Similar code seems to be in IO​::AIO, Coro and Ev, see s_signum()
in schmorp.h.

My code searches also turned up
https://metacpan.org/source/ABERGMAN/Devel-GC-Helper-0.25/Helper.xs#L359
in addition to the schmorp.h implementations. I suppose it's also possible
that there's user (non-CPAN) code with something like this out there.

s_signum() itself has a bug​:

$ perl -MAsync​::Interrupt -le 'print Async​::Interrupt​::sig2num("POLL")'
67

Indeed. AIUI, it should be looking the signal number up in PL_sig_num[]
instead of assuming the signal number is same as the array index.

I'll try to push for fixing the s_signum() implementations one way or
another and see how it goes.

Thanks for looking at this,
--
Niko

@p5pRT
Copy link
Author

p5pRT commented Sep 18, 2017

From @doughera88

On Sun, Sep 17, 2017 at 11​:25​:08AM +0300, Niko Tyni wrote​:

On Sat, Sep 16, 2017 at 01​:26​:24PM -0700, James E Keenan via RT wrote​:

On Sat, 16 Sep 2017 18​:01​:56 GMT, ntyni@​debian.org wrote​:

The problem seems to be that Perl's global array PL_sig_name[] and
the SIG_SIZE constant ($Config{sig_name}, $Config{sig_size} on the
perl language side) change due to the removal of the (long?) obsolete
SIGUNUSED constant in glibc. In this case, the Async​::Interrupt XS
module compiles in SIG_SIZE and walks PL_sig_name[] at run time based on
that. When Perl is rebuilt against the new glibc but Async​::Interrupt is
not, the latter does not know that the array has become smaller and
accesses it out of bounds.

Async​::Interrupt is maintained upstream on CPAN; it is not maintained by Perl 5 Porters.

Of course.

Can you be a bit more specific as to what changes you want in the core distribution?

I want stable Perl releases to have the same binary interface whether
built with glibc 2.25 or 2.26. Currently this is not the case.
That leads to segmentation faults when certain CPAN modules are built
against one binary interface of Perl but run on the other.

In general, Perl can't guarantee that the same binary interface is
available whether built with glibc 2.x or 2.x+1, though it very often
works just fine. I understand and agree that it is surprising that
an apparently trivial removal of a normally-unused #define in signal.h
causes this problem down the line, but that is what happened here.

I suggest that this could be fixed in maint-5.* by adding a placeholder
for the removed SIGUNUSED entry in PL_sig_name[] on glibc >= 2.26.

I also suggest that bleadperl could ignore SIGUNUSED on glibc < 2.26,
which would eliminate the issue for future stable releases from 5.28
onward.

I can look at implementing these if this is considered feasible and
desirable.

These both sound like sensible workarounds.
Unfortunately, the signal name generating code doesn't offer any easy
way to inject OS-specific tweaks. On the other hand, it should be
easy to simply override the whole thing at the distribution level by
specifying the signal arrays in a custom config.over file. That would
be my first inclination.

I suppose an alternative to the above approach would be to declare that
Async​::Interrupt et al. are fiddling with Perl internals in a way they
shouldn't, and that those internals are not covered by ABI stability
guarantees.

I don't think I would put it that way. It's more that perl is fiddling
with glibc internals by attempting to expose all the names for signals,
and those names have changed. Accordingly, perl needs to be recompiled.

I would not be very happy with such an outcome, as it would cause pain
for us downstream. We would then have to push for getting those CPAN
modules fixed (which may be challenging), or artificially make them have
very strict dependencies on the Perl versions they were built with
(which does not scale.)

I think I understand the pain, but I fear that's ultimately the only sure
answer. I don't think it's an artificial dependence -- Async​::Interrupt
really does depend on perl internals, so that when perl is rebuilt,
Async​::Interrupt should be as well. We're just used to most such
dependencies being much less fragile.

Still I agree that such breakage is rare, and there is value in working
around this specific instance. I recommend tyring fiddling with a
config.over file to set sig_name_init, sig_num_init, and sig_size.

--
  Andy Dougherty doughera@​lafayette.edu

@p5pRT
Copy link
Author

p5pRT commented Sep 27, 2017

From @tonycoz

On Mon, 18 Sep 2017 10​:46​:17 -0700, ntyni@​debian.org wrote​:

On Sun, Sep 17, 2017 at 08​:47​:43PM -0700, Tony Cook via RT wrote​:

The real fix for blead would probably be to make SIG_SIZE a variable,
or for code that iterates over sig_num[] to terminate on the 0 at the
end.

Making SIG_SIZE a variable on the Perl core side works for me and
would
avoid this in the future.

Making SIG_SIZE a variable would further break Async​::Interrupt, since it contains​:

  static async_t *sig_async [SIG_SIZE];

which would fail to compile if SIG_SIZE isn't a compile-time constant.

So I don't think there's anything else we can do to handle this.

Tony

@p5pRT
Copy link
Author

p5pRT commented Oct 12, 2017

From @tonycoz

On Tue, 26 Sep 2017 21​:27​:42 -0700, tonyc wrote​:

On Mon, 18 Sep 2017 10​:46​:17 -0700, ntyni@​debian.org wrote​:

On Sun, Sep 17, 2017 at 08​:47​:43PM -0700, Tony Cook via RT wrote​:

The real fix for blead would probably be to make SIG_SIZE a
variable,
or for code that iterates over sig_num[] to terminate on the 0 at
the
end.

Making SIG_SIZE a variable on the Perl core side works for me and
would
avoid this in the future.

Making SIG_SIZE a variable would further break Async​::Interrupt, since
it contains​:

static async_t *sig_async [SIG_SIZE];

which would fail to compile if SIG_SIZE isn't a compile-time constant.

So I don't think there's anything else we can do to handle this.

So closing.

Tony

@p5pRT
Copy link
Author

p5pRT commented Oct 12, 2017

@tonycoz - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant