disable Storable stacksize probing #16780
Comments
From @ntyniThis is a bug report for perl from ntyni@debian.org, It would be nice to have a way to disable the Storable build time Probing system properties at build time is not ideal for binary The probing also has implications for build reproducibility: having I see cross compiling is already an exception in blead, perhaps that (This is also https://bugs.debian.org/914133 ) Many thanks for your work on Perl! Flags: Site configuration information for perl 5.28.1: Configured by Debian at Thu Nov 29 19:17:43 UTC 2018. Summary of my perl5 (revision 5 version 28 subversion 1) configuration: Locally applied patches: @INC for perl 5.28.1: Environment for perl 5.28.1: |
From @tonycozOn Sun, 02 Dec 2018 04:49:43 -0800, ntyni@debian.org wrote:
I'll remove build-time probing entirely. I'll leave the stacksize tool in the distribution, but it won't be generating lib/Storable/Limit.pm anymore. Tony |
|
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Mon, 03 Dec 2018 16:03:34 -0800, tonyc wrote:
Like this. Tony |
From @tonycoz0001-perl-133708-remove-build-time-probing-for-stack-limi.patchFrom ab16f905f7ef09c44575581bfa3322dd963b4997 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 4 Dec 2018 15:11:17 +1100
Subject: (perl #133708) remove build-time probing for stack limits for
Storable
---
.gitignore | 1 -
Makefile.SH | 25 --------------
dist/.gitignore | 1 -
dist/Storable/ChangeLog | 4 +++
dist/Storable/Makefile.PL | 40 ----------------------
dist/Storable/__Storable__.pm | 14 ++++----
dist/Storable/stacksize | 77 ++++++++++---------------------------------
dist/Storable/t/recurse.t | 4 ++-
win32/GNUmakefile | 12 +------
win32/Makefile | 10 +-----
win32/makefile.mk | 15 ++-------
11 files changed, 36 insertions(+), 167 deletions(-)
diff --git a/.gitignore b/.gitignore
index 6ce50f0efd..7f3b7b1933 100644
--- a/.gitignore
+++ b/.gitignore
@@ -116,7 +116,6 @@ lib/Config.pod
lib/Cross.pm
lib/ExtUtils/MANIFEST.SKIP
lib/ExtUtils/xsubpp
-lib/Storable/Limit.pm
lib/auto/
lib/perldoc.pod
lib/buildcustomize.pl
diff --git a/Makefile.SH b/Makefile.SH
index 8ea2ba80e0..757a369e6a 100755
--- a/Makefile.SH
+++ b/Makefile.SH
@@ -184,15 +184,11 @@ esac
: is Cwd static or dynamic
static_cwd='define'
-storable_limit_dep=''
-storable_type=''
list_util_dep='$(PERL_EXE)'
for f in $dynamic_ext; do
case $f in
Cwd) static_cwd='undef' ;;
List/Util) list_util_dep=lib/auto/List/Util/Util.$dlext ;;
- Storable) storable_limit_dep=lib/auto/Storable/Storable.$dlext
- storable_type='dynamic' ;;
esac
done
@@ -229,7 +225,6 @@ for f in $static_ext; do
$this_target: lib/auto/List/Util/Util\$(LIB_EXT)" ;;
Unicode/Normalize) extra_dep="$extra_dep
$this_target: uni.data" ;;
- Storable) storable_type='static' ;;
esac
done
@@ -1188,26 +1183,6 @@ no_install no-install: install-notify
install: install-all
!NO!SUBS!
-if test "$storable_type" != "" ; then
-
-case "$static_cwd" in
-undef) storable_limit_dep="$storable_limit_dep lib/auto/Cwd/Cwd.$dlext" ;;
-esac
-
-$spitshell >>$Makefile <<EOT
-
-dist/Storable/lib/Storable/Limit.pm : \$(PERL_EXE) dist/Storable/stacksize $storable_limit_dep
- cd dist/Storable ; \$(LDLIBPTH) \$(MAKE) lib/Storable/Limit.pm
-
-lib/Storable/Limit.pm : dist/Storable/lib/Storable/Limit.pm
- test -d lib/Storable || mkdir lib/Storable
- cp dist/Storable/lib/Storable/Limit.pm lib/Storable/Limit.pm
-EOT
-
-common_build_deps="$common_build_deps lib/Storable/Limit.pm"
-
-fi
-
for name in all notify silent strip verbose; do
flags="--$name";
flags=`echo $flags | sed -e 's/--all//'`
diff --git a/dist/.gitignore b/dist/.gitignore
index 2120f50040..d8a22d5c8f 100644
--- a/dist/.gitignore
+++ b/dist/.gitignore
@@ -6,6 +6,5 @@ pm_to_blib
Makefile
Makefile.PL
ppport.h
-Storable/lib/Storable/Limit.pm
Time-HiRes/*.inc
Time-HiRes/xdefine
diff --git a/dist/Storable/ChangeLog b/dist/Storable/ChangeLog
index 401466fbf5..5e63ea66e2 100644
--- a/dist/Storable/ChangeLog
+++ b/dist/Storable/ChangeLog
@@ -1,4 +1,8 @@
unreleased
+ version 3.14
+ * (perl #133708) don't build-time probe for stack limits at all
+
+unreleased
version 3.12
* (perl #133411) don't probe for stack limits with -Dusecrosscompile
diff --git a/dist/Storable/Makefile.PL b/dist/Storable/Makefile.PL
index db420ab30b..4a39125562 100644
--- a/dist/Storable/Makefile.PL
+++ b/dist/Storable/Makefile.PL
@@ -13,15 +13,7 @@ use Config;
use File::Copy qw(move copy);
use File::Spec;
-unlink "lib/Storable/Limit.pm";
-
-my $limit_pm = File::Spec->catfile('lib', 'Storable', 'Limit.pm');
-
my $pm = { 'Storable.pm' => '$(INST_ARCHLIB)/Storable.pm' };
-unless ($ENV{PERL_CORE}) {
- # the core Makefile takes care of this for core builds
- $pm->{$limit_pm} = '$(INST_ARCHLIB)/Storable/Limit.pm';
-}
WriteMakefile(
NAME => 'Storable',
@@ -86,27 +78,7 @@ sub xlinkext {
}
sub depend {
- my $extra_deps = "";
- my $options = "";
- if ($ENV{PERL_CORE}) {
- $options = "--core";
- }
- else {
- # blib.pm needs arch/lib
- $extra_deps = ' Storable.pm';
- }
- my $whichperl;
- if ($Config::Config{usecrosscompile}) {
- $whichperl = '$(PERLRUN)';
- }
- else {
- $whichperl = '$(FULLPERLRUNINST)';
- }
- my $linktype = uc($_[0]->{LINKTYPE});
"
-$limit_pm : stacksize \$(INST_$linktype)$extra_deps
- \$(MKPATH) \$(INST_LIB)
- $whichperl stacksize $options
release : dist
git tag \$(VERSION)
@@ -116,18 +88,6 @@ release : dist
"
}
-sub test {
- my ($self, %attr) = @_;
-
- my $out = $self->SUPER::test(%attr);
-
- if ($ENV{PERL_CORE}) {
- $out =~ s!^(test(?:db)?_(?:static|dynamic)\b.*)!$1 $limit_pm!gm;
- }
-
- $out;
-}
-
sub postamble {
'
all :: Storable.pm
diff --git a/dist/Storable/__Storable__.pm b/dist/Storable/__Storable__.pm
index 6c36099bed..cf291b4b6e 100644
--- a/dist/Storable/__Storable__.pm
+++ b/dist/Storable/__Storable__.pm
@@ -27,13 +27,11 @@ our @EXPORT_OK = qw(
our ($canonical, $forgive_me);
-our $VERSION = '3.13';
+our $VERSION = '3.14';
our $recursion_limit;
our $recursion_limit_hash;
-do "Storable/Limit.pm";
-
$recursion_limit = 512
unless defined $recursion_limit;
$recursion_limit_hash = 256
@@ -950,13 +948,13 @@ There are a few things you need to know, however:
=item *
-Since Storable 3.05 we probe for the stack recursion limit for references,
+From Storable 3.05 to 3.13 we probed for the stack recursion limit for references,
arrays and hashes to a maximal depth of ~1200-35000, otherwise we might
fall into a stack-overflow. On JSON::XS this limit is 512 btw. With
references not immediately referencing each other there's no such
limit yet, so you might fall into such a stack-overflow segfault.
-This probing and the checks performed have some limitations:
+This probing and the checks we performed have some limitations:
=over
@@ -964,7 +962,9 @@ This probing and the checks performed have some limitations:
the stack size at build time might be different at run time, eg. the
stack size may have been modified with ulimit(1). If it's larger at
-run time Storable may fail the freeze() or thaw() unnecessarily.
+run time Storable may fail the freeze() or thaw() unnecessarily. If
+it's larger at build time Storable may segmentation fault when
+processing a deep structure.
=item *
@@ -979,6 +979,8 @@ stack without triggering Storable's recursion protection.
=back
+So these now have simple defaults rather than probing at build-time.
+
You can control the maximum array and hash recursion depths by
modifying C<$Storable::recursion_limit> and
C<$Storable::recursion_limit_hash> respectively. Either can be set to
diff --git a/dist/Storable/stacksize b/dist/Storable/stacksize
index 14e0739734..f93eccce1a 100644
--- a/dist/Storable/stacksize
+++ b/dist/Storable/stacksize
@@ -1,21 +1,17 @@
#!/usr/bin/perl
# binary search maximum stack depth for arrays and hashes
-# and store it in lib/Storable/Limit.pm
+# and report it to stdout as code to set the limits
use Config;
use Cwd;
use File::Spec;
use strict;
--d "lib" or mkdir "lib";
--d "lib/Storable" or mkdir "lib/Storable";
-
-my $fn = "lib/Storable/Limit.pm";
my $ptrsize = $Config{ptrsize};
my ($bad1, $bad2) = (65001, 25000);
sub QUIET () {
(defined $ENV{MAKEFLAGS} and $ENV{MAKEFLAGS} =~ /\b(s|silent|quiet)\b/
- and !defined($ENV{TRAVIS}))
+ and !defined($ENV{TRAVIS})) || @ARGV && $ARGV[0] eq "-q"
? 1 : 0
}
sub PARALLEL () {
@@ -32,11 +28,7 @@ sub is_miniperl {
}
if (is_miniperl()) {
- if ($Config{usecrosscompile}) {
- write_limits(500, 265);
- exit;
- }
- die "Should not run during miniperl\n";
+ die "Should not run using miniperl\n";
}
my $prefix = "";
if ($^O eq "MSWin32") {
@@ -51,58 +43,28 @@ elsif (system("ulimit -c 0 ;") == 0) {
# try to prevent core dumps
$prefix = "ulimit -c 0 ; ";
}
-if (@ARGV and $ARGV[0] eq '--core') {
- $ENV{PERL_CORE} = 1;
-}
my $PERL = $^X;
-if ($ENV{PERL_CORE}) {
- my $path;
- my $ldlib = $Config{ldlibpthname};
- if (-d 'dist/Storable') {
- chdir 'dist/Storable';
- $PERL = "../../$PERL" unless $PERL =~ m|^/|;
- }
- if ($ldlib) {
- $path = getcwd()."/../..";
- }
- if ($^O eq 'MSWin32' and -d '../dist/Storable') {
- chdir '..\dist\Storable';
- $PERL = "..\\..\\$PERL" unless $PERL =~ /^[A-Za-z]:\\/;
- }
- $PERL = "\"$PERL\"" if $PERL =~ / /;
- if ($ldlib and $ldlib ne 'PATH') {
- $PERL = "$ldlib=$path $PERL";
- }
-}
-
if ($^O eq "MSWin32") {
require Win32;
my ($str, $major, $minor) = Win32::GetOSVersion();
if ($major < 6 || $major == 6 && $minor < 1) {
- print "Using defaults for older Win32\n";
+ print "# Using defaults for older Win32\n";
write_limits(500, 256);
exit;
}
}
my ($n, $good, $bad, $found) =
(65000, 100, $bad1, undef);
-print "probe for max. stack sizes...\n" unless QUIET;
+print "# probe for max. stack sizes...\n" unless QUIET;
# -I. since we're run before pm_to_blib (which is going to copy the
# file we create) and need to load our Storable.pm, not the already
# installed Storable.pm
-my $mblib = '-Mblib -I.';
-if ($ENV{PERL_CORE}) {
- if ($^O eq 'MSWin32') {
- $mblib = '-I..\..\lib\auto -I..\..\lib';
- } else {
- $mblib = '-I../../lib/auto -I../../lib';
- }
+my $mblib = '';
+if (-d 'blib') {
+ $mblib = '-Mblib -I.';
}
-if (PARALLEL) {
- # problem with parallel builds. wait for INST_DYNAMIC linking to be done.
- # the problem is the RM_F INST_DYNAMIC race.
- print "parallel build race - wait for linker ...\n" unless QUIET;
- sleep(2.0);
+elsif (-f "Configure") {
+ $mblib = '-Ilib';
}
sub cmd {
@@ -117,7 +79,7 @@ sub cmd {
sub good {
my $i = shift; # this passed
my $j = $i + abs(int(($bad - $i) / 2));
- print "Storable: determining recursion limit: $i passed, try more $j ...\n" unless QUIET;
+ print "# Storable: determining recursion limit: $i passed, try more $j ...\n" unless QUIET;
$good = $i;
if ($j <= $i) {
$found++;
@@ -128,7 +90,7 @@ sub good {
sub bad {
my $i = shift; # this failed
my $j = $i - abs(int(($i - $good) / 2));
- print "Storable: determining recursion limit: $i too big, try less $j ...\n" unless QUIET;
+ print "# Storable: determining recursion limit: $i too big, try less $j ...\n" unless QUIET;
$bad = $i;
if ($j >= $i) {
$j = $good;
@@ -162,7 +124,7 @@ while (!$found) {
$n = bad($n);
}
}
-print "MAX_DEPTH = $n\n" unless QUIET;
+print "# MAX_DEPTH = $n\n" unless QUIET;
my $max_depth = $n;
($n, $good, $bad, $found) =
@@ -186,13 +148,13 @@ if ($max_depth == $bad1-1
and $n == $bad2-1)
{
# more likely the shell. travis docker ubuntu, mingw e.g.
- print "Error: Apparently your system(SHELLSTRING) cannot catch stack overflows\n"
+ print "# Apparently your system(SHELLSTRING) cannot catch stack overflows\n"
unless QUIET;
$max_depth = 512;
$n = 256;
print "MAX_DEPTH = $max_depth\n" unless QUIET;
}
-print "MAX_DEPTH_HASH = $n\n" unless QUIET;
+print "# MAX_DEPTH_HASH = $n\n" unless QUIET;
my $max_depth_hash = $n;
# Previously this calculation was done in the macro, calculate it here
@@ -203,7 +165,7 @@ my $max_depth_hash = $n;
# several apparently random failures here, eg. working in one
# configuration, but not in a very similar configuration.
$max_depth = int(0.6 * $max_depth);
-$max_depth_hash = int(0.6 * $max_depth);
+$max_depth_hash = int(0.6 * $max_depth_hash);
my $stack_reserve = $^O eq "MSWin32" ? 32 : 16;
if ($] ge "5.016" && !($^O eq "cygwin" && $ptrsize == 8)) {
@@ -221,16 +183,11 @@ write_limits($max_depth, $max_depth_hash);
sub write_limits {
my ($max_depth, $max_depth_hash) = @_;
- my $f;
- open $f, ">", $fn or die "$fn $!";
- print $f <<EOS;
+ print <<EOS;
# bisected by stacksize
\$Storable::recursion_limit = $max_depth
unless defined \$Storable::recursion_limit;
\$Storable::recursion_limit_hash = $max_depth_hash
unless defined \$Storable::recursion_limit_hash;
-1;
EOS
- close $f
- or die "Failed to close $fn: $!\n";
}
diff --git a/dist/Storable/t/recurse.t b/dist/Storable/t/recurse.t
index 63fde90fdf..b5967a072c 100644
--- a/dist/Storable/t/recurse.t
+++ b/dist/Storable/t/recurse.t
@@ -318,9 +318,11 @@ is($refcount_ok, 1, "check refcount");
# Small 64bit systems fail with 1200 (c++ debugging), with gcc 3000.
# Optimized 64bit allows up to 33.000 recursion depth.
# with asan the limit is 255 though.
+
+local $Storable::recursion_limit = 30;
+local $Storable::recursion_limit_hash = 20;
sub MAX_DEPTH () { Storable::stack_depth() }
sub MAX_DEPTH_HASH () { Storable::stack_depth_hash() }
-sub OVERFLOW () { 35000 }
{
my $t;
print "# max depth ", MAX_DEPTH, "\n";
diff --git a/win32/GNUmakefile b/win32/GNUmakefile
index 67b980d619..b89d86f3c8 100644
--- a/win32/GNUmakefile
+++ b/win32/GNUmakefile
@@ -1208,7 +1208,7 @@ CFG_VARS = \
.PHONY: all info
-all : info rebasePE Extensions_nonxs $(PERLSTATIC) PostExt
+all : info rebasePE Extensions_nonxs $(PERLSTATIC)
info :
ifeq ($(CCTYPE),GCC)
@@ -1659,16 +1659,6 @@ Extensions_clean :
Extensions_realclean :
-if exist $(MINIPERL) $(MINIPERL) -I..\lib ..\make_ext.pl "MAKE=$(PLMAKE)" --dir=$(CPANDIR) --dir=$(DISTDIR) --dir=$(EXTDIR) --all --target=realclean
-PostExt : ..\lib\Storable\Limit.pm
-
-# we need the exe, perl(ver).dll, and the Exporter, Storable, Win32 extensions
-# rebasePE most of that, including adjustment for static builds, so we
-# just need non-xs extensions
-..\lib\Storable\Limit.pm : rebasePE Extensions_nonxs
- $(PLMAKE) -C ..\dist\Storable lib\Storable\Limit.pm
- if not exist ..\lib\Storable mkdir ..\lib\Storable
- copy ..\dist\Storable\lib\Storable\Limit.pm ..\lib\Storable\Limit.pm
-
# all PE files need to be built by the time this target runs, PP files can still
# be running in parallel like UNIDATAFILES, this target a placeholder for the
# future
diff --git a/win32/Makefile b/win32/Makefile
index 2f478ceff0..47ae6d95b9 100644
--- a/win32/Makefile
+++ b/win32/Makefile
@@ -955,7 +955,7 @@ CFG_VARS = \
#
all : ..\git_version.h $(GLOBEXE) $(CONFIGPM) \
- $(UNIDATAFILES) MakePPPort $(PERLEXE) Extensions_nonxs Extensions PostExt \
+ $(UNIDATAFILES) MakePPPort $(PERLEXE) Extensions_nonxs Extensions \
$(PERLSTATIC)
@echo Everything is up to date. '$(MAKE_BARE) test' to run test suite.
@@ -1244,13 +1244,6 @@ Extensions_clean:
Extensions_realclean:
-if exist $(MINIPERL) $(MINIPERL) -I..\lib ..\make_ext.pl "MAKE=$(MAKE)" --dir=$(CPANDIR) --dir=$(DISTDIR) --dir=$(EXTDIR) --all --target=realclean
-PostExt: ..\lib\Storable\Limit.pm
-
-..\lib\Storable\Limit.pm: $(PERLEXE) Extensions
- cd ..\dist\Storable && $(MAKE) lib\Storable\Limit.pm
- if not exist ..\lib\Storable mkdir ..\lib\Storable
- copy ..\dist\Storable\lib\Storable\Limit.pm ..\lib\Storable\Limit.pm
-
#-------------------------------------------------------------------------------
doc: $(PERLEXE) ..\pod\perltoc.pod
@@ -1329,7 +1322,6 @@ distclean: realclean
-del /f $(LIBDIR)\Time\HiRes.pm
-del /f $(LIBDIR)\Unicode\Normalize.pm
-del /f $(LIBDIR)\Math\BigInt\FastCalc.pm
- -del /f $(LIBDIR)\Storable.pm $(LIBDIR)\Storable\Limit.pm
-del /f $(LIBDIR)\Win32.pm
-del /f $(LIBDIR)\Win32CORE.pm
-del /f $(LIBDIR)\Win32API\File.pm
diff --git a/win32/makefile.mk b/win32/makefile.mk
index 6c7ac8aff3..d813624810 100644
--- a/win32/makefile.mk
+++ b/win32/makefile.mk
@@ -1181,7 +1181,7 @@ CFG_VARS = \
# Top targets
#
-all : CHECKDMAKE rebasePE Extensions_nonxs $(PERLSTATIC) PostExt
+all : CHECKDMAKE rebasePE Extensions_nonxs $(PERLSTATIC)
info :
.IF "$(CCTYPE)" == "GCC"
@@ -1629,17 +1629,6 @@ rebasePE : Extensions $(PERLDLL) $(NORMALIZE_DYN) $(PERLEXE)
.ENDIF
$(NOOP)
-PostExt : ..\lib\Storable\Limit.pm
- $(NOOP)
-
-# we need the exe, perl(ver).dll, and the Exporter, Storable, Win32 extensions
-# rebasePE most of that, including adjustment for static builds, so we
-# just need non-xs extensions
-..\lib\Storable\Limit.pm : rebasePE Extensions_nonxs
- cd ..\dist\Storable && $(MAKE) lib\Storable\Limit.pm
- if not exist ..\lib\Storable mkdir ..\lib\Storable
- copy ..\dist\Storable\lib\Storable\Limit.pm ..\lib\Storable\Limit.pm
-
#-------------------------------------------------------------------------------
@@ -1714,7 +1703,7 @@ distclean: realclean
-del /f $(LIBDIR)\Time\HiRes.pm
-del /f $(LIBDIR)\Unicode\Normalize.pm
-del /f $(LIBDIR)\Math\BigInt\FastCalc.pm
- -del /f $(LIBDIR)\Storable.pm $(LIBDIR)\Storable\Limit.pm
+ -del /f $(LIBDIR)\Storable.pm
-del /f $(LIBDIR)\Win32.pm
-del /f $(LIBDIR)\Win32CORE.pm
-del /f $(LIBDIR)\Win32API\File.pm
--
2.11.0
|
From @tonycozOn Tue, 04 Dec 2018 16:41:53 -0800, tonyc wrote:
Applied as 2a0bbd3. Tony |
|
@tonycoz - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release today of Perl 5.30.0, this and 160 other issues have been Perl 5.30.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#133708 (status was 'resolved')
Searchable as RT133708$
The text was updated successfully, but these errors were encountered: