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

Bleadperl v5.27.1-66-g87058c31e9 breaks TONYC/Imager-1.005.tar.gz #16121

Closed
p5pRT opened this issue Aug 21, 2017 · 12 comments
Closed

Bleadperl v5.27.1-66-g87058c31e9 breaks TONYC/Imager-1.005.tar.gz #16121

p5pRT opened this issue Aug 21, 2017 · 12 comments
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s)

Comments

@p5pRT
Copy link

p5pRT commented Aug 21, 2017

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

Searchable as RT131938$

@p5pRT
Copy link
Author

p5pRT commented Aug 21, 2017

From @andk

Affected are -DDEBUGGING builds only.

bisect


commit 87058c3
Author​: David Mitchell <davem@​iabyn.com>
Date​: Tue Jun 13 09​:11​:13 2017 +0100

  add PL_curstackinfo->si_stack_hwm

other distros also affected


SHLOMIF/Math-Cephes-0.5305.tar.gz
LETO/Math-GSL-0.39.tar.gz
ILYAZ/modules/Math-Pari-2.01080900.zip
PJB/Math-WalshTransform-1.17.tar.gz
HANK/Text-Aspell-0.09.tar.gz

cpantesters report


http​://www.cpantesters.org/cpan/report/5156c90c-69c1-11e7-b361-273c01f1587f

perl -V


Summary of my perl5 (revision 5 version 27 subversion 2) configuration​:
  Commit id​: ec268cc
  Platform​:
  osname=linux
  osvers=4.9.0-2-amd64
  archname=x86_64-linux-ld
  uname='linux k93msid 4.9.0-2-amd64 #1 smp debian 4.9.18-1 (2017-03-30) x86_64 gnulinux '
  config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.1-239-gec268cc8df/fc43 -Dmyhostname=k93msid -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Dlibswanted=cl pthread socket inet nsl gdbm dbm malloc dl ld sun m crypt sec util c cposix posix ucb BSD gdbm_compat -Uuseithreads -Duselongdouble -DEBUGGING=both'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=undef
  usemultiplicity=undef
  use64bitint=define
  use64bitall=define
  uselongdouble=define
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='cc'
  ccflags ='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
  optimize='-O2 -g'
  cppflags='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='6.3.0 20170406'
  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='long double'
  nvsize=16
  Off_t='off_t'
  lseeksize=8
  alignbytes=16
  prototype=define
  Linker and Libraries​:
  ld='cc'
  ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/6/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=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.24.so
  so=so
  useshrplib=false
  libperl=libperl.a
  gnulibc_version='2.24'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags='-Wl,-E'
  cccdlflags='-fPIC'
  lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl)​:
  Compile-time options​:
  DEBUGGING
  HAS_TIMES
  PERLIO_LAYERS
  PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_MALLOC_WRAP
  PERL_OP_PARENT
  PERL_PRESERVE_IVUV
  PERL_USE_DEVEL
  USE_64_BIT_ALL
  USE_64_BIT_INT
  USE_LARGE_FILES
  USE_LOCALE
  USE_LOCALE_COLLATE
  USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC
  USE_LOCALE_TIME
  USE_LONG_DOUBLE
  USE_PERLIO
  USE_PERL_ATOF
  Built under linux
  Compiled at Jul 15 2017 21​:32​:42
  %ENV​:
  PERL5LIB=""
  PERL5OPT=""
  PERL5_CPANPLUS_IS_RUNNING="4825"
  PERL5_CPAN_IS_RUNNING="4825"
  PERL_CANARY_STABILITY_NOPROMPT="1"
  PERL_MM_USE_DEFAULT="1"
  PERL_USE_UNSAFE_INC="1"
  @​INC​:
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.1-239-gec268cc8df/fc43/lib/site_perl/5.27.2/x86_64-linux-ld
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.1-239-gec268cc8df/fc43/lib/site_perl/5.27.2
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.1-239-gec268cc8df/fc43/lib/5.27.2/x86_64-linux-ld
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.1-239-gec268cc8df/fc43/lib/5.27.2
  .
 
--
andreas

@p5pRT
Copy link
Author

p5pRT commented Aug 21, 2017

From @tonycoz

On Sun, 20 Aug 2017 22​:56​:20 -0700, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

Affected are -DDEBUGGING builds only.

