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

stack checks make -DDEBUGGING builds incompatible with normal ones #16607

Open
p5pRT opened this issue Jul 3, 2018 · 6 comments
Open

stack checks make -DDEBUGGING builds incompatible with normal ones #16607

p5pRT opened this issue Jul 3, 2018 · 6 comments
Assignees

Comments

@p5pRT
Copy link

p5pRT commented Jul 3, 2018

Migrated from rt.perl.org#133327 (status was 'open')

Searchable as RT133327$

@p5pRT
Copy link
Author

p5pRT commented Jul 3, 2018

From @ntyni

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


The stack 'high water mark' checks introduced in v5.27.1-66-g87058c31e9
seem to have introduced an incompatibility when XS modules are built
with a normal Perl but run with a (otherwise similarly configured)
-DDEBUGGING Perl.

We're using this combination in Debian as we offer a -DDEBUGGING
interpreter (called /usr/bin/debugperl) in a separate package for the
benefit of our users, but we don't rebuild all the XS modules with that.

It would be useful for us to keep these builds binary compatible.
Last time an incompatibility happened it was deemed worth fixing​:
see [perl #127212] and Aaron's related commit message in 5ca5a62 .

Some symptoms of the breakage​:

# debugperl -MDateTime -e 'print DateTime->today'
panic​: XSUB DateTime​::_rd2ymd (DateTime.c) failed to extend arg stack​: base=55b63e6b3b48, sp=55b63e6b3b80, hwm=5
5b63e6b3b68

# printf '<AAA>\n<DDD/><CCC><DDD>\n</DDD></CCC>\n</AAA>' | debugperl -MXML​::LibXML -e 'XML​::LibXML->new->parse_f
h(*STDIN)->documentElement->childNodes; '
panic​: XSUB XML​::LibXML​::Node​::_childNodes (LibXML.c) failed to extend arg stack​: base=561827c61b48, sp=561827c6
1b68, hwm=561827c61b60

These are just examples, and I believe the fault is not with the
modules.

If I grok this correctly, the problem is that EXTEND only updates
the high-water mark when the DEBUGGING preprocessor symbol is defined
(i.e. the choice is done at compile time.) If the XS module is built
without -DDEBUGGING in ccflags (as is the case for Debian XS module
packages), the high-water mark doesn't get updated. If the interpreter
side is built with -DDEBUGGING (as our debugperl is), it will still
check the hwm, resulting in the panic.

Would it be feasible move the -DDEBUGGING check in EXTEND to run time,
for instance by calling a function that's a no-op in non-DEBUGGING
interpreters, or checking for an interpreter variable that's only set
in -DDEBUGGING builds? This has a runtime cost, but I'm not sure how
significant that is?

Many thanks for your work, and congratulations for another great Perl
stable release!

( This is also https://bugs.debian.org/902779 )



Flags​:
  category=core
  severity=medium


Site configuration information for perl 5.28.0​:

Configured by Debian at Mon Jun 25 19​:20​:16 UTC 2018.

Summary of my perl5 (revision 5 version 28 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-yH5Gew/perl-5.28.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.28 -Darchlib=/usr/lib/x86_64-linux-gnu/perl/5.28 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/x86_64-linux-gnu/perl5/5.28 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.28.0 -Dsitearch=/usr/local/lib/x86_64-linux-gnu/perl/5.28.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 -Ui_xlocale -Uversiononly -DDEBUGGING=-g -Doptimize=-O2 -dEs -Duseshrplib -Dlibperl=libperl.so.5.28.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.3.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.27.so
  so=so
  useshrplib=true
  libperl=libperl.so.5.28
  gnulibc_version='2.27'
  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/perlivp - https://bugs.debian.org/510895 Make perlivp skip include directories in /usr/local
  DEBPKG​:debian/squelch-locale-warnings - https://bugs.debian.org/508764 Squelch locale warnings in Debian package maintainer scripts
  DEBPKG​:debian/patchlevel - https://bugs.debian.org/567489 List packaged patches for 5.28.0-1 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/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​: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/getopt-long-4 - https://bugs.debian.org/864544 [rt.cpan.org #122068] Fix issue #122068.
  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​:debian/perldoc-pager - https://bugs.debian.org/870340 [rt.cpan.org #120229] Fix perldoc terminal escapes when sensible-pager is less
  DEBPKG​:debian/prune_libs - https://bugs.debian.org/128355 Prune the list of libraries wanted to what we actually need.
  DEBPKG​:debian/mod_paths - Tweak @​INC ordering for Debian
  DEBPKG​:debian/configure-regen - https://bugs.debian.org/762638 Regenerate Configure et al. after probe unit changes
  DEBPKG​:debian/deprecate-with-apt - https://bugs.debian.org/747628 Point users to Debian packages of deprecated core modules


@​INC for perl 5.28.0​:
  /etc/perl
  /usr/local/lib/x86_64-linux-gnu/perl/5.28.0
  /usr/local/share/perl/5.28.0
  /usr/lib/x86_64-linux-gnu/perl5/5.28
  /usr/share/perl5
  /usr/lib/x86_64-linux-gnu/perl/5.28
  /usr/share/perl/5.28
  /usr/local/lib/site_perl
  /usr/lib/x86_64-linux-gnu/perl-base


Environment for perl 5.28.0​:
  HOME=/home/ntyni
  LANG (unset)
  LANGUAGE (unset)
  LD_LIBRARY_PATH=/usr/lib/libeatmydata
  LOGDIR (unset)
  PATH=/usr/local/bin​:/usr/bin​:/bin​:/usr/local/games​:/usr/games
  PERL_BADLANG (unset)
  SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Jul 3, 2018

From @dur-randir

On Tue, 03 Jul 2018 08​:49​:38 -0700, ntyni@​debian.org wrote​:

Would it be feasible move the -DDEBUGGING check in EXTEND to run time,
for instance by calling a function that's a no-op in non-DEBUGGING
interpreters, or checking for an interpreter variable that's only set
in -DDEBUGGING builds? This has a runtime cost, but I'm not sure how
significant that is?

A lot of module authors use EXTEND pessimistically, like the whole XPUSH* macro family, effectively calling EXTEND in O(return values) number of times. And the suggested approach will surely penalize this. Cpan search showed more than 1000 distribution with this pattern.

@p5pRT
Copy link
Author

p5pRT commented Jul 3, 2018

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

@p5pRT
Copy link
Author

p5pRT commented Jul 4, 2018

From @iabyn

On Tue, Jul 03, 2018 at 08​:49​:39AM -0700, Niko Tyni (via RT) wrote​:

The stack 'high water mark' checks introduced in v5.27.1-66-g87058c31e9
seem to have introduced an incompatibility when XS modules are built
with a normal Perl but run with a (otherwise similarly configured)
-DDEBUGGING Perl.

We're using this combination in Debian as we offer a -DDEBUGGING
interpreter (called /usr/bin/debugperl) in a separate package for the
benefit of our users, but we don't rebuild all the XS modules with that.
[snip]
If I grok this correctly, the problem is that EXTEND only updates
the high-water mark when the DEBUGGING preprocessor symbol is defined
(i.e. the choice is done at compile time.) If the XS module is built
without -DDEBUGGING in ccflags (as is the case for Debian XS module
packages), the high-water mark doesn't get updated. If the interpreter
side is built with -DDEBUGGING (as our debugperl is), it will still
check the hwm, resulting in the panic.

Would it be feasible move the -DDEBUGGING check in EXTEND to run time,
for instance by calling a function that's a no-op in non-DEBUGGING
interpreters, or checking for an interpreter variable that's only set
in -DDEBUGGING builds? This has a runtime cost, but I'm not sure how
significant that is?

I wouldn't like any solution that adds extra run-time overhead to
non-debugging-build XS modules.

To avoid that, the debugging part of the perl interperter would need to be
able to determine when it calls an XS sub or a non-core pp_foo() function,
whether that code what compiled under DEBUGGING. I can't think of a way to
determine this.

A fall-back position would be to add support for a PERL_NO_STACK_HWM_CHECK
define / build option which would completely disable checks, even if
the core was built with DEBUGGING. That would of course disable
the check even for XS modules built with DEBUGGING.

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

@p5pRT
Copy link
Author

p5pRT commented Jul 4, 2018

From @bulk88

On Tue, 03 Jul 2018 08​:49​:38 -0700, ntyni@​debian.org wrote​:

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

-----------------------------------------------------------------

The stack 'high water mark' checks introduced in v5.27.1-66-
g87058c31e9
seem to have introduced an incompatibility when XS modules are built
with a normal Perl but run with a (otherwise similarly configured)
-DDEBUGGING Perl.

We're using this combination in Debian as we offer a -DDEBUGGING
interpreter (called /usr/bin/debugperl) in a separate package for the
benefit of our users, but we don't rebuild all the XS modules with
that.

It would be useful for us to keep these builds binary compatible.
Last time an incompatibility happened it was deemed worth fixing​:
see [perl #127212] and Aaron's related commit message in 5ca5a62
.

Some symptoms of the breakage​:

# debugperl -MDateTime -e 'print DateTime->today'
panic​: XSUB DateTime​::_rd2ymd (DateTime.c) failed to extend arg stack​:
base=55b63e6b3b48, sp=55b63e6b3b80, hwm=5
5b63e6b3b68

# printf '<AAA>\n<DDD/><CCC><DDD>\n</DDD></CCC>\n</AAA>' | debugperl
-MXML​::LibXML -e 'XML​::LibXML->new->parse_f
h(*STDIN)->documentElement->childNodes; '
panic​: XSUB XML​::LibXML​::Node​::_childNodes (LibXML.c) failed to extend
arg stack​: base=561827c61b48, sp=561827c6
1b68, hwm=561827c61b60

These are just examples, and I believe the fault is not with the
modules.

If I grok this correctly, the problem is that EXTEND only updates
the high-water mark when the DEBUGGING preprocessor symbol is defined
(i.e. the choice is done at compile time.) If the XS module is built
without -DDEBUGGING in ccflags (as is the case for Debian XS module
packages), the high-water mark doesn't get updated. If the
interpreter
side is built with -DDEBUGGING (as our debugperl is), it will still
check the hwm, resulting in the panic.

Would it be feasible move the -DDEBUGGING check in EXTEND to run time,
for instance by calling a function that's a no-op in non-DEBUGGING
interpreters, or checking for an interpreter variable that's only set
in -DDEBUGGING builds? This has a runtime cost, but I'm not sure how
significant that is?

DEBUGGING is a build option, either the whole interp has it or it doesn't. P5P already questionably added struct fields that are unused on non-DEBUGGING to make DEBUGGING vs not binary compatible but I disagree with that quick fix. Perhaps perl.h should be tweaked to turn on libc assert() without rest of DEBUGGING features and /usr/bin/debugperl should be compiled with that just-libc-assert flag.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Jul 4, 2018

From @ntyni

On Wed, Jul 04, 2018 at 05​:28​:21AM -0700, Dave Mitchell via RT wrote​:

On Tue, Jul 03, 2018 at 08​:49​:39AM -0700, Niko Tyni (via RT) wrote​:

Would it be feasible move the -DDEBUGGING check in EXTEND to run time,
for instance by calling a function that's a no-op in non-DEBUGGING
interpreters, or checking for an interpreter variable that's only set
in -DDEBUGGING builds? This has a runtime cost, but I'm not sure how
significant that is?

I wouldn't like any solution that adds extra run-time overhead to
non-debugging-build XS modules.

To avoid that, the debugging part of the perl interperter would need to be
able to determine when it calls an XS sub or a non-core pp_foo() function,
whether that code what compiled under DEBUGGING. I can't think of a way to
determine this.

I see.

A fall-back position would be to add support for a PERL_NO_STACK_HWM_CHECK
define / build option which would completely disable checks, even if
the core was built with DEBUGGING. That would of course disable
the check even for XS modules built with DEBUGGING.

Yeah, I suppose we will have to disable the checks in the Debian
debugperl for now. A supported way to do that in 5.30 would be very
much appreciated.

Thanks for looking at this,
--
Niko

@xenu xenu removed the Severity Low label Dec 29, 2021
@tonycoz tonycoz self-assigned this Oct 8, 2023
tonycoz added a commit to tonycoz/perl5 that referenced this issue Oct 30, 2023
Since the interpreter struct is (meant to be) compatible between
DEBUGGING and non-DEBUGGING this allows a vendor to build a normal
perl and then build a DEBUGGING perl binary compatible with that
non-DEBUGGING perl that doesn't panic on the first XS call.

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

No branches or pull requests

3 participants