Front page | perl.perl5.porters |
Postings from August 2013
Re: HP-UX 11.23/PA
Thread Previous
|
Thread Next
From:
Nicholas Clark
Date:
August 8, 2013 09:22
Subject:
Re: HP-UX 11.23/PA
Message ID:
20130808092209.GK3729@plum.flirble.org
On Mon, Jul 29, 2013 at 03:18:34PM +0100, Nicholas Clark wrote:
> On Thu, Jul 25, 2013 at 02:46:09PM +0200, H.Merijn Brand wrote:
>
> > Test Summary Report
> > -------------------
> > re/pat_thr.t (Wstat: 138 Tests: 508 Failed: 0)
> > Non-zero wait status: 138
> > Parse errors: Bad plan. You planned 670 tests but ran 508.
>
> I can replicate this on the old machine (providing I remember to build with
> -Duse64bitall and -Dusethreads)
>
> I can also bisect it with this:
>
> perl Porting/bisect.pl --start=v5.18.0 -Dusethreads -Duse64bitall -Dnoextensions=Encode -- ./perl -MTestInit t/re/pat_thr.t
>
> (-Dnoextenions=Encode is an attempt to speed it up slightly)
>
> and it says that the first commit where that fails is this:
>
> f1e1b256c5c1773d90e828cca6323c53fa23391b is the first bad commit
> commit f1e1b256c5c1773d90e828cca6323c53fa23391b
> Author: Yves Orton <demerphq@gmail.com>
> Date: Tue Jun 25 21:01:27 2013 +0200
>
> Fix rules for parsing numeric escapes in regexes
>
>
>
> I *can't* replicate that on a 64 bit big endian Linux machine, so I'm
> confused. Also, the debugger on the old HP-UX machine turns out to be
> epic fail :-( when it comes to 64 bit binaries:
>
>
> $ gdb ./perl
> Detected 64-bit executable.
> Invoking /pro/local/bin/gdb64.
> /usr/lib/pa20_64/dld.sl: Unable to find library 'libiconv.sl.4'.
> Killed
It turns out I didn't need to fight various HP debuggers (much). In that, the
combination of SIGBUS and "the stack seems to be corrupt" triggered something
in my head. At which point I could replicate it (down to the same test) by
tweaking the stack size on x86_64 Linux. It's nothing directly to do with
architecture or OS. The default thread stack size on PA-RISC HP-UX is too
small. The regex compiler recurses (in C) for each () group, so 100 nested
() groups makes an abnormally large amount of recursion.
I could reproduce it by setting the thread stack size to 86016 on x86_64
(that number will vary with build options), and tune the failure point
earlier or later by changing the thread size. I fixed it like this:
commit 66478a1b3096c57617a7adbfc1efe497d55d60ad
Author: Nicholas Clark <nick@ccl4.org>
Date: Tue Jul 30 19:59:48 2013 +0200
Set a large thread stack when running the regex tests in a thread.
For testing ithreads cloning, all the regex tests are run twice. Once
"normally", and once in a child ithread, to verify that all regex
constructions can be cloned.
The recently added tests for backreferences starting with 8 or 9 causes a
lot of C recursion in the child thread, enough to bust the default thread
stack size on (at least) HP-UX. So set a large explicit thread stack size.
It doesn't matter that it's large, as we are only running one child thread.
diff --git a/t/re/pat.t b/t/re/pat.t
index 020068c..fcc52ca 100644
--- a/t/re/pat.t
+++ b/t/re/pat.t
@@ -1419,6 +1419,8 @@ EOP
{
# if we have 87 capture buffers defined then \87 should refer to the 87th.
# test that this is true for 1..100
+ # Note that this test causes the engine to recurse at runtime, and henc
+ # use a lot of C stack.
for my $i (1..100) {
my $capture= "a";
$capture= "($capture)" for 1 .. $i;
diff --git a/t/thread_it.pl b/t/thread_it.pl
index 37d4680..aa5e18b 100644
--- a/t/thread_it.pl
+++ b/t/thread_it.pl
@@ -26,7 +26,12 @@ require $file;
note('running tests in a new thread');
-my $curr = threads->create(sub {
+# Currently 59*4096 is the minimum stack size to just get t/re/pat_thr.t to
+# pass on HP-UX 64bit PA-RISC. The test for capture buffers (eg \87)
+# recurses heavily, and busts the default stack size (65536 on PA-RISC)
+my $curr = threads->create({
+ stack_size => 524288,
+ }, sub {
run_tests();
return defined &curr_test ? curr_test() : ()
})->join();
It's good that perl itself is "stackless", else we'd hit these stack issues
with threading whenever Perl code recurses too much.
> > ../dist/Storable/t/blessed.t (Wstat: 6400 Tests: 75 Failed: 0)
>
> I can replicate this with
>
> perl Porting/bisect.pl --start=v5.18.0 -Duse64bitall -Dnoextensions=Encode -- ./perl -MTestInit dist/Storable/t/blessed.t
>
> Author: Father Chrysostomos <sprout@cpan.org>
> Date: Sun Jul 21 00:38:28 2013 -0700
>
> [perl #72766] Allow huge pos() settings
>
> Again, I can't replicate it on a big endian Linux machine, and the failure
> of the debugger makes it pretty hard to figure it out locally.
I wasn't right on that. I could replicate it on anything big endian 64 bit.
I fixed it with:
commit d2af8e81df47b54a6c2fffeb9458370d03498c4e
Author: Nicholas Clark <nick@ccl4.org>
Date: Wed Jul 31 12:43:51 2013 +0200
Storable should not assume that sizeof(mg_len) is 4.
Commit 6174b39a88cd4874 changed mg_len from I32 to SSize_t. sizeof(I32) is 4
everywhere (except certain Crays, for which Storable has work-around code).
In the version object serialisation code, Storable was passing mg->len
directly to the macro WLEN(), and and it turns out that some paths through
this macro is relying on the assumption that the value passed in is 32 bits.
This is now invalid on on 64 bit systems, but only triggered an error with
the existing tests on big endian systems.
The easiest fix is to assign the value to a temporary variable of the
correct size, and process that. However, a lot of the code makes a lot of
unwarranted assumptions about sizeof(int), and ideally should be audited and
rewritten to use more appropriate types.
diff --git a/dist/Storable/Storable.xs b/dist/Storable/Storable.xs
index 150d9e8..9e461f2 100644
--- a/dist/Storable/Storable.xs
+++ b/dist/Storable/Storable.xs
@@ -885,6 +885,7 @@ static const char byteorderstr_56[] = {BYTEORDER_BYTES_56, 0};
#ifdef HAS_HTONL
#define WLEN(x) \
STMT_START { \
+ ASSERT(sizeof(x) == sizeof(int), ("WLEN writing an int")); \
if (cxt->netorder) { \
int y = (int) htonl(x); \
if (!cxt->fio) \
@@ -2191,9 +2192,13 @@ static int store_scalar(pTHX_ stcxt_t *cxt, SV *sv)
string:
#ifdef SvVOK
- if (SvMAGICAL(sv) && (mg = mg_find(sv, 'V')))
+ if (SvMAGICAL(sv) && (mg = mg_find(sv, 'V'))) {
+ /* The macro passes this by address, not value, and a lot of
+ assumes that it's 32 bits without checking. */
+ const int len = mg->mg_len;
STORE_PV_LEN((const char *)mg->mg_ptr,
- mg->mg_len, SX_VSTRING, SX_LVSTRING);
+ len, SX_VSTRING, SX_LVSTRING);
+ }
#endif
wlen = (I32) len; /* WLEN via STORE_SCALAR expects I32 */
Nicholas Clark
Thread Previous
|
Thread Next