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: Blead Breaks BSON #22868

Open
cjg-cguevara opened this issue Dec 19, 2024 · 5 comments
Open

BBC: Blead Breaks BSON #22868

cjg-cguevara opened this issue Dec 19, 2024 · 5 comments
Assignees
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s) hasPatch

Comments

@cjg-cguevara
Copy link

This is a bug report for perl from "Carlos Guevara" carlos@carlosguevara.com,
generated with the help of perlbug 1.43 running under perl 5.41.7.


BBC: Blead Breaks BSON

Please see http://fast-matrix.cpantesters.org/?dist=BSON


Flags

  • category=core
  • severity=low

Perl configuration

Site configuration information for perl 5.41.7:

Configured by cpan at Thu Dec 19 00:23:29 EST 2024.

Summary of my perl5 (revision 5 version 41 subversion 7) configuration:
  Commit id: 7496ff12fc1a5176641e424139fc7ad8e8ca08e5
  Platform:
    osname=linux
    osvers=5.14.0-503.16.1.el9_5.x86_64
    archname=x86_64-linux
    uname='linux cjg-rhel9 5.14.0-503.16.1.el9_5.x86_64 #1 smp preempt_dynamic thu nov 21 07:26:21 est 2024 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Dprefix=/home/cpan/bin/perl -Dscriptdir=/home/cpan/bin/perl/bin -Dusedevel -Duse64bitall'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  Compiler:
    cc='cc'
    ccflags ='-fwrapv -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'
    cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='11.5.0 20240719 (Red Hat 11.5.0-2)'
    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='cc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib /usr/lib64 /usr/local/lib64
    libs=-lpthread -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc
    libc=/lib/../lib64/libc.so.6
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.34'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'


---
@INC for perl 5.41.7:
    /home/cpan/bin/perl/lib/site_perl/5.41.7/x86_64-linux
    /home/cpan/bin/perl/lib/site_perl/5.41.7
    /home/cpan/bin/perl/lib/5.41.7/x86_64-linux
    /home/cpan/bin/perl/lib/5.41.7

---
Environment for perl 5.41.7:
    HOME=/home/cpan
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LC_ALL=C
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/cpan/bin/perl/bin:/home/cpan/bin:/usr/share/Modules/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin
    PERL_BADLANG (unset)
    SHELL=/bin/bash
@jkeenan jkeenan added the BBC Blead Breaks CPAN - changes in blead broke a cpan module(s) label Dec 19, 2024
@jkeenan
Copy link
Contributor

jkeenan commented Dec 19, 2024

Bisecting with the following invocation:

perl Porting/bisect.pl \
--start=6f836886eca09c5df3a9f985231636e29b8f10dc \
--end=7496ff12fc1a5176641e424139fc7ad8e8ca08e5 \
--module=BSON

... pointed to this as the breaking point:

fbebf96ce083dc15b63c04d5695997d6e2657b03 is the first bad commit
commit fbebf96ce083dc15b63c04d5695997d6e2657b03
Author: Richard Leach <richardleach@users.noreply.github.com>
Date:   Wed Oct 23 23:32:04 2024 +0000
Commit:     Richard Leach <rich+perl@hyphen-dash-hyphen.info>
CommitDate: Mon Dec 16 21:33:09 2024 +0000

    Perl_sv_setsv_flags: handle mixed IV and NV fast case
    
    When the fast code at the start of Perl_sv_setsv_flags was modified to
    also support bodyless NVs, the simplest possible change was made.
    However, this meant that there was no fast handling when one SV was an
    IV and the other a NV. Actually having this seems desirable since it
    avoids the need to allocate (and later release) an XPVIV or XPVNV body.

 sv.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
bisect found first bad commit
That took 1579 seconds.

@richardleach, can you take a look? Thanks.

@mauke
Copy link
Contributor

mauke commented Dec 19, 2024

For reference, this is the error/failing test:

t/mapping/double.t ............... Possible precedence problem between ! and string eq at /home/mauke/.cpan/build/BSON-v1.12.2-1/blib/lib/BSON.pm line 99.
t/mapping/double.t ............... 1/?                                                                                                                                                                                                       
#   Failed test 'Inf as double->double' 
#   at t/mapping/double.t line 55.                                                                                                                                                                                                           
#          got: 'NV'                     
#     expected: 'PVNV'
                                                           
#   Failed test '-Inf as double->double'
#   at t/mapping/double.t line 55.                                                                                    
#          got: 'NV'                   
#     expected: 'PVNV'   
                                                                                                                      
#   Failed test 'NaN as double->double'
#   at t/mapping/double.t line 55.                
#          got: 'NV'                             
#     expected: 'PVNV'       
                                                           
#   Failed test 'Inf as BSON::Double->BSON::Double'                                                                   
#   at t/mapping/double.t line 69.   
#          got: 'NV'       
#     expected: 'PVNV'                                                                                                

#   Failed test '-Inf as BSON::Double->BSON::Double'
#   at t/mapping/double.t line 69.
#          got: 'NV'
#     expected: 'PVNV'

#   Failed test 'NaN as BSON::Double->BSON::Double'
#   at t/mapping/double.t line 69.
#          got: 'NV'
#     expected: 'PVNV'
# Looks like you failed 6 tests of 39.
t/mapping/double.t ............... Dubious, test returned 6 (wstat 1536, 0x600)
Failed 6/39 subtests 

