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

perl5db.pl DB::sub does not properly handle overloaded blessed anonymous subroutines #13503

Open
p5pRT opened this issue Jan 1, 2014 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 1, 2014

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

Searchable as RT120911$

@p5pRT
Copy link
Author

p5pRT commented Jan 1, 2014

From @mjdominus

Created by @mjdominus

Consider the following program​:

  package Foo;
  use overload '""' => sub { "overloaded string" };

  package main;
  my $s = bless(sub { "I like pie!" } => "Foo");

  $s->();

  print "Done\n";

Under Perl 5.18.1 this runs without fault, prints "Done", and exits
successfully.

However, under 'perl -d' it causes a fatal error in perl5db.pl's "DB​::sub" method. To see this, run

  perl -d perlbugbug

and enter the "c" command twice. Instead of printing "Done" the
program faults in the debugger, emitting the message​:

  Operation "eq"​: no method found,
  left argument in overloaded package Foo,
  right argument has no overloaded magic at /usr/local/lib/perl5/5.18.1/perl5db.pl line 4106.
  at /usr/local/lib/perl5/5.18.1/perl5db.pl line 4106.

Sure enough, line 4106 is performing an "eq" comparison of the sub
with a plain string, assuming that this will work. The faulty line reads​:

  if ($sub eq 'threads​::new' && $ENV{PERL5DB_THREADED}) {

It seems that the code here assumes that the sub will be a string, and
that assumption is incorrect. I'm not sure whether the assumption is
unwarranted; it may be that $DB​::sub is guaranteed to be a string but
there is a bug in the internals responsible for populating that
variable. Or the bug may be in the assumption itself.

I have a auperstition that whatever I am doing at exactly midnight
will be what I do most for the rest of the year. I tried to spend
midnight working on my favorite unfinished program. Instead, I spent
midnight tracking down this freakish bug in the Perl debuggger.

2014 is only a few hours old. Nevertheless, I nominate this bug for
the most obscure bug reported in 2014.

Perl Info

Flags:
    category=library
    severity=low
    module=DB

Site configuration information for perl 5.14.2:

Configured by Debian Project at Mon Mar 18 19:16:26 UTC 2013.

Summary of my perl5 (revision 5 version 14 subversion 2) configuration:
   
  Platform:
    osname=linux, osvers=2.6.42-37-generic, archname=x86_64-linux-gnu-thread-multi
    uname='linux batsu 2.6.42-37-generic #58-ubuntu smp thu jan 24 15:28:10 utc 2013 x86_64 x86_64 x86_64 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.14 -Darchlib=/usr/lib/perl/5.14 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.14.2 -Dsitearch=/usr/local/lib/perl/5.14.2 -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 -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.14.2 -des'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2 -g',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.6.3', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=, so=so, useshrplib=true, libperl=libperl.so.5.14.2
    gnulibc_version='2.15'
  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'

Locally applied patches:
    


@INC for perl 5.14.2:
    /etc/perl
    /usr/local/lib/perl/5.14.2
    /usr/local/share/perl/5.14.2
    /usr/lib/perl5
    /usr/share/perl5
    /usr/lib/perl/5.14
    /usr/share/perl/5.14
    /usr/local/lib/site_perl
    .


Environment for perl 5.14.2:
    HOME=/home/mjd
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/mjd/bin:/usr/local/nmh/bin:/home/mjd/misc/blog/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/var/qmail/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Jan 1, 2014

From mjd@cpan.org

On Tue Dec 31 21​:15​:26 2013, mjd@​plover.com wrote​:

and enter the "c" command twice.

This was an editing error. You only need to enter "c" once. If you use "n" instead of "c", you will need to enter it twice.

@p5pRT
Copy link
Author

p5pRT commented Jan 1, 2014

From [Unknown Contact. See original ticket]

On Tue Dec 31 21​:15​:26 2013, mjd@​plover.com wrote​:

and enter the "c" command twice.

This was an editing error. You only need to enter "c" once. If you use "n" instead of "c", you will need to enter it twice.

@p5pRT
Copy link
Author

p5pRT commented Jan 2, 2014

From @cpansprout

Mark-Jason Dominus wrote​:

I'm not sure whether the assumption is
unwarranted; it may be that $DB​::sub is guaranteed to be a string but
there is a bug in the internals responsible for populating that
variable.

$DB​::sub will be set to a string if the sub name is determinable
before DB​::sub is called and the sub is reachable under that name.
Otherwise it is set to a reference.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jan 2, 2014

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

@p5pRT
Copy link
Author

p5pRT commented Jan 2, 2014

From @mjdominus

The faulty line reads​:

if \($sub eq 'threads​::new' && $ENV\{PERL5DB\_THREADED\}\) \{

Having thought about this more, I no longer think this is a bug in
perl5db.pl. It is unreasonable to require every piece of code that
uses the "eq" operator to defend against the possibility of being
given an object with overloading that fails to define the 'eq'
overloading. More reasonable is to require that the programmer
writing the overloading should define "eq" or be prepared for weird
failures.

The 'overload' man page should probably have a warning about this, but
honestly, if it did I probably would not have seen it. (Or maybe it
does, and I didn't see it.)

The overload module itself could emit a warning if you try to define
an overloading that omits certain methods​: Hey, you didn't define
'eq', and if some other code tries to use 'eq' on your object, it will
die. This might be worth doing.

@p5pRT
Copy link
Author

p5pRT commented Jan 2, 2014

From @csjewell

On Thu, Jan 2, 2014, at 8​:19, Mark Dominus wrote​:

The faulty line reads​:

if \($sub eq 'threads​::new' && $ENV\{PERL5DB\_THREADED\}\) \{

Having thought about this more, I no longer think this is a bug in
perl5db.pl. It is unreasonable to require every piece of code that
uses the "eq" operator to defend against the possibility of being
given an object with overloading that fails to define the 'eq'
overloading. More reasonable is to require that the programmer
writing the overloading should define "eq" or be prepared for weird
failures.

The 'overload' man page should probably have a warning about this, but
honestly, if it did I probably would not have seen it. (Or maybe it
does, and I didn't see it.)

The overload module itself could emit a warning if you try to define
an overloading that omits certain methods​: Hey, you didn't define
'eq', and if some other code tries to use 'eq' on your object, it will
die. This might be worth doing.

Except for one thing​: Defining 'eq' may not necessarily be needed in
order to act sanely when 'eq' is used. Defining an overload for "" can
be enough to return something that compares sanely via the normal
function of 'eq'. Or defining an overload for cmp can define the general
'how do I compare this as a string' case, without having to specifically
overload 'eq'.

The same things can be said for overloading 0+ or the spaceship operator
(<=>) in the case of ==.

So your proposed warning would have to be smart enough to know about
those cases.

--Curtis Jewell
--
Curtis Jewell
csjewell@​cpan.org http​://csjewell.dreamwidth.org/
perl@​csjewell.fastmail.us http​://csjewell.comyr.org/perl/

"Your random numbers are not that random" -- perl-5.10.1.tar.gz/util.c

Strawberry Perl for Windows betas​: http​://strawberryperl.com/beta/

@p5pRT
Copy link
Author

p5pRT commented Jan 2, 2014

From perl5-porters@perl.org

Mark Dominus wrote​:

The 'overload' man page should probably have a warning about this, but
honestly, if it did I probably would not have seen it. (Or maybe it
does, and I didn't see it.)

At the risk of derailing this thread, I wish the fallback default
were 1 for overloaded objects, just as it is effectively for non-over-
loaded objects.

@p5pRT
Copy link
Author

p5pRT commented Jan 3, 2014

From @ap

* Mark Jason Dominus <mjd@​plover.com> [2014-01-02 16​:25]​:

Having thought about this more, I no longer think this is a bug in
perl5db.pl.

I continue to think so.

It is unreasonable to require every piece of code that uses the "eq"
operator to defend against the possibility of being given an object
with overloading that fails to define the 'eq' overloading.

I concur.

But I do not believe that the debugger ought to be regarded as a piece
of code like any other. Subjectively it feels to me like part of perl
(so what if this part happens to be implemented in Perl – implementation
detail, no?), which ought to be capable of handling any well-defined
value (in terms of interpreter state), regardless how unusual.

* Father Chrysostomos <sprout@​cpan.org> [2014-01-02 17​:25]​:

At the risk of derailing this thread, I wish the fallback default were
1 for overloaded objects, just as it is effectively for non-overloaded
objects.

That makes sense, generally speaking; in terms of the bug, it wouldn’t
address it at all.

Regards,
--
Aristotle Pagaltzis // <http​://plasmasturm.org/>

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

2 participants