bisect
------
commit 87058c3
Author​: David Mitchell <davem@​iabyn.com>
Date​: Tue Jun 13 09​:11​:13 2017 +0100

add PL_curstackinfo->si_stack_hwm

The Imager failure is a bug in Imager - i_errors() isn't extending the stack.

Tony

@p5pRT
Copy link
Author

p5pRT commented Aug 21, 2017

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

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2017

From @jkeenan

On Mon, 21 Aug 2017 05​:56​:20 GMT, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

Affected are -DDEBUGGING builds only.

bisect
------
commit 87058c3
Author​: David Mitchell <davem@​iabyn.com>
Date​: Tue Jun 13 09​:11​:13 2017 +0100

add PL_curstackinfo->si_stack_hwm

other distros also affected
---------------------------
SHLOMIF/Math-Cephes-0.5305.tar.gz
LETO/Math-GSL-0.39.tar.gz
ILYAZ/modules/Math-Pari-2.01080900.zip
PJB/Math-WalshTransform-1.17.tar.gz
HANK/Text-Aspell-0.09.tar.gz

cpantesters report
------------------
http​://www.cpantesters.org/cpan/report/5156c90c-69c1-11e7-b361-
273c01f1587f

Examples of failures​:

#####
[Math-Cephes-0.5305] 521 $ /testing/blead/bin/prove -I/testing/blead/lib -vb t/elliptics.t t/misc.t
t/elliptics.t ..
1..10
ok 1
ok 2
ok 3
ok 4
ok 5
panic​: previous op failed to extend arg stack​: base=27d8b50, sp=27d8b78, hwm=27d8b68
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 5/10 subtests
t/misc.t .......
1..33
ok 1
ok 2
ok 3
ok 4
panic​: previous op failed to extend arg stack​: base=17b5b50, sp=17b5b68, hwm=17b5b60
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 29/33 subtests

Test Summary Report


t/elliptics.t (Wstat​: 65280 Tests​: 5 Failed​: 0)
  Non-zero exit status​: 255
  Parse errors​: Bad plan. You planned 10 tests but ran 5.
t/misc.t (Wstat​: 65280 Tests​: 4 Failed​: 0)
  Non-zero exit status​: 255
  Parse errors​: Bad plan. You planned 33 tests but ran 4.
