BBC: Blead Breaks Inline::CPP

[cjg-cguevara]

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.39.4.

---

BBC: Blead Breaks Inline::CPP

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

---

Flags

• category=core
• severity=low

---

Perl configuration

Site configuration information for perl 5.39.4:

Configured by cpan at Tue Sep 26 14:11:09 EDT 2023.

Summary of my perl5 (revision 5 version 39 subversion 4) configuration:
  Commit id: b5f24b1f49b3a977a4b039f4bf3a593941351661
  Platform:
    osname=linux
    osvers=6.1.55-0-lts
    archname=x86_64-linux
    uname='linux cjg-alpine3 6.1.55-0-lts #1-alpine smp preempt_dynamic sun, 24 sep 2023 23:14:02 +0000 x86_64 linux '
    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 ='-D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    optimize='-O2'
    cppflags='-D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong'
    ccversion=''
    gccversion='12.2.1 20220924'
    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/lib /usr/local/lib /lib
    libs=-lpthread -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc
    libc=/usr/lib/libc.a
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version=''
  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.39.4:
    /home/cpan/bin/perl/lib/site_perl/5.39.4/x86_64-linux
    /home/cpan/bin/perl/lib/site_perl/5.39.4
    /home/cpan/bin/perl/lib/5.39.4/x86_64-linux
    /home/cpan/bin/perl/lib/5.39.4

---
Environment for perl 5.39.4:
    HOME=/home/cpan
    LANG=C.UTF-8
    LANGUAGE (unset)
    LC_COLLATE=C
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/cpan/bin/perl/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

[mauke]

make[1]: Entering directory '/home/cpan/.cpan/build/Inline-CPP-0.80-0/_Inline/build/_01nherit_t_388c'
Running Mkbootstrap for _01nherit_t_388c ()
chmod 644 "_01nherit_t_388c.bs"
"/home/cpan/bin/perl/bin/perl5.39.4" -MExtUtils::Command::MM -e 'cp_nonempty' -- _01nherit_t_388c.bs blib/arch/auto/_01nherit_t_388c/_01nherit_t_388c.bs 644
"/home/cpan/bin/perl/bin/perl5.39.4" "/home/cpan/bin/perl/lib/5.39.4/ExtUtils/xsubpp"  -typemap "/home/cpan/bin/perl/lib/5.39.4/ExtUtils/typemap" -typemap "/home/cpan/.cpan/build/Inline-CPP-0.80-0/_Inline/build/_01nherit_t_388c/CPP.map"   _01nherit_t_388c.xs > _01nherit_t_388c.xsc
mv _01nherit_t_388c.xsc _01nherit_t_388c.c
g++ -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -xc++ -c  -iquote"/home/cpan/.cpan/build/Inline-CPP-0.80-0/t/grammar" -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -O2   -DVERSION=\"0.00\" -DXS_VERSION=\"0.00\" -fPIC "-I/home/cpan/bin/perl/lib/5.39.4/x86_64-linux/CORE"   _01nherit_t_388c.c
_01nherit_t_388c.xs: In function 'void XS_Foo_set_secret(CV*)':
_01nherit_t_388c.xs:70:48: error: cannot convert 'ssize_t*' {aka 'long int*'} to 'I32*' {aka 'int*'} in assignment
   70 |         __temp_markstack_ptr = PL_markstack_ptr++;
      |                                ~~~~~~~~~~~~~~~~^~
      |                                                |
      |                                                ssize_t* {aka long int*}
_01nherit_t_388c.xs:72:30: error: comparison between distinct pointer types 'ssize_t*' {aka 'long int*'} and 'I32*' {aka 'int*'} lacks a cast [-fpermissive]
   72 |         if (PL_markstack_ptr != __temp_markstack_ptr) {
      |             ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
_01nherit_t_388c.xs:74:30: error: cannot convert 'I32*' {aka 'int*'} to 'ssize_t*' {aka 'long int*'} in assignment
   74 |           PL_markstack_ptr = __temp_markstack_ptr;
      |                              ^~~~~~~~~~~~~~~~~~~~
      |                              |
      |                              I32* {aka int*}
_01nherit_t_388c.xs: In function 'void XS_Bar_set_secret(CV*)':
_01nherit_t_388c.xs:98:48: error: cannot convert 'ssize_t*' {aka 'long int*'} to 'I32*' {aka 'int*'} in assignment
   98 |         __temp_markstack_ptr = PL_markstack_ptr++;
      |                                ~~~~~~~~~~~~~~~~^~
      |                                                |
      |                                                ssize_t* {aka long int*}
_01nherit_t_388c.xs:100:30: error: comparison between distinct pointer types 'ssize_t*' {aka 'long int*'} and 'I32*' {aka 'int*'} lacks a cast [-fpermissive]
  100 |         if (PL_markstack_ptr != __temp_markstack_ptr) {
      |             ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
_01nherit_t_388c.xs:102:30: error: cannot convert 'I32*' {aka 'int*'} to 'ssize_t*' {aka 'long int*'} in assignment
  102 |           PL_markstack_ptr = __temp_markstack_ptr;
      |                              ^~~~~~~~~~~~~~~~~~~~
      |                              |
      |                              I32* {aka int*}
make[1]: *** [Makefile:339: _01nherit_t_388c.o] Error 1
make[1]: Leaving directory '/home/cpan/.cpan/build/Inline-CPP-0.80-0/_Inline/build/_01nherit_t_388c'

Sounds like commit fdf5d79, "Several updates to support an argument stack over 2**31 entries on 64-bit systems".
@tonycoz ?

[jkeenan]

Bisecting with this invocation:

perl Porting/bisect.pl --module=Inline::CPP \
--start=41d0ddc96a06ae00a2a5d821dd25fba6f7ee0a41 \
--end=b5f24b1f49b3a977a4b039f4bf3a593941351661

... points to c058892 as the breaking commit:

commit c0588928a3ddf0a25cdd6b8ba2679ad29a20fcd9
Author:     Tony Cook <tony@develop-help.com>
AuthorDate: Tue Mar 14 15:45:29 2023 +1100
Commit:     Tony Cook <tony@develop-help.com>
CommitDate: Mon Sep 25 14:35:17 2023 +1000

    64-bit stack: first pass, marks are now 64-bits on 64-bit platforms
    
    The basic test passes
    
    Works towards #20917

[sisyphus]

I see the same errors building Inline-CPP on 64-bit Windows (gcc-13.2.0).
And a similar thing happens with Inline::C except that there are only compilation warnings, but no compilation errors.

UPDATE: The following patches don't port to 32-bit builds of perl. Back to the drawing board.

Here is the (portable ?) patch for I-CPP's CPP.pm:

--- lib/Inline/CPP.pm_orig	2019-04-20 00:50:28.000000000 +1000
+++ lib/Inline/CPP.pm	2023-09-28 22:15:21.871118100 +1000
@@ -548,7 +548,7 @@
 
   # Wrap "complicated" subs in stack-checking code
   if ($void or $ellipsis) {
-    push @PREINIT, "\tI32 *\t__temp_markstack_ptr;\n";
+    push @PREINIT, "\tIV *\t__temp_markstack_ptr;\n";
     push @CODE,    "\t__temp_markstack_ptr = PL_markstack_ptr++;\n";
   }

And the (portable ?) patch for I-C's C.pm:

--- lib/Inline/C.pm_orig	2022-03-03 02:38:00.000000000 +1100
+++ lib/Inline/C.pm	2023-09-28 22:12:53.693760500 +1000
@@ -735,7 +735,7 @@
                 $XS .= <<END;
         PREINIT:
         PerlIO* stream;
-        I32* temp;
+        IV* temp;
         PPCODE:
         temp = PL_markstack_ptr++;
         $function($arg_name_list);
@@ -755,7 +755,7 @@
             else {
                 $XS .= <<END;
         PREINIT:
-        I32* temp;
+        IV* temp;
         PPCODE:
         temp = PL_markstack_ptr++;
         $function($arg_name_list);
@@ -772,7 +772,7 @@
         elsif ($listargs) {
             $XS .= <<END;
         PREINIT:
-        I32* temp;
+        IV* temp;
         CODE:
         temp = PL_markstack_ptr++;
         RETVAL = $function($arg_name_list);

Does anyone envisage there being any portabllity issues with those 2 patches ?
So far, I've only tested them on MSWin32-x64-multi-thread, but I'll test them on other Windows configurations, including 32-bit (x86) builds, before sending them off to the appropriate places.

Cheers,
Rob

[sisyphus]

For some reason, on 32-bit Windows builds of current blead, the problematic temp variable in Inline::C and Inline::CPP needs to be declared as int*, otherwise Inline::C throws compilation warnings and Inline::CPP throws compilation errors.
Neither I32* nor IV* are acceptable.
But, on 32-bit Windows builds of perl-5.39.3, temp could be declared as I32*.

It's a bit odd - the comments accompanying the breaking commit (as identified above by @jkeenan) indicate that 32-bit builds should not be affected.

I'll take a fresh look at it later.

Cheers,
Rob

[sisyphus]

Seems to work ok if I change the declaration of Cpm'stemp and CPP.pm's __temp_markstack_ptr to SSize_t*.

I think (unverified as yet) that Inline::CPP also requires that Inline::C has been patched.

I'll file issues against Inline::C and Inline::CPP.
Here is the revised patch for Inline::C:

--- lib/Inline/C.pm_orig	2022-03-03 02:38:00.000000000 +1100
+++ lib/Inline/C.pm	2023-10-01 00:22:46.690094000 +1000
@@ -735,7 +735,7 @@
                 $XS .= <<END;
         PREINIT:
         PerlIO* stream;
-        I32* temp;
+        SSize_t* temp;
         PPCODE:
         temp = PL_markstack_ptr++;
         $function($arg_name_list);
@@ -755,7 +755,7 @@
             else {
                 $XS .= <<END;
         PREINIT:
-        I32* temp;
+        SSize_t* temp;
         PPCODE:
         temp = PL_markstack_ptr++;
         $function($arg_name_list);
@@ -772,7 +772,7 @@
         elsif ($listargs) {
             $XS .= <<END;
         PREINIT:
-        I32* temp;
+        SSize_t* temp;
         CODE:
         temp = PL_markstack_ptr++;
         RETVAL = $function($arg_name_list);

and the revised patch for Inline::CPP:

--- lib/Inline/CPP.pm_orig	2019-04-20 00:50:28.000000000 +1000
+++ lib/Inline/CPP.pm	2023-10-01 00:29:16.445509500 +1000
@@ -548,7 +548,7 @@
 
   # Wrap "complicated" subs in stack-checking code
   if ($void or $ellipsis) {
-    push @PREINIT, "\tI32 *\t__temp_markstack_ptr;\n";
+    push @PREINIT, "\tSSize_t *\t__temp_markstack_ptr;\n";
     push @CODE,    "\t__temp_markstack_ptr = PL_markstack_ptr++;\n";
   }

Cheers,
Rob

[tonycoz]

I'll look at this.

The problem with directly using SSize_t *temp; is that it will be incorrect for older perls, which need the I32.

Inline::CPP in particular could use push @CODE, "auto \t__temp_markstack_ptr = PL_markstack_ptr++;\n"; and eliminate the @PREINIT, assuming they want to require C++11, but I suspect they don't.

I plan to make the stack offset resizing optional, as with RC_STACK and add a new type that's I32 if it's disabled (via ppport.h for old perls) and SSize_t for new perls with the option on.

[tonycoz]

Fixed by #21552