Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign up[patch] Restore ability to debug threaded scripts #14605
Comments
This comment has been minimized.
This comment has been minimized.
From vega.james@gmail.comPrior to Perl 5.18, it was possible to use “perl -d threaded-script.pl” lock can only be used on shared values errors coming out of perl5db.pl. It was possible to work around this by Ok, some sanity is restored. However, it would still be nice to be able Modification of a read-only value attempted errors while trying to grow @stack in perl5db.pl. The second patch Thank you for considering these patches. Currently, 5.18 and 5.20 are Cheers, |
This comment has been minimized.
This comment has been minimized.
From vega.james@gmail.com0001-lib-perl5db.pl-Restore-noop-lock-prototype.patchFrom c3cc714b2096b20613323b4187bbc9a02a5c9314 Mon Sep 17 00:00:00 2001
From: James McCoy <vega.james@gmail.com>
Date: Thu, 19 Mar 2015 22:55:18 -0400
Subject: [PATCH 1/2] lib/perl5db.pl: Restore noop lock prototype
cde405a6b9b86bd8110f63531b42d89590a4c56e removed the lock prototype
"because it's already a do-nothing weak keyword without threads".
However, that causes "perl -d threaded-script.pl" to complain
lock can only be used on shared values at /usr/share/perl/5.20/perl5db.pl line 4101.
BEGIN failed--compilation aborted at threaded-script.pl line 2.
lock can only be used on shared values at /usr/share/perl/5.20/perl5db.pl line 2514.
END failed--call queue aborted at threaded-script.pl line 2.
Unbalanced scopes: 3 more ENTERs than LEAVEs
because threaded-script.pl's importing of threads::shared enable's
lock()'s non-noop behavior. Restoring the lock() prototype fixes the
inconsistency between lock() and share() usage.
Signed-off-by: James McCoy <vega.james@gmail.com>
---
lib/perl5db.pl | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/perl5db.pl b/lib/perl5db.pl
index 8babb45..4194428 100644
--- a/lib/perl5db.pl
+++ b/lib/perl5db.pl
@@ -871,6 +871,7 @@ BEGIN {
lock($DBGR);
print "Threads support enabled\n";
} else {
+ *lock = sub(*) {};
*share = sub(\[$@%]) {};
}
}
--
2.1.4
|
This comment has been minimized.
This comment has been minimized.
From vega.james@gmail.com0002-lib-perl5db.pl-Fix-read-only-value-modification.patchFrom dbe49a5757b761263f75db1d82dc510a8bf5775e Mon Sep 17 00:00:00 2001
From: James McCoy <vega.james@gmail.com>
Date: Thu, 19 Mar 2015 23:05:35 -0400
Subject: [PATCH 2/2] lib/perl5db.pl: Fix "read-only value" modification
ce0d59fdd1c7d145efdf6bf8da56a259fed483e4 changed handling of
non-existent array elements. This causes "perl -d threaded-script.pl"
to error out with the "Modification of a read-only value attempted"
error.
Pushing onto @stack rather than resizing by changing the max index
avoids this error.
Signed-off-by: James McCoy <vega.james@gmail.com>
---
lib/perl5db.pl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/perl5db.pl b/lib/perl5db.pl
index 4194428..b2e1093 100644
--- a/lib/perl5db.pl
+++ b/lib/perl5db.pl
@@ -4138,7 +4138,7 @@ sub DB::sub {
local $stack_depth = $stack_depth + 1; # Protect from non-local exits
# Expand @stack.
- $#stack = $stack_depth;
+ push @stack, (0) x ($stack_depth - $#stack);
# Save current single-step setting.
$stack[-1] = $single;
@@ -4279,7 +4279,7 @@ sub lsub : lvalue {
local $stack_depth = $stack_depth + 1; # Protect from non-local exits
# Expand @stack.
- $#stack = $stack_depth;
+ push @stack, (0) x ($stack_depth - $#stack);
# Save current single-step setting.
$stack[-1] = $single;
--
2.1.4
|
This comment has been minimized.
This comment has been minimized.
From @jkeenanOn Fri Mar 20 04:11:22 2015, vega.james@gmail.com wrote:
[snip] Would you be able to attach (a) a small but meaningful example of a program with threads that exhibits this behavior under the debugger? Also, could you attach the output of 'perl -V' from the perl build you are using to run that program? Thank you very much. -- |
This comment has been minimized.
This comment has been minimized.
|
The RT System itself - Status changed from 'new' to 'open' |
This comment has been minimized.
This comment has been minimized.
From vega.james@gmail.comOn Sat, Mar 21, 2015 at 05:18:17AM -0700, James E Keenan via RT wrote:
Sure. Attached. Cheers, |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
From vega.james@gmail.comSummary of my perl5 (revision 5 version 20 subversion 2) configuration: Characteristics of this binary (from libperl): |
This comment has been minimized.
This comment has been minimized.
From @jkeenanOn Sat Mar 21 08:15:51 2015, vega.james@gmail.com wrote:
Problem confirmed. I reduced the list of config_args to just '-Dusethreads' to simplify the analysis. See file attached for results before and after applying poster's two patches. -- |
This comment has been minimized.
This comment has been minimized.
From @jkeenan# blead, threaded build at commit 5390239 sh ./Configure -des -Dusedevel -Dusethreads & make -j8 $ ./perl -Ilib -d ~/learn/perl/p5p/124127-foo.pl Loading DB routines from perl5db.pl version 1.48 Enter h or 'h h' for help, or 'man perldebug' for more help. lock can only be used on shared values at lib/perl5db.pl line 4116. ##### # git clean -dfx; apply both patches; Configure and build anew as above $ ./perl -Ilib -d ~/learn/perl/p5p/124127-foo.pl Loading DB routines from perl5db.pl version 1.48 Enter h or 'h h' for help, or 'man perldebug' for more help. main::(/home/jkeenan/learn/perl/p5p/124127-foo.pl:12): |
This comment has been minimized.
This comment has been minimized.
From vega.james@gmail.comOn Sat, Mar 21, 2015 at 09:49:07AM -0700, James E Keenan via RT wrote:
Would the fix for these issues be candidates for maintenance releases, Cheers, |
This comment has been minimized.
This comment has been minimized.
From @jkeenanOn Tue Mar 24 19:26:59 2015, vega.james@gmail.com wrote:
It would be a candidate but that's subject to caveats. 1) While I've confirmed the bug, I haven't confirmed the fix -- and it's probably outside my expertise to do so. p5p list: Can you review this patch? 2) 5.18 is nearing the May 2015 end of its maintenance window. It is up to the maintenance pumpking to decide whether there will be another release of 5.18 at all. If so, and if the patch is approved, it is eligible. Thank you very much. -- |
This comment has been minimized.
This comment has been minimized.
From @jkeenanOn Wed Mar 25 03:27:08 2015, jkeenan wrote:
Another bug report (https://rt-archive.perl.org/perl5/Ticket/Display.html?id=124203) touched upon a very similar problem, but was reported to have introduced a new bug. -- |
This comment has been minimized.
This comment has been minimized.
From vega.james@gmail.comOn Sun, Mar 29, 2015 at 03:03:59PM -0700, James E Keenan via RT wrote:
Hmm, I'm not really sure how to debug the debugger. The patches were If it is introduced by my patches and not just uncovered by being able I'll try to dig into this more over the next few days, regardless. Cheers, |
This comment has been minimized.
This comment has been minimized.
From vega.james@gmail.comOn Sun, Mar 29, 2015 at 10:52:00PM -0400, James McCoy wrote:
Except I was missing some logic. :) Updated patch attached to handle the Cheers, |
This comment has been minimized.
This comment has been minimized.
From vega.james@gmail.com0002-lib-perl5db.pl-Fix-read-only-value-modification.patchFrom 05bc4308e4b82982cebdf08c970cba7bbb599356 Mon Sep 17 00:00:00 2001
From: James McCoy <vega.james@gmail.com>
Date: Thu, 19 Mar 2015 23:05:35 -0400
Subject: [PATCH 2/2] lib/perl5db.pl: Fix "read-only value" modification
ce0d59fdd1c7d145efdf6bf8da56a259fed483e4 changed handling of
non-existent array elements. This causes "perl -d threaded-script.pl"
to error out with the "Modification of a read-only value attempted"
error.
Pushing/popping @stack rather than resizing by changing the max index
avoids this error.
Signed-off-by: James McCoy <vega.james@gmail.com>
---
lib/perl5db.pl | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/lib/perl5db.pl b/lib/perl5db.pl
index 4194428..f531f03 100644
--- a/lib/perl5db.pl
+++ b/lib/perl5db.pl
@@ -4138,7 +4138,13 @@ sub DB::sub {
local $stack_depth = $stack_depth + 1; # Protect from non-local exits
# Expand @stack.
- $#stack = $stack_depth;
+ my $stack_change = $stack_depth - $#stack;
+ if ($stack_change > 0) {
+ push @stack, (undef) x $stack_change;
+ }
+ else {
+ pop @stack while $stack_change++;
+ }
# Save current single-step setting.
$stack[-1] = $single;
@@ -4279,7 +4285,13 @@ sub lsub : lvalue {
local $stack_depth = $stack_depth + 1; # Protect from non-local exits
# Expand @stack.
- $#stack = $stack_depth;
+ my $stack_change = $stack_depth - $#stack;
+ if ($stack_change > 0) {
+ push @stack, (undef) x $stack_change;
+ }
+ else {
+ pop @stack while $stack_change++;
+ }
# Save current single-step setting.
$stack[-1] = $single;
--
2.1.4
|
This comment has been minimized.
This comment has been minimized.
From @ntyniOn Sun, Mar 29, 2015 at 11:54:52PM -0400, James McCoy wrote:
This seems to me like a workaround for a deeper problem. I don't see why It's probably the same thing as this regression: % perl -Mthreads -le 'my @a = ("foo"); threads->create(sub { $#a=1; $a[1]="bar"; print @a})->join;' which (also) regressed with ce0d59f. Reproducible with current blead (v5.21.10-71-g2c04452) |
This comment has been minimized.
This comment has been minimized.
From @tonycozOn Mon Apr 13 11:47:06 2015, ntyni@debian.org wrote:
The attached fixes that issue for me, but when I try to use the debugger I see: $ ./perl -Ilib -d ../123127.pl Loading DB routines from perl5db.pl version 1.48 Enter h or 'h h' for help, or 'man perldebug' for more help. lock can only be used on shared values at lib/perl5db.pl line 4116. (-DDEBUGGING build) Tony |
This comment has been minimized.
This comment has been minimized.
From @tonycoz0001-perl-124127-fix-cloning-arrays-with-unused-elements.patchFrom a7efcbd868beeeca474ca98b6137570053c6fecc Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 14 Apr 2015 15:59:03 +1000
Subject: [PATCH] [perl #124127] fix cloning arrays with unused elements
ce0d59fd changed arrays to use NULL instead of &PL_sv_undef for
unused elements, unfortunately it missed updating sv_dup_common()'s
initialization of unused elements, leaving them as NULL.
This resulted in modification of read only value errors at runtime.
---
sv.c | 2 +-
t/op/threads.t | 8 +++++++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/sv.c b/sv.c
index 467dc24..2bb0346 100644
--- a/sv.c
+++ b/sv.c
@@ -13641,7 +13641,7 @@ S_sv_dup_common(pTHX_ const SV *const sstr, CLONE_PARAMS *const param)
}
items = AvMAX((const AV *)sstr) - AvFILLp((const AV *)sstr);
while (items-- > 0) {
- *dst_ary++ = &PL_sv_undef;
+ *dst_ary++ = NULL;
}
}
else {
diff --git a/t/op/threads.t b/t/op/threads.t
index 6fb2410..e76c956 100644
--- a/t/op/threads.t
+++ b/t/op/threads.t
@@ -9,7 +9,7 @@ BEGIN {
skip_all_without_config('useithreads');
skip_all_if_miniperl("no dynamic loading on miniperl, no threads");
- plan(27);
+ plan(28);
}
use strict;
@@ -399,4 +399,10 @@ fresh_perl_is(
'no crash when deleting $::{INC} in thread'
);
+fresh_perl_is(<<'CODE', 'ok', 'no crash modifying extended array element');
+use threads;
+my @a = 1;
+threads->create(sub { $#a = 1; $a[1] = 2; print qq/ok\n/ })->join;
+CODE
+
# EOF
--
1.7.10.4
|
This comment has been minimized.
This comment has been minimized.
From @ntyniOn Mon, Apr 13, 2015 at 11:06:21PM -0700, Tony Cook via RT wrote:
Awesome, thanks! The original patch 1/2 in this ticket by James McCoy So the 'perl -d threaded-script.pl' breakage was a 5.17.6 regression and the I can confirm that applying your patch and Hope this clarifies |
This comment has been minimized.
This comment has been minimized.
From @tonycozOn Tue Apr 14 11:42:47 2015, ntyni@debian.org wrote:
You're right. This fixes the default case, unfortunately PERL5DB_THREADED is also broken, I'll open a new ticket for that. Tony |
This comment has been minimized.
This comment has been minimized.
From @tonycozOn Tue Apr 14 17:23:55 2015, tonyc wrote:
These aren't regressions from 5.20, but I think they're serious enough to be Opinions? Tony |
This comment has been minimized.
This comment has been minimized.
From @tonycozOn Tue Apr 14 18:14:16 2015, tonyc wrote:
Applied as 41ef2c6 and 902d169. Closing Tony |
This comment has been minimized.
This comment has been minimized.
|
@tonycoz - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#124127 (status was 'resolved')
Searchable as RT124127$