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

BBC: coredump in pp_hot.c after B::UNOP_AUX::aux_list() #17301

Closed
dk opened this issue Nov 17, 2019 · 6 comments
Closed

BBC: coredump in pp_hot.c after B::UNOP_AUX::aux_list() #17301

dk opened this issue Nov 17, 2019 · 6 comments
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s)

Comments

@dk
Copy link

dk commented Nov 17, 2019

This is a bug report for perl from dmitry@karasik.eu.org,
generated with the help of perlbug 1.41 running under perl 5.30.1.


[Please describe your issue here]

Hello

I observed a coredump with the following code:

use B;
my $self;
my $sub = sub { $self->{h1}{h2}{h3}{v} == 1 };
my $cv = B::svref_2object($sub);
my $op = $cv->ROOT->first->first->next;
my @Items = $op->aux_list($cv); # <-- coredumps here

The stack is:

#0 Perl_pp_aassign () at pp_hot.c:2394
2394 if (SvTEMP(rsv) && !SvGMAGICAL(rsv) && SvREFCNT(rsv) == 1) {
(gdb) bt
#0 Perl_pp_aassign () at pp_hot.c:2394
#1 0xb73fedcd in Perl_runops_debug () at dump.c:2537
#2 0xb736a3a7 in S_run_body (oldscope=1) at perl.c:2716
#3 perl_run (my_perl=0xb82c2008) at perl.c:2639

Valgrind output:

==8845== Use of uninitialised value of size 4
==8845== at 0x23E684: Perl_pp_aassign (pp_hot.c:2394)
==8845== by 0x202DCC: Perl_runops_debug (dump.c:2537)
==8845== by 0x16E3A6: S_run_body (perl.c:2716)
==8845== by 0x16E3A6: perl_run (perl.c:2639)
==8845== by 0x133DF5: main (perlmain.c:127)

Seems to be introduced by this commit:

commit 8b0c337
Author: David Mitchell davem@iabyn.com
Date: Wed Oct 5 10:10:56 2016 +0100

I could only reproduce it on i686-linux architecture. The module
(DBIx::Perlish) that stumbles upon that problem, also has problems on
armv6l-linux and i386-freebsd, but never on amd64/x86_64
( http://matrix.cpantesters.org/?dist=DBIx-Perlish ).

Regards,
Dmitry Karasik

[Please do not change anything below this line]


Flags:
category=core
severity=medium

Site configuration information for perl 5.30.1:

Configured by dk at Sat Nov 16 22:06:12 CET 2019.

Summary of my perl5 (revision 5 version 30 subversion 1) configuration:

Platform:
osname=linux
osvers=3.2.0-4-686-pae
archname=i686-linux
uname='linux udayin 3.2.0-4-686-pae #1 smp debian 3.2.65-1 i686 gnulinux '
config_args='-de -Dprefix=/home/dk/perl5/perlbrew/perls/perl-5.30.1-debug -Dman1dir=none -Dman3dir=none -DDEBUGGING -Aeval:scriptdir=/home/dk/perl5/perlbrew/perls/perl-5.30.1-debug/bin'
hint=recommended
useposix=true
d_sigaction=define
useithreads=undef
usemultiplicity=undef
use64bitint=undef
use64bitall=undef
uselongdouble=undef
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 -D_FORTIFY_SOURCE=2'
optimize='-O2 -g'
cppflags='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
ccversion=''
gccversion='6.3.0 20170516'
gccosandvers=''
intsize=4
longsize=4
ptrsize=4
doublesize=8
byteorder=1234
doublekind=3
d_longlong=define
longlongsize=8
d_longdbl=define
longdblsize=12
longdblkind=3
ivtype='long'
ivsize=4
nvtype='double'
nvsize=8
Off_t='off_t'
lseeksize=8
alignbytes=4
prototype=define
Linker and Libraries:
ld='cc'
ldflags =' -fstack-protector-strong -L/usr/local/lib'
libpth=/usr/local/lib /usr/lib/gcc/i686-linux-gnu/6/include-fixed /usr/include/i386-linux-gnu /usr/lib /lib/i386-linux-gnu /lib/../lib /usr/lib/i386-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'

Locally applied patches:
Devel::PatchPerl 1.66


@inc for perl 5.30.1:
/home/dk/perl5/perlbrew/perls/perl-5.30.1-debug/lib/site_perl/5.30.1/i686-linux
/home/dk/perl5/perlbrew/perls/perl-5.30.1-debug/lib/site_perl/5.30.1
/home/dk/perl5/perlbrew/perls/perl-5.30.1-debug/lib/5.30.1/i686-linux
/home/dk/perl5/perlbrew/perls/perl-5.30.1-debug/lib/5.30.1


Environment for perl 5.30.1:
HOME=/home/dk
LANG=en_US.UTF-8
LANGUAGE=en_US:en
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/home/dk/perl5/perlbrew/bin:/home/dk/perl5/perlbrew/perls/perl-5.30.1-debug/bin:/usr/local/bin:/usr/bin:/bin:/usr/games:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/usr/X11R6/bin:/usr/local/site/bin:/usr/local/site/sbin:/home/dk/bin
PERLBREW_HOME=/home/dk/.perlbrew
PERLBREW_MANPATH=/home/dk/perl5/perlbrew/perls/perl-5.30.1-debug/man
PERLBREW_PATH=/home/dk/perl5/perlbrew/bin:/home/dk/perl5/perlbrew/perls/perl-5.30.1-debug/bin
PERLBREW_PERL=perl-5.30.1-debug
PERLBREW_ROOT=/home/dk/perl5/perlbrew
PERLBREW_SHELLRC_VERSION=0.87
PERLBREW_VERSION=0.87
PERL_BADLANG (unset)
SHELL=/usr/bin/zsh

@dk
Copy link
Author

dk commented Nov 17, 2019

cc @iabyn

@jkeenan
Copy link
Contributor

jkeenan commented Nov 17, 2019

@bingos I believe you have an i386-FreeBSD CPANtesters rig (per http://www.cpantesters.org/cpan/report/acb434aa-083a-11ea-9066-e374b0ba08e8). Could you test out the code provided in the original post in this thread?

Thank you very much.
Jim Keenan

@dk
Copy link
Author

dk commented Nov 17, 2019

Some more digging show that the commit in pp_hot.c is behaving okay, circling through 7 SVs where only 6 has been pushed to stack but polished with XSRETURN(7). That seem to be happening because ext/B/B.xs:1289

            UV len = items[-1].uv;

gets len=7 on my i686-linux but len=6 on x86_64.

It seems that the OP_MULTIDEREF is created badly on this arch. This [-1] addressing though looks strange, at least. I couldn't understand how this works at all, and neither there is anything related in newUNOP_AUX

@dk
Copy link
Author

dk commented Nov 17, 2019

Here's a patch that covers for this particular issue, however I think while the coredump itself is fixed, the real issue is being masked. If there is a similar code elsewhere, it might explode too. It would be great if David Mitchell could take second look at it.

patch.txt

@jkeenan
Copy link
Contributor

jkeenan commented Dec 8, 2019

Here's a patch that covers for this particular issue, however I think while the coredump itself is fixed, the real issue is being masked. If there is a similar code elsewhere, it might explode too. It would be great if David Mitchell could take second look at it.

patch.txt

@iabyn can you take a look at this issue?

Thank you very much.
Jim Keenan

@xsawyerx xsawyerx added the BBC Blead Breaks CPAN - changes in blead broke a cpan module(s) label Dec 28, 2019
iabyn added a commit that referenced this issue Jan 2, 2020
GH #17301

The aux array in an OP_MULTIDEREF op consists of an action word
containing as many actions as will fit shifted together, followed by
words containing the arguments for those actions. Then another action
word, and so on. The code in S_maybe_multideref() which creates those
ops was reserving a new slot in the aux array for a new action word when
the old one became full. If it then turned out that no  more actions
were needed, this extra slot was harmlessly filled with a zero.

However it turns out that the B::UNOP_AUX::aux_list() introspection
method would, under those circumstances, claim to have returned one
more SV on the stack than it actually had, leading to SEGVs etc.

I could have fixed aux_list() directly to cope with an extra null word,
but instead I did the more general fix of ensuring that
S_maybe_multideref() never adds an extra null word in the first place.

The test added to ext/B/t/b.t fails before this commit; the new test
in lib/B/Deparse.t doesn't, but was added for completeness.
@iabyn
Copy link
Contributor

iabyn commented Jan 2, 2020 via email

@iabyn iabyn closed this as completed Jan 2, 2020
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

4 participants