Files=2, Tests=9, 0 wallclock secs ( 0.02 usr 0.01 sys + 0.01 cusr 0.00 csys = 0.04 CPU)
Result​: FAIL
#####
$ make test
"/home/jkeenan/testing/blead/bin/perl" -MExtUtils​::Command​::MM -e 'cp_nonempty' -- WalshTransform.bs blib/arch/auto/Math/WalshTransform/WalshTransform.bs 644
PERL_DL_NONLAZY=1 "/home/jkeenan/testing/blead/bin/perl" "-Iblib/lib" "-Iblib/arch" test.pl
1..16
Use of assignment to $[ is deprecated at blib/lib/Math/WalshTransform.pm line 196.
ok 1 - Hadamard transform
ok 2 - Walsh transform
ok 3 - Inverse Hadamard transform
ok 4 - Inverse Walsh transform
ok 5 - Hadamard transform and inverse
ok 6 - Walsh transform and inverse
ok 7 - Hadamard to Walsh
ok 8 - Walsh to Hadamard
panic​: previous op failed to extend arg stack​: base=2d6f500, sp=2d70500, hwm=2d6f518
# Looks like your test exited with 255 just after 8.
Makefile​:1009​: recipe for target 'test_dynamic' failed
make​: *** [test_dynamic] Error 255
#####

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

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2017

From @iabyn

On Sun, Aug 20, 2017 at 10​:56​:21PM -0700, Andreas J. Koenig via RT wrote​:

SHLOMIF/Math-Cephes-0.5305.tar.gz

At least one failure here is due to XS(_wrap_ellpj) trying to repeatedly
extend the stack by 1 before storing a new value, but without increasing
SP each time, So all the EXTEND(SP,1)'s after the first don't extend the
stack any further.

--
A problem shared is a problem doubled.

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2017

From @iabyn

On Sun, Aug 20, 2017 at 10​:56​:21PM -0700, Andreas J. Koenig via RT wrote​:

LETO/Math-GSL-0.39.tar.gz

In _wrap_gsl_fit_linear(), repeatedly doing

  EXTEND(sp,1);
  ST(argvi) = ...
  argvi++

but since sp isn't being advanced at the same time as argvi, the
EXTEND()s don't accumlate.

ILYAZ/modules/Math-Pari-2.01080900.zip

This XS code has​:

  EXTEND(sp, 3); /* Got cv + 0, return 4. */
  PUSHs(sv_2mortal(newSViv(SVnumtotal)));
  PUSHs(sv_2mortal(newSViv(SVnum)));
  PUSHs(sv_2mortal(newSViv(onStack)));
  PUSHs(sv_2mortal(newSViv(offStack)));

which, despite the code comment, only extends the stack by 3, not 4.

PJB/Math-WalshTransform-1.17.tar.gz

This XS module, in product(), returns lots of values without ever
extending the stack. From a quick glance, that oversight seems to be
common to most of the functions in in the XS code.

HANK/Text-Aspell-0.09.tar.gz

list_dictionaries() pushes a bunch of items on the stack without
extending it.

--
No man treats a motor car as foolishly as he treats another human being.
When the car will not go, he does not attribute its annoying behaviour to
sin, he does not say, You are a wicked motorcar, and I shall not give you
any more petrol until you go. He attempts to find out what is wrong and
set it right.
  -- Bertrand Russell,
  Has Religion Made Useful Contributions to Civilization?

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2017

From @jkeenan

On Wed, 23 Aug 2017 12​:06​:09 GMT, davem wrote​:

On Sun, Aug 20, 2017 at 10​:56​:21PM -0700, Andreas J. Koenig via RT wrote​:

LETO/Math-GSL-0.39.tar.gz

In _wrap_gsl_fit_linear(), repeatedly doing

EXTEND\(sp\,1\);
ST\(argvi\) = \.\.\.
argvi\+\+

but since sp isn't being advanced at the same time as argvi, the
EXTEND()s don't accumlate.

ILYAZ/modules/Math-Pari-2.01080900.zip

This XS code has​:

EXTEND(sp, 3); /* Got cv + 0, return 4. */
PUSHs(sv_2mortal(newSViv(SVnumtotal)));
PUSHs(sv_2mortal(newSViv(SVnum)));
PUSHs(sv_2mortal(newSViv(onStack)));
PUSHs(sv_2mortal(newSViv(offStack)));

which, despite the code comment, only extends the stack by 3, not 4.

PJB/Math-WalshTransform-1.17.tar.gz

This XS module, in product(), returns lots of values without ever
extending the stack. From a quick glance, that oversight seems to be
common to most of the functions in in the XS code.

HANK/Text-Aspell-0.09.tar.gz

list_dictionaries() pushes a bunch of items on the stack without
extending it.

So, does this fall into the category of "Recent commit to blead exposes sub-optimal code in CPAN distros"?

If so, is there some stock language we can include in rt.cpan.org tickets (or github issues) to point the distro maintainers in the right direction?

Thank you very much.
--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2017

From @iabyn

On Wed, Aug 23, 2017 at 10​:11​:37AM -0700, James E Keenan via RT wrote​:

So, does this fall into the category of "Recent commit to blead exposes
sub-optimal code in CPAN distros"?

More specifically, a new feature added to DEBUGGING builds of bleadperl
will detect many cases where code (either in in the core or in XS) pushes
items onto the perl stack without first allocating sufficient space for
them. So it's detecting code that is buggy and which could crash or be a
security risk under some circumstances

If so, is there some stock language we can include in rt.cpan.org
tickets (or github issues) to point the distro maintainers in the right
direction?

Since 5.27.2, DEBUGGING builds of perl will check whether the number of
arguments left on the stack after the execution of each op were matched by
suitable EXTEND() or XPUSHs() etc during execution of the op. If not, the
perl interpreter will now panic with a message like​:

  panic​: previous op failed to extend arg stack​:
  base=29f4958, sp=29f4990, hwm=29f4988

where the 3 hexadecimal values represent​:

  * the address of the base of the stack;
  * the current stack pointer on return from executing the op;
  * the "high water mark" - the highest slot in the stack which was
  guaranteed to have been allocated via a call to EXTEND() etc
  during execution of the op.

In the example above, one more 8-byte pointer has been pushed onto the
stack than for which space has been allocated.

For XS code, the op just executed is likely to be entersub, which will
have called an XS function and returned whatever that function pushed onto
the stack.

From the limited experience of XS modules so far known to have broken, the
main sins seem to be​:

  * simply pushing return values without extending, e.g. using several
  PUSHs() rather than several XPUSHs() or an EXTEND();

  * doing EXTEND(n) then more than n PUSHs()s;

  * doing something like​:

  while (...) {
  EXTEND(SP, 1);
  ST(i) = ...;
  i++;
  }

  which only ever extends the stack by one slot, since SP isn't
  updated between calls to EXTEND().

--
Modern art​:
  "That's easy, I could have done that!"
  "Ah, but you didn't!"

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2017

From @tonycoz

On Sun, 20 Aug 2017 22​:56​:20 -0700, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

Affected are -DDEBUGGING builds only.

bisect
------
commit 87058c3
Author​: David Mitchell <davem@​iabyn.com>
Date​: Tue Jun 13 09​:11​:13 2017 +0100

add PL_curstackinfo->si_stack_hwm

other distros also affected
---------------------------
SHLOMIF/Math-Cephes-0.5305.tar.gz

https://rt.cpan.org/Ticket/Display.html?id=122912

LETO/Math-GSL-0.39.tar.gz

https://rt.cpan.org/Ticket/Display.html?id=122911

These two use SWIG, from looking at the generated code I suspect this is a SWIG bug.

ILYAZ/modules/Math-Pari-2.01080900.zip

memUsage() pushes 4 items after doing EXTEND(SP, 3);. The code
justifies this​:

void
memUsage()
PPCODE​:
#ifdef DEBUG_PARI
  EXTEND(sp, 3); /* Got cv + 0, return 4. */
  PUSHs(sv_2mortal(newSViv(SVnumtotal)));
  PUSHs(sv_2mortal(newSViv(SVnum)));
  PUSHs(sv_2mortal(newSViv(onStack)));
  PUSHs(sv_2mortal(newSViv(offStack)));
#endif

which seems broken to me.

PJB/Math-WalshTransform-1.17.tar.gz

product() doesn't extend the stack at all.

https://rt.cpan.org/Ticket/Display.html?id=122909

HANK/Text-Aspell-0.09.tar.gz

list_dictionaries() was naughty here.

https://rt.cpan.org/Ticket/Display.html?id=122910

I detected which functions were misbehaving using my change from 131975.

Tony

@p5pRT
Copy link
Author

p5pRT commented Apr 19, 2018

From @iabyn

On Sun, Aug 27, 2017 at 10​:30​:05PM -0700, Tony Cook via RT wrote​:

On Sun, 20 Aug 2017 22​:56​:20 -0700, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

commit 87058c3
Author​: David Mitchell <davem@​iabyn.com>
Date​: Tue Jun 13 09​:11​:13 2017 +0100

add PL_curstackinfo->si_stack_hwm

Since the change in perl was simply that under DEBUGGING builds, XS modules
which incorrectly failed to extend the stack before pushing item would now
panic,

Since
1) these are all bugs in the distributions or their dependences;
2) the detection and thus panic don't happen on production builds of perl,

I propose this ticket is closed, or failing that, removed from 5.28
blockers.

--
Never work with children, animals, or actors.

@p5pRT
Copy link
Author

p5pRT commented Apr 19, 2018

From @xsawyerx

On 04/19/2018 11​:14 PM, Dave Mitchell wrote​:

On Sun, Aug 27, 2017 at 10​:30​:05PM -0700, Tony Cook via RT wrote​:

On Sun, 20 Aug 2017 22​:56​:20 -0700, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

commit 87058c3
Author​: David Mitchell <davem@​iabyn.com>
Date​: Tue Jun 13 09​:11​:13 2017 +0100

add PL_curstackinfo->si_stack_hwm
Since the change in perl was simply that under DEBUGGING builds, XS modules
which incorrectly failed to extend the stack before pushing item would now
panic,

Since
1) these are all bugs in the distributions or their dependences;
2) the detection and thus panic don't happen on production builds of perl,

I propose this ticket is closed, or failing that, removed from 5.28
blockers.

More than reasonable. I second removing this from 5.28 and closing.

@p5pRT
Copy link
Author

p5pRT commented Apr 20, 2018

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

@p5pRT p5pRT closed this as completed Apr 20, 2018
@p5pRT p5pRT added BBC Blead Breaks CPAN - changes in blead broke a cpan module(s) Severity Low labels Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s)
Projects
None yet
Development

No branches or pull requests

1 participant