... which I'm pretty sure is just the module relying on implementation details that it shouldn't rely on. In this case, it tests that bson_decode(bson_encode($x)) (for $x = Inf, -Inf, NaN) returns floating-point values represented as PVNV instead of the (slimmer) NV.

(To be honest, I can't figure out why Perl would use PVNV here in the first place, and running the test in the perl debugger makes it use PVMG instead.)

@Leont
Copy link
Contributor

Leont commented Dec 19, 2024

which I'm pretty sure is just the module relying on implementation details that it shouldn't rely on

Agreed.

@richardleach richardleach self-assigned this Dec 19, 2024
@richardleach
Copy link
Contributor

In this case, it tests that bson_decode(bson_encode($x)) (for $x = Inf, -Inf, NaN) returns floating-point values represented as PVNV instead of the (slimmer) NV.

Yes, and a portable fix seems straightforward:

diff --git a/BSON-v1.12.2/t/mapping/double.t b/BSON-v1.12.2_patched/t/mapping/double.t
index fb0d51b..fdb0c86 100644
--- a/BSON-v1.12.2/t/mapping/double.t
+++ b/BSON-v1.12.2_patched/t/mapping/double.t
@@ -50,9 +50,12 @@ my %special = (
     "NaN"  => BSON::Double::NaN(),
 );
 
+my $iv2nv_type = 0; $iv2nv_type = 1.1; $iv2nv_type = sv_type($iv2nv_type);
+
+
 for my $s ( qw/Inf -Inf NaN/ ) {
     $hash = decode( encode( { A => $special{$s} } ) );
-    is( sv_type( $hash->{A} ), 'PVNV', "$s as double->double" );
+    is( sv_type( $hash->{A} ), $iv2nv_type, "$s as double->double" );
     packed_is( FLOAT, $hash->{A}, $special{$s}, "value correct" );
 }
 
@@ -66,7 +69,7 @@ for my $s ( qw/Inf -Inf NaN/ ) {
 # test special BSON::Double
 for my $s ( qw/Inf -Inf NaN/ ) {
     $hash = decode( encode( { A => bson_double($special{$s}) } ) );
-    is( sv_type( $hash->{A} ), 'PVNV', "$s as BSON::Double->BSON::Double" );
+    is( sv_type( $hash->{A} ), $iv2nv_type, "$s as BSON::Double->BSON::Double" );
     packed_is( FLOAT, $hash->{A}, $special{$s}, "value correct" );
 }

The problem for this module is that it's been abandoned. https://metacpan.org/pod/BSON says:

"Version v1.12.0 was the final feature release of the MongoDB BSON library and version v1.12.2 is the final patch release.

As of August 13, 2020, the MongoDB Perl driver and related libraries have reached end of life and are no longer supported by MongoDB. See the August 2019 deprecation notice for rationale." The repository has been made read-only: https://github.com/mongodb-labs/mongo-perl-bson

I don't know where to send the patch, or if a supported fork of this module exists.

@jkeenan
Copy link
Contributor

jkeenan commented Dec 19, 2024

In this case, it tests that bson_decode(bson_encode($x)) (for $x = Inf, -Inf, NaN) returns floating-point values represented as PVNV instead of the (slimmer) NV.

Yes, and a portable fix seems straightforward:

diff --git a/BSON-v1.12.2/t/mapping/double.t b/BSON-v1.12.2_patched/t/mapping/double.t
index fb0d51b..fdb0c86 100644
--- a/BSON-v1.12.2/t/mapping/double.t
+++ b/BSON-v1.12.2_patched/t/mapping/double.t
@@ -50,9 +50,12 @@ my %special = (
     "NaN"  => BSON::Double::NaN(),
 );
 
+my $iv2nv_type = 0; $iv2nv_type = 1.1; $iv2nv_type = sv_type($iv2nv_type);
+
+
 for my $s ( qw/Inf -Inf NaN/ ) {
     $hash = decode( encode( { A => $special{$s} } ) );
-    is( sv_type( $hash->{A} ), 'PVNV', "$s as double->double" );
+    is( sv_type( $hash->{A} ), $iv2nv_type, "$s as double->double" );
     packed_is( FLOAT, $hash->{A}, $special{$s}, "value correct" );
 }
 
@@ -66,7 +69,7 @@ for my $s ( qw/Inf -Inf NaN/ ) {
 # test special BSON::Double
 for my $s ( qw/Inf -Inf NaN/ ) {
     $hash = decode( encode( { A => bson_double($special{$s}) } ) );
-    is( sv_type( $hash->{A} ), 'PVNV', "$s as BSON::Double->BSON::Double" );
+    is( sv_type( $hash->{A} ), $iv2nv_type, "$s as BSON::Double->BSON::Double" );
     packed_is( FLOAT, $hash->{A}, $special{$s}, "value correct" );
 }

The problem for this module is that it's been abandoned. https://metacpan.org/pod/BSON says:

"Version v1.12.0 was the final feature release of the MongoDB BSON library and version v1.12.2 is the final patch release.

As of August 13, 2020, the MongoDB Perl driver and related libraries have reached end of life and are no longer supported by MongoDB. See the August 2019 deprecation notice for rationale." The repository has been made read-only: https://github.com/mongodb-labs/mongo-perl-bson

I don't know where to send the patch, or if a supported fork of this module exists.

Okay, provided we understand how the breaking commit actually broke the module's tests, we don't have to worry too much about this. We should probably focus on #22866 (B-Utils).

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) hasPatch
Projects
None yet
Development

No branches or pull requests

5 participants