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

bug in Math::BigInt with undef #13283

Closed
p5pRT opened this issue Sep 17, 2013 · 6 comments
Closed

bug in Math::BigInt with undef #13283

p5pRT opened this issue Sep 17, 2013 · 6 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 17, 2013

Migrated from rt.perl.org#119863 (status was 'rejected')

Searchable as RT119863$

@p5pRT
Copy link
Author

p5pRT commented Sep 17, 2013

From hb-perlbug@bsws.de

Created by hb-perlbug@bsws.de

This is a bug report for perl from hb-perlbug@​bsws.de,
generated with the help of perlbug 1.39 running under perl 5.16.3.

-----------------------------------------------------------------
Math​::BigInt got broken when one component is undef. Unfortunately
the I can only reproduce teh bug in a big custome webapp, but it
comes down to BigInt.pm's objectify() being called with an undef
param, which it won't objectify, due to the check on line 2673ff

  # Don't do anything with undefs.

  unless (defined($a[$i])) {
  next;
  }

callers, i. e. badd(), don't deal with any component being undef.

sub badd
  {
  # add second arg (BINT or string) to first (BINT) (modifies first)
  # return result as BINT

  # set up parameters
  my ($self,$x,$y,@​r) = (ref($_[0]),@​_);
  # objectify is costly, so avoid it
  if ((!ref($_[0])) || (ref($_[0]) ne ref($_[1])))
  {
  ($self,$x,$y,@​r) = objectify(2,@​_);
  }

  return $x if $x->modify('badd');
  return $upgrade->badd($upgrade->new($x),$upgrade->new($y),@​r) if defined $upgrade &&
  ((!$x->isa($self)) || (!$y->isa($self)));

in my case $y ends up being undef, objectify doesn't do anything to it,
and the $y->isa call chokes.
[error] Can't call method "isa" on an undefined value at /usr/libdata/perl5/Math/BigInt.pm line 1131.\n

crude (and almost certainly not correct) fix​:

Inline Patch
--- BigInt.pm.orig	Tue Sep 17 10:37:09 2013
+++ BigInt.pm	Tue Sep 17 10:32:12 2013
@@ -2670,11 +2670,12 @@ sub objectify {
             next;
         }
 
-        # Don't do anything with undefs.
-
-        unless (defined($a[$i])) {
-            next;
-        }
+#        # Don't do anything with undefs.
+#
+#        unless (defined($a[$i])) {
+#            next;
+#        }
+        $a[$i] //= 0;
 
         # Perl scalars are fed to the appropriate constructor.
 
Perl Info

Flags:
    category=library
    severity=high
    module=Math::BigInt

Site configuration information for perl 5.16.3:

Configured by root at Thu Jan  1  0:00:00 UTC 1970.

Summary of my perl5 (revision 5 version 16 subversion 3) configuration:
   
  Platform:
    osname=openbsd, osvers=5.4, archname=amd64-openbsd
    uname='openbsd'
    config_args='-dsE -Dopenbsd_distribution=defined -Dccflags=-DNO_LOCALE_NUMERIC -DNO_LOCALE_COLLATE -Dmksymlinks'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-DNO_LOCALE_NUMERIC -DNO_LOCALE_COLLATE -fno-strict-aliasing -fno-delete-null-pointer-checks -pipe -fstack-protector -I/usr/local/include',
    optimize='-O2',
    cppflags='-DNO_LOCALE_NUMERIC -DNO_LOCALE_COLLATE -fno-strict-aliasing -fno-delete-null-pointer-checks -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.2.1 20070719 ', gccosandvers='openbsd5.4'
    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 ='-Wl,-E  -fstack-protector'
    libpth=/usr/lib
    libs=-lm -lutil -lc
    perllibs=-lm -lutil -lc
    libc=/usr/lib/libc.a, so=so, useshrplib=true, libperl=libperl.so.14.0
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-R/usr/libdata/perl5/amd64-openbsd/5.16.3/CORE'
    cccdlflags='-DPIC -fPIC ', lddlflags='-shared -fPIC  -fstack-protector'

Locally applied patches:
    


@INC for perl 5.16.3:
    /usr/local/libdata/perl5/site_perl/amd64-openbsd
    /usr/libdata/perl5/site_perl/amd64-openbsd
    /usr/local/libdata/perl5/site_perl
    /usr/libdata/perl5/site_perl
    /usr/libdata/perl5/amd64-openbsd/5.16.3
    /usr/local/libdata/perl5/amd64-openbsd/5.16.3
    /usr/libdata/perl5
    /usr/local/libdata/perl5
    .


Environment for perl 5.16.3:
    HOME=/home/brahe
    LANG (unset)
    LANGUAGE (unset)
    LC_CTYPE=en_US.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/brahe/bin:/sbin:/usr/sbin:/bin:/usr/bin:/sbin:/usr/sbin:/usr/X11R6/bin:/usr/local/bin:/usr/local/sbin:/usr/local/sbin
    PERL_BADLANG (unset)
    SHELL=/bin/ksh

@p5pRT
Copy link
Author

p5pRT commented Sep 17, 2013

From @cpansprout

On Tue Sep 17 07​:23​:11 2013, hb-perlbug@​bsws.de wrote​:

Math​::BigInt got broken when one component is undef. Unfortunately
the I can only reproduce teh bug in a big custome webapp, but it
comes down to BigInt.pm's objectify() being called with an undef
param, which it won't objectify, due to the check on line 2673ff

    \# Don't do anything with undefs\.

    unless \(defined\($a\[$i\]\)\) \{
        next;
    \}

That appears to be by design. I suspect your code is buggy in giving it
undef to begin with.

Could you try to reduce your big webapp to something smaller that can
reproduce the bug?

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 17, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Sep 19, 2013

From hb@bsws.de

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2013-09-17 17​:38]​:

On Tue Sep 17 07​:23​:11 2013, hb-perlbug@​bsws.de wrote​:

Math​::BigInt got broken when one component is undef. Unfortunately
the I can only reproduce teh bug in a big custome webapp, but it
comes down to BigInt.pm's objectify() being called with an undef
param, which it won't objectify, due to the check on line 2673ff

    \# Don't do anything with undefs\.

    unless \(defined\($a\[$i\]\)\) \{
        next;
    \}

That appears to be by design. I suspect your code is buggy in giving it
undef to begin with.

no, it is not, your statement last not least contradicts perlop​:

  "undef" is always treated as numeric, and in particular is
  changed to 0 before incrementing (so that a post-increment of
  an undef value will return 0 rather than "undef").

and this has worked for ages, up until the upgrade to OpenBSD 5.4 and
thus perl 5.16.3.

the bug was introduced in
http​://perl5.git.perl.org/perl.git/commitdiff/66a0495875e8130c45cac4fabd5f8d05f2;3Cf4c372?hp=7833bfdd94cb7b5afbbc1b18e75e664482f529d5

the trigger is sth like

my %foo;
$foo{nonexistantmember} += $somenumber;

that code is under "use warnings;" and doesn't trigger any, fwiw.

Could you try to reduce your big webapp to something smaller that can
reproduce the bug?

I tried, of course, but haven't been able to, i must miss some bit
that gets to that codepath. it should be obvious enough tho.

--
Henning Brauer, hb@​bsws.de, henning@​openbsd.org
BS Web Services GmbH, AG Hamburg HRB 128289, http​://bsws.de
Full-Service ISP - Secure Hosting, Mail and DNS Services
Dedicated Servers, Rootservers, Application Hosting

@p5pRT
Copy link
Author

p5pRT commented Dec 15, 2017

From zefram@fysh.org

Math-BigInt is maintained primarily as a CPAN distribution, so this
is the wrong place to report bugs in it. If the problem described
still occurs, you should report it to <bug-Math-BigInt@​rt.cpan.org>.
This ticket in the perl5 queue should be closed.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Dec 15, 2017

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

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