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

[patch] Restore ability to debug threaded scripts #14605

Closed
p5pRT opened this issue Mar 20, 2015 · 24 comments
Closed

[patch] Restore ability to debug threaded scripts #14605

p5pRT opened this issue Mar 20, 2015 · 24 comments
Labels

Comments

@p5pRT
Copy link

p5pRT commented Mar 20, 2015

Migrated from rt.perl.org#124127 (status was 'resolved')

Searchable as RT124127$

@p5pRT
Copy link
Author

p5pRT commented Mar 20, 2015

From vega.james@gmail.com

Prior to Perl 5.18, it was possible to use “perl -d threaded-script.pl”
to debug a script which was imports threads/threads​::shared. Starting
in Perl 5.18, this would fail with

  lock can only be used on shared values

errors coming out of perl5db.pl. It was possible to work around this by
running “perl -dt threaded-script.pl”. This would let the debugger run
as long as no threads actually got spawned. If a thread was spawned,
then the debugger just wedges (c.f., #120170).

Ok, some sanity is restored. However, it would still be nice to be able
to run with “perl -d” again so debugging of threads can take place.
That was still failing due to

  Modification of a read-only value attempted

errors while trying to grow @​stack in perl5db.pl. The second patch
works around this by pushing onto @​stack instead of growing it through
setting $#stack to a larger value. There may be a better way to fix
this so the $#stack approach still works, but that's a bit beyond my
depth currently.

Thank you for considering these patches. Currently, 5.18 and 5.20 are
basically DOA at $work due to the above issues (and another
5.18-specific issue I'll follow up on).

Cheers,
--
James
GPG Key​: 4096R/331BA3DB 2011-12-05 James McCoy <vega.james@​gmail.com>

@p5pRT
Copy link
Author

p5pRT commented Mar 20, 2015

From vega.james@gmail.com

0001-lib-perl5db.pl-Restore-noop-lock-prototype.patch
From 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

@p5pRT
Copy link
Author

p5pRT commented Mar 20, 2015

From vega.james@gmail.com

0002-lib-perl5db.pl-Fix-read-only-value-modification.patch
From 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

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2015

From @jkeenan

On Fri Mar 20 04​:11​:22 2015, vega.james@​gmail.com wrote​:

Prior to Perl 5.18, it was possible to use “perl -d threaded-script.pl”
to debug a script which was imports threads/threads​::shared. Starting
in Perl 5.18, this would fail with

lock can only be used on shared values

errors coming out of perl5db.pl. It was possible to work around this by
running “perl -dt threaded-script.pl”. This would let the debugger run
as long as no threads actually got spawned. If a thread was spawned,
then the debugger just wedges (c.f., #120170).

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

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2015

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2015

From vega.james@gmail.com

On Sat, Mar 21, 2015 at 05​:18​:17AM -0700, James E Keenan via RT wrote​:

On Fri Mar 20 04​:11​:22 2015, vega.james@​gmail.com wrote​:

Prior to Perl 5.18, it was possible to use “perl -d threaded-script.pl”
to debug a script which was imports threads/threads​::shared. Starting
in Perl 5.18, this would fail with

lock can only be used on shared values

errors coming out of perl5db.pl. It was possible to work around this by
running “perl -dt threaded-script.pl”. This would let the debugger run
as long as no threads actually got spawned. If a thread was spawned,
then the debugger just wedges (c.f., #120170).

[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?

Sure. Attached.

Cheers,
--
James
GPG Key​: 4096R/331BA3DB 2011-12-05 James McCoy <vega.james@​gmail.com>

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2015

From vega.james@gmail.com

foo.pl

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2015

From vega.james@gmail.com

Summary of my perl5 (revision 5 version 20 subversion 2) configuration​:
 
  Platform​:
  osname=linux, osvers=3.2.0-4-amd64, archname=x86_64-linux-gnu-thread-multi
  uname='linux brahms 3.2.0-4-amd64 #1 smp debian 3.2.65-1+deb7u2 x86_64 gnulinux '
  config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Dldflags= -Wl,-z,relro -Dlddlflags=-shared -Wl,-z,relro -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.20 -Darchlib=/usr/lib/x86_64-linux-gnu/perl/5.20 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/x86_64-linux-gnu/perl5/5.20 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.20.2 -Dsitearch=/usr/local/lib/x86_64-linux-gnu/perl/5.20.2 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Duse64bitint -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -Ui_libutil -Uversiononly -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.20.2 -des'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2 -g',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include'
  ccversion='', gccversion='4.9.2', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  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 -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.9/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
  perllibs=-ldl -lm -lpthread -lc -lcrypt
  libc=libc-2.19.so, so=so, useshrplib=true, libperl=libperl.so.5.20
  gnulibc_version='2.19'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib -fstack-protector'

Characteristics of this binary (from libperl)​:
  Compile-time options​: DEBUGGING HAS_TIMES MULTIPLICITY PERLIO_LAYERS
  PERL_DONT_CREATE_GVSV
  PERL_HASH_FUNC_ONE_AT_A_TIME_HARD
  PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP
  PERL_NEW_COPY_ON_WRITE PERL_PRESERVE_IVUV
  PERL_TRACK_MEMPOOL USE_64_BIT_ALL USE_64_BIT_INT
  USE_ITHREADS USE_LARGE_FILES USE_LOCALE
  USE_LOCALE_COLLATE USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC USE_PERLIO USE_PERL_ATOF
  USE_REENTRANT_API
  Locally applied patches​:
  DEBPKG​:debian/cpan_definstalldirs - Provide a sensible INSTALLDIRS default for modules installed from CPAN.
  DEBPKG​:debian/db_file_ver - http​://bugs.debian.org/340047 Remove overly restrictive DB_File version check.
  DEBPKG​:debian/doc_info - Replace generic man(1) instructions with Debian-specific information.
  DEBPKG​:debian/enc2xs_inc - http​://bugs.debian.org/290336 Tweak enc2xs to follow symlinks and ignore missing @​INC directories.
  DEBPKG​:debian/errno_ver - http​://bugs.debian.org/343351 Remove Errno version check due to upgrade problems with long-running processes.
  DEBPKG​:debian/libperl_embed_doc - http​://bugs.debian.org/186778 Note that libperl-dev package is required for embedded linking
  DEBPKG​:fixes/respect_umask - Respect umask during installation
  DEBPKG​:debian/writable_site_dirs - Set umask approproately for site install directories
  DEBPKG​:debian/extutils_set_libperl_path - EU​:MM​: set location of libperl.a under /usr/lib
  DEBPKG​:debian/no_packlist_perllocal - Don't install .packlist or perllocal.pod for perl or vendor
  DEBPKG​:debian/prefix_changes - Fiddle with *PREFIX and variables written to the makefile
  DEBPKG​:debian/fakeroot - Postpone LD_LIBRARY_PATH evaluation to the binary targets.
  DEBPKG​:debian/instmodsh_doc - Debian policy doesn't install .packlist files for core or vendor.
  DEBPKG​:debian/ld_run_path - Remove standard libs from LD_RUN_PATH as per Debian policy.
  DEBPKG​:debian/libnet_config_path - Set location of libnet.cfg to /etc/perl/Net as /usr may not be writable.
  DEBPKG​:debian/mod_paths - Tweak @​INC ordering for Debian
  DEBPKG​:debian/module_build_man_extensions - http​://bugs.debian.org/479460 Adjust Module​::Build manual page extensions for the Debian Perl policy
  DEBPKG​:debian/prune_libs - http​://bugs.debian.org/128355 Prune the list of libraries wanted to what we actually need.
  DEBPKG​:fixes/net_smtp_docs - [rt.cpan.org #36038] http​://bugs.debian.org/100195 Document the Net​::SMTP 'Port' option
  DEBPKG​:debian/perlivp - http​://bugs.debian.org/510895 Make perlivp skip include directories in /usr/local
  DEBPKG​:debian/deprecate-with-apt - http​://bugs.debian.org/747628 Point users to Debian packages of deprecated core modules
  DEBPKG​:debian/squelch-locale-warnings - http​://bugs.debian.org/508764 Squelch locale warnings in Debian package maintainer scripts
  DEBPKG​:debian/skip-upstream-git-tests - Skip tests specific to the upstream Git repository
  DEBPKG​:debian/patchlevel - http​://bugs.debian.org/567489 List packaged patches for 5.20.2-2 in patchlevel.h
  DEBPKG​:debian/skip-kfreebsd-crash - http​://bugs.debian.org/628493 [perl #96272] Skip a crashing test case in t/op/threads.t on GNU/kFreeBSD
  DEBPKG​:fixes/document_makemaker_ccflags - http​://bugs.debian.org/628522 [rt.cpan.org #68613] Document that CCFLAGS should include $Config{ccflags}
  DEBPKG​:debian/find_html2text - http​://bugs.debian.org/640479 Configure CPAN​::Distribution with correct name of html2text
  DEBPKG​:debian/perl5db-x-terminal-emulator.patch - http​://bugs.debian.org/668490 Invoke x-terminal-emulator rather than xterm in perl5db.pl
  DEBPKG​:debian/cpan-missing-site-dirs - http​://bugs.debian.org/688842 Fix CPAN​::FirstTime defaults with nonexisting site dirs if a parent is writable
  DEBPKG​:fixes/memoize_storable_nstore - [rt.cpan.org #77790] http​://bugs.debian.org/587650 Memoize​::Storable​: respect 'nstore' option not respected
  DEBPKG​:debian/regen-skip - Skip a regeneration check in unrelated git repositories
  DEBPKG​:fixes/regcomp-mips-optim - [perl #122817] http​://bugs.debian.org/754054 Downgrade the optimization of regcomp.c on mips and mipsel due to a gcc-4.9 bug
  DEBPKG​:debian/makemaker-pasthru - http​://bugs.debian.org/758471 Pass LD settings through to subdirectories
  DEBPKG​:fixes/perldoc-less-R - [rt.cpan.org #98636] http​://bugs.debian.org/758689 Tell the 'less' pager to allow terminal escape sequences
  DEBPKG​:fixes/pod_man_reproducible_date - http​://bugs.debian.org/759405 Support POD_MAN_DATE in Pod​::Man for the left-hand footer
  DEBPKG​:fixes/io_uncompress_gunzip_inmemory - http​://bugs.debian.org/747363 [rt.cpan.org #95494] Fix gunzip to in-memory file handle
  DEBPKG​:fixes/socket_test_recv_fix - http​://bugs.debian.org/758718 [perl #122657] Compare recv return value to peername in socket test
  DEBPKG​:fixes/hurd_socket_recv_todo - http​://bugs.debian.org/758718 [perl #122657] TODO checking the result of recv() on hurd
  DEBPKG​:fixes/regexp-performance - [0fa70a0] http​://bugs.debian.org/777556 [perl #123743] simpify and speed up /.*.../ handling
  Built under linux
  Compiled at Mar 1 2015 22​:06​:11
  @​INC​:
  /etc/perl
  /usr/local/lib/x86_64-linux-gnu/perl/5.20.2
  /usr/local/share/perl/5.20.2
  /usr/lib/x86_64-linux-gnu/perl5/5.20
  /usr/share/perl5
  /usr/lib/x86_64-linux-gnu/perl/5.20
  /usr/share/perl/5.20
  /usr/local/lib/site_perl
  .

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2015

From @jkeenan

On Sat Mar 21 08​:15​:51 2015, vega.james@​gmail.com wrote​:

On Sat, Mar 21, 2015 at 05​:18​:17AM -0700, James E Keenan via RT 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?

Sure. Attached.

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.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2015

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
Editor support available.

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.
BEGIN failed--compilation aborted at /home/jkeenan/learn/perl/p5p/124127-foo.pl line 5.
lock can only be used on shared values at lib/perl5db.pl line 2529.
END failed--call queue aborted at /home/jkeenan/learn/perl/p5p/124127-foo.pl line 5.
Unbalanced scopes​: 3 more ENTERs than LEAVEs

#####

# 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
Editor support available.

Enter h or 'h h' for help, or 'man perldebug' for more help.

main​::(/home/jkeenan/learn/perl/p5p/124127-foo.pl​:12)​:
12​: my $thr = threads->create(\&foo);
  DB<1> n
Hello, world!
Debugged program terminated. Use q to quit or R to restart,
use o inhibit_exit to avoid stopping after program termination,
h q, h R or h o to get additional info.
  DB<1> q

@p5pRT
Copy link
Author

p5pRT commented Mar 25, 2015

From vega.james@gmail.com

On Sat, Mar 21, 2015 at 09​:49​:07AM -0700, James E Keenan via RT wrote​:

On Sat Mar 21 08​:15​:51 2015, vega.james@​gmail.com wrote​:

On Sat, Mar 21, 2015 at 05​:18​:17AM -0700, James E Keenan via RT 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?

Sure. Attached.

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.

Would the fix for these issues be candidates for maintenance releases,
since it was a regression from 5.16 → 5.18?

Cheers,
--
James
GPG Key​: 4096R/331BA3DB 2011-12-05 James McCoy <vega.james@​gmail.com>

@p5pRT
Copy link
Author

p5pRT commented Mar 25, 2015

From @jkeenan

On Tue Mar 24 19​:26​:59 2015, vega.james@​gmail.com wrote​:

On Sat, Mar 21, 2015 at 09​:49​:07AM -0700, James E Keenan via RT wrote​:

On Sat Mar 21 08​:15​:51 2015, vega.james@​gmail.com wrote​:

On Sat, Mar 21, 2015 at 05​:18​:17AM -0700, James E Keenan via RT
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?

Sure. Attached.

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.

Would the fix for these issues be candidates for maintenance releases,
since it was a regression from 5.16 → 5.18?

Cheers,

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.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Mar 29, 2015

From @jkeenan

On Wed Mar 25 03​:27​:08 2015, jkeenan wrote​:

On Tue Mar 24 19​:26​:59 2015, vega.james@​gmail.com wrote​:

On Sat, Mar 21, 2015 at 09​:49​:07AM -0700, James E Keenan via RT
wrote​:

On Sat Mar 21 08​:15​:51 2015, vega.james@​gmail.com wrote​:

On Sat, Mar 21, 2015 at 05​:18​:17AM -0700, James E Keenan via RT
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?

Sure. Attached.

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.

Would the fix for these issues be candidates for maintenance
releases,
since it was a regression from 5.16 → 5.18?

Cheers,

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.

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.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Mar 30, 2015

From vega.james@gmail.com

On Sun, Mar 29, 2015 at 03​:03​:59PM -0700, James E Keenan via RT wrote​:

On Wed Mar 25 03​:27​:08 2015, jkeenan wrote​:

On Tue Mar 24 19​:26​:59 2015, vega.james@​gmail.com wrote​:

On Sat, Mar 21, 2015 at 09​:49​:07AM -0700, James E Keenan via RT
wrote​:

On Sat Mar 21 08​:15​:51 2015, vega.james@​gmail.com wrote​:

On Sat, Mar 21, 2015 at 05​:18​:17AM -0700, James E Keenan via RT
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?

Sure. Attached.

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.

Would the fix for these issues be candidates for maintenance
releases,
since it was a regression from 5.16 → 5.18?

Cheers,

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.

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.

Hmm, I'm not really sure how to debug the debugger. The patches were
just based on bisecting to find where the specific behaviors were
introduced and making logical changes. Any pointers here would be
appreciated.

If it is introduced by my patches and not just uncovered by being able
to debug threaded scripts again, my guess would be that it's due to
0002-lib-perl5db.pl-Fix-read-only-value-modification.patch (or the
commit referenced in that patch).

I'll try to dig into this more over the next few days, regardless.

Cheers,
--
James
GPG Key​: 4096R/331BA3DB 2011-12-05 James McCoy <jamessan@​debian.org>

@p5pRT
Copy link
Author

p5pRT commented Mar 30, 2015

From vega.james@gmail.com

On Sun, Mar 29, 2015 at 10​:52​:00PM -0400, James McCoy 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.

Hmm, I'm not really sure how to debug the debugger. The patches were
just based on bisecting to find where the specific behaviors were
introduced and making logical changes.

Except I was missing some logic. :) Updated patch attached to handle the
fact that $stack_depth isn't always larger than $#stack.

Cheers,
--
James
GPG Key​: 4096R/331BA3DB 2011-12-05 James McCoy <vega.james@​gmail.com>

@p5pRT
Copy link
Author

p5pRT commented Mar 30, 2015

From vega.james@gmail.com

0002-lib-perl5db.pl-Fix-read-only-value-modification.patch
From 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

@p5pRT
Copy link
Author

p5pRT commented Apr 13, 2015

From @ntyni

On Sun, Mar 29, 2015 at 11​:54​:52PM -0400, James McCoy wrote​:

From 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

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

This seems to me like a workaround for a deeper problem. I don't see why
extending the @​stack array by modifying $#stack should create read-only
elements.

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;'
Thread 1 terminated abnormally​: Modification of a read-only value attempted at -e line 1.

which (also) regressed with ce0d59f.

Reproducible with current blead (v5.21.10-71-g2c04452)
with just "Configure -des -Dusethreads -Dusedevel".
--
Niko Tyni ntyni@​debian.org

@p5pRT
Copy link
Author

p5pRT commented Apr 14, 2015

From @tonycoz

On Mon Apr 13 11​:47​:06 2015, ntyni@​debian.org wrote​:

On Sun, Mar 29, 2015 at 11​:54​:52PM -0400, James McCoy wrote​:

From 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

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

This seems to me like a workaround for a deeper problem. I don't see
why
extending the @​stack array by modifying $#stack should create read-
only
elements.

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;'
Thread 1 terminated abnormally​: Modification of a read-only value
attempted at -e line 1.

which (also) regressed with ce0d59f.

Reproducible with current blead (v5.21.10-71-g2c04452)
with just "Configure -des -Dusethreads -Dusedevel".

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
Editor support available.

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.
BEGIN failed--compilation aborted at ../123127.pl line 5.
lock can only be used on shared values at lib/perl5db.pl line 2529.
END failed--call queue aborted at ../123127.pl line 5.
perl​: perl.c​:584​: perl_destruct​: Assertion `(my_perl->Iscopestack_ix) == 0' failed.
Aborted

(-DDEBUGGING build)

Tony

@p5pRT
Copy link
Author

p5pRT commented Apr 14, 2015

From @tonycoz

0001-perl-124127-fix-cloning-arrays-with-unused-elements.patch
From 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

@p5pRT
Copy link
Author

p5pRT commented Apr 14, 2015

From @ntyni

On Mon, Apr 13, 2015 at 11​:06​:21PM -0700, Tony Cook via RT wrote​:

On Mon Apr 13 11​:47​:06 2015, ntyni@​debian.org wrote​:

% perl -Mthreads -le 'my @​a = ("foo"); threads->create(sub { $#a=1;
$a[1]="bar"; print @​a})->join;'
Thread 1 terminated abnormally​: Modification of a read-only value
attempted at -e line 1.

which (also) regressed with ce0d59f.

Reproducible with current blead (v5.21.10-71-g2c04452)
with just "Configure -des -Dusethreads -Dusedevel".

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
Editor support available.

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.

Awesome, thanks! The original patch 1/2 in this ticket by James McCoy
fixes the other problem by reintroducing the lock prototype removed
in v5.17.5-409-gcde405a.

So the 'perl -d threaded-script.pl' breakage was a 5.17.6 regression and the
'Modification of a read-only value' issue just broke it further in 5.19.4.

I can confirm that applying your patch and
https://rt-archive.perl.org/perl5/Ticket/Attachment/1336725/714399/0001-lib-perl5db.pl-Restore-noop-lock-prototype.patch
make the debugger work again for me, at least on a trivial threaded script.

Hope this clarifies
--
Niko Tyni ntyni@​debian.org

@p5pRT
Copy link
Author

p5pRT commented Apr 15, 2015

From @tonycoz

On Tue Apr 14 11​:42​:47 2015, ntyni@​debian.org wrote​:

On Mon, Apr 13, 2015 at 11​:06​:21PM -0700, Tony Cook via RT wrote​:

On Mon Apr 13 11​:47​:06 2015, ntyni@​debian.org wrote​:

% perl -Mthreads -le 'my @​a = ("foo"); threads->create(sub {
$#a=1;
$a[1]="bar"; print @​a})->join;'
Thread 1 terminated abnormally​: Modification of a read-only value
attempted at -e line 1.

which (also) regressed with
ce0d59f.

Reproducible with current blead (v5.21.10-71-g2c04452)
with just "Configure -des -Dusethreads -Dusedevel".

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
Editor support available.

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.

Awesome, thanks! The original patch 1/2 in this ticket by James McCoy
fixes the other problem by reintroducing the lock prototype removed
in v5.17.5-409-gcde405a.

So the 'perl -d threaded-script.pl' breakage was a 5.17.6 regression
and the
'Modification of a read-only value' issue just broke it further in
5.19.4.

I can confirm that applying your patch and
https://rt-archive.perl.org/perl5/Ticket/Attachment/1336725/714399/0001-lib-
perl5db.pl-Restore-noop-lock-prototype.patch
make the debugger work again for me, at least on a trivial threaded
script.

Hope this clarifies

You're right.

This fixes the default case, unfortunately PERL5DB_THREADED is also broken, I'll open a new ticket for that.

Tony

@p5pRT
Copy link
Author

p5pRT commented Apr 15, 2015

From @tonycoz

On Tue Apr 14 17​:23​:55 2015, tonyc wrote​:

I can confirm that applying your patch and
https://rt-archive.perl.org/perl5/Ticket/Attachment/1336725/714399/0001-lib-
perl5db.pl-Restore-noop-lock-prototype.patch
make the debugger work again for me, at least on a trivial threaded
script.

Hope this clarifies

You're right.

This fixes the default case, ...

These aren't regressions from 5.20, but I think they're serious enough to be
blockers, so I'll make this ticket a 5.22 blocker.

Opinions?

Tony

@p5pRT
Copy link
Author

p5pRT commented Apr 16, 2015

From @tonycoz

On Tue Apr 14 18​:14​:16 2015, tonyc wrote​:

On Tue Apr 14 17​:23​:55 2015, tonyc wrote​:

I can confirm that applying your patch and
https://rt-archive.perl.org/perl5/Ticket/Attachment/1336725/714399/0001-lib-
perl5db.pl-Restore-noop-lock-prototype.patch
make the debugger work again for me, at least on a trivial threaded
script.

Hope this clarifies

You're right.

This fixes the default case, ...

These aren't regressions from 5.20, but I think they're serious enough to be
blockers, so I'll make this ticket a 5.22 blocker.

Applied as 41ef2c6 and 902d169.

Closing

Tony

@p5pRT
Copy link
Author

p5pRT commented Apr 16, 2015

@tonycoz - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant