Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 559 forks source link

Format wrongly recognizes argument line in braces as hash #13306

Closed p5pRT closed 9 years ago

p5pRT commented 11 years ago

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

Searchable as RT119973$

p5pRT commented 11 years ago

From springl-perlbug@bfw-online.de

This is a bug report for perl from Stephan springl \springl\-perlbug@​bfw\-online\.de\, generated with the help of perlbug 1.39 running under perl 5.18.1.

The program


#!/usr/bin/perl format STDOUT = @​\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\< @​### {'hi'\, 1} . write;


will write hi 1 to stdout using perl version upto 5.16. Perl version 5.18 will give HASH(0x82e80d8) 0

The change was introduced by commit 705fe0e5f8a324e10c292190237cac35c5af4109\, "Don´t let format arguments `leak out´ of formline". It seems to be a problem in the lexer because replacing "{'hi\, 1}" by the two lines "{'hi'\n\, 1}" produces the desired result. Please note that the comma may not end the first line. Tracing the lexer by starting perl with -D257 gives the key change​: -Reading a token​: Next token is token HASHBRACK (0x0) -Shifting token HASHBRACK\, Entering state 45 +Reading a token​: Next token is token '{' (0x4) +Shifting token '{'\, Entering state 74 +Reducing stack by rule 15 (line 226)\, -> remember +Entering state 191 +Reducing stack by rule 18 (line 244)\, -> stmtseq +Entering state 280

Obviously\, the lexer tries to figure out whether to return a HASHBRACK or '{' token and the decision is influenced by the presence of a comma in the first argument line.



Flags​:   category=core   severity=medium


Site configuration information for perl 5.18.1​:

Configured by Debian Project at Sun Sep 22 14​:25​:59 MSZ 2013.

Summary of my perl5 (revision 5 version 18 subversion 1) configuration​:   Commit id​: ea3edeabc96995d8bfd8ed1f8c267d2d5565303c   Platform​:   osname=linux\, osvers=3.10.12-64+\, archname=i486-linux-gnu-thread-multi-64int   uname='linux lar 3.10.12-64+ #1 smp preempt mon sep 16 07​:40​:15 msz 2013 i686 gnulinux '   config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Dldflags= -Wl\,-z\,relro -Dlddlflags=-shared -Wl\,-z\,relro -Dcccdlflags=-fPIC -Darchname=i486-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.18 -Darchlib=/usr/lib/perl/5.18 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.18.1 -Dsitearch=/usr/local/lib/perl/5.18.1 -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.18.1 -des'   hint=recommended\, useposix=true\, d_sigaction=define   useithreads=define\, usemultiplicity=define   useperlio=define\, d_sfio=undef\, uselargefiles=define\, usesocks=undef   use64bitint=define\, use64bitall=undef\, uselongdouble=undef   usemymalloc=n\, bincompat5005=undef   Compiler​:   cc='cc'\, ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fstack-protector -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 -fstack-protector -fno-strict-aliasing -pipe -I/usr/local/include'   ccversion=''\, gccversion='4.7.3'\, gccosandvers=''   intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=12345678   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=12   ivtype='long long'\, ivsize=8\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8   alignbytes=4\, prototype=define   Linker and Libraries​:   ld='cc'\, ldflags =' -fstack-protector -L/usr/local/lib'   libpth=/usr/local/lib /lib/i386-linux-gnu /lib/../lib /usr/lib/i386-linux-gnu /usr/lib/../lib /lib /usr/lib /lib64 /usr/lib64   libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt   perllibs=-ldl -lm -lpthread -lc -lcrypt   libc=\, so=so\, useshrplib=true\, libperl=libperl.so.5.18.1   gnulibc_version='2.17'   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-Wl\,-E'   cccdlflags='-fPIC'\, lddlflags='-shared -L/usr/local/lib -fstack-protector'

Locally applied patches​:   uncommitted-changes   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 to /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/cpanplus_definstalldirs - http​://bugs.debian.org/533707 Configure CPANPLUS to use the site directories by default.   DEBPKG​:debian/cpanplus_config_path - Save local versions of CPANPLUS​::Config​::System into /etc/perl.   DEBPKG​:debian/deprecate-with-apt - http​://bugs.debian.org/702096 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.18.1-4bfw1 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/hurd_test_todo_syslog - http​://bugs.debian.org/650093 Disable failing GNU/Hurd tests in cpan/Sys-Syslog/t/syslog.t   DEBPKG​:debian/hurd_test_skip_sigdispatch - http​://bugs.debian.org/650188 Disable failing GNU/Hurd tests op/sigdispatch.t   DEBPKG​:debian/hurd_test_skip_stack - http​://bugs.debian.org/650175 Disable failing GNU/Hurd tests dist/threads/t/stack.t   DEBPKG​:debian/hurd_test_skip_pipe - http​://bugs.debian.org/650187 Disable failing GNU/Hurd tests io/pipe.t   DEBPKG​:debian/hurd_test_skip_io_pipe - http​://bugs.debian.org/650096 Disable failing GNU/Hurd tests dist/IO/t/io_pipe.t   DEBPKG​:fixes/manpage_name_Test-Harness - http​://bugs.debian.org/650451 [rt.cpan.org #73399] cpan/Test-Harness​: add NAME headings in modules with POD   DEBPKG​:debian/makemaker-pasthru - http​://bugs.debian.org/660195 [rt.cpan.org #28632] Make EU​::MM pass LD through to recursive Makefile.PL invocations   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​:debian/hurd_net_ping_disable_test - http​://bugs.debian.org/709385 Disable failing Net-Ping tests for GNU/Hurd   DEBPKG​:fixes/memoize_storable_nstore - [rt.cpan.org #77790] http​://bugs.debian.org/587650 Memoize​::Storable​: respect 'nstore' option not respected   DEBPKG​:fixes/net_ftp_failed_command - [rt.cpan.org #37700] http​://bugs.debian.org/491062 Net​::FTP​: cope gracefully with a failed command   DEBPKG​:fixes/perlbug-patchlist - [3541c11] http​://bugs.debian.org/710842 [perl #118433] Make perlbug look up the list of local patches at run time   DEBPKG​:fixes/regexp-preserve - http​://bugs.debian.org/718209 [perl #118213] [f4194b2] RT #118213​: handle $r=qr/.../; /$r/p properly   DEBPKG​:fixes/regexp-preserve-testcases - http​://bugs.debian.org/718209 [perl #118213] [4d7b2f5] Disable new //p tests   DEBPKG​:fixes/module_metadata_security_doc - [68cdd4b] CVE-2013-1437 documentation fix   DEBPKG​:fixes/module_metadata_taint_fix - [bff978f] http​://bugs.debian.org/722210 [rt.cpan.org #88576] untaint version\, if needed\, in Module​::Metadata


@​INC for perl 5.18.1​:   /etc/perl   /usr/local/lib/perl/5.18.1   /usr/local/share/perl/5.18.1   /usr/lib/perl5   /usr/share/perl5   /usr/lib/perl/5.18   /usr/share/perl/5.18   /usr/local/lib/site_perl   .


Environment for perl 5.18.1​:   HOME=/home/springl   LANG (unset)   LANGUAGE (unset)   LC_CTYPE=de_DE   LD_LIBRARY_PATH (unset)   LOGDIR (unset)   PATH=/home/springl/bin​:/home/springl/bin​:/home/springl/local/bin​:.​:/usr/local/bin​:/usr/bin​:/bin​:/usr/local/games​:/usr/games​:/bfw/bin​:/usr/ASQL/bin​:/usr/ASQL/diag​:/home/src/appl/verwaltung/bin   PERL_BADLANG (unset)   SHELL=/bin/bash

p5pRT commented 10 years ago

From @cpansprout

On Tue Sep 24 04​:57​:09 2013\, springl-perlbug@​bfw-online.de wrote​:

This is a bug report for perl from Stephan springl \springl\-perlbug@&#8203;bfw\-online\.de\, generated with the help of perlbug 1.39 running under perl 5.18.1.

The program

---------------------------------------- #!/usr/bin/perl format STDOUT = @​\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\< @​### {'hi'\, 1} . write; ----------------------------------------

will write hi 1 to stdout using perl version upto 5.16. Perl version 5.18 will give HASH(0x82e80d8) 0

The change was introduced by commit 705fe0e5f8a324e10c292190237cac35c5af4109\, "Don´t let format arguments `leak out´ of formline". It seems to be a problem in the lexer because replacing "{'hi\, 1}" by the two lines "{'hi'\n\, 1}" produces the desired result. Please note that the comma may not end the first line. Tracing the lexer by starting perl with -D257 gives the key change​: -Reading a token​: Next token is token HASHBRACK (0x0) -Shifting token HASHBRACK\, Entering state 45 +Reading a token​: Next token is token '{' (0x4) +Shifting token '{'\, Entering state 74 +Reducing stack by rule 15 (line 226)\, -> remember +Entering state 191 +Reducing stack by rule 18 (line 244)\, -> stmtseq +Entering state 280

Obviously\, the lexer tries to figure out whether to return a HASHBRACK or '{' token and the decision is influenced by the presence of a comma in the first argument line.

Sorry for the long delay in responding.

Yes\, this does appear to be a problem. That commit was only intended to fix clearly buggy parsing (including the fact that unambiguous anonymous hash constructors were being treated as blocks). It did not occur to me at the time that it would have this effect.

I think the least controversial solution would be to treat { as the beginning of a block when it is the first token in a format argument line (this could be backported to 5.18.3 as well)\, but I want to see what other perl developers think.

--

Father Chrysostomos

p5pRT commented 10 years ago

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

p5pRT commented 10 years ago

From @tux

On Sat\, 18 Jan 2014 06​:05​:12 -0800\, "Father Chrysostomos via RT" \perlbug\-followup@&#8203;perl\.org wrote​:

On Tue Sep 24 04​:57​:09 2013\, springl-perlbug@​bfw-online.de wrote​:

This is a bug report for perl from Stephan springl \springl\-perlbug@&#8203;bfw\-online\.de\, generated with the help of perlbug 1.39 running under perl 5.18.1.

The program

---------------------------------------- #!/usr/bin/perl format STDOUT = @​\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\<\< @​### {'hi'\, 1} . write; ----------------------------------------

will write hi 1 to stdout using perl version upto 5.16. Perl version 5.18 will give HASH(0x82e80d8) 0

The change was introduced by commit 705fe0e5f8a324e10c292190237cac35c5af4109\, "Don´t let format arguments `leak out´ of formline". It seems to be a problem in the lexer because replacing "{'hi\, 1}" by the two lines "{'hi'\n\, 1}" produces the desired result. Please note that the comma may not end the first line. Tracing the lexer by starting perl with -D257 gives the key change​: -Reading a token​: Next token is token HASHBRACK (0x0) -Shifting token HASHBRACK\, Entering state 45 +Reading a token​: Next token is token '{' (0x4) +Shifting token '{'\, Entering state 74 +Reducing stack by rule 15 (line 226)\, -> remember +Entering state 191 +Reducing stack by rule 18 (line 244)\, -> stmtseq +Entering state 280

Obviously\, the lexer tries to figure out whether to return a HASHBRACK or '{' token and the decision is influenced by the presence of a comma in the first argument line.

Sorry for the long delay in responding.

Yes\, this does appear to be a problem. That commit was only intended to fix clearly buggy parsing (including the fact that unambiguous anonymous hash constructors were being treated as blocks). It did not occur to me at the time that it would have this effect.

I think the least controversial solution would be to treat { as the beginning of a block when it is the first token in a format argument line (this could be backported to 5.18.3 as well)\, but I want to see what other perl developers think.

that is what it is documented to be I use it in production code\, but have not been able to test that against 5.18. I am very glad someone found it before I did

The minimum action here is to add tests to t/write.t to guarantee it won't happen again

-- H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/ using perl5.00307 .. 5.19 porting perl5 on HP-UX\, AIX\, and openSUSE http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/ http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

p5pRT commented 10 years ago

From @cpansprout

I thought I had replied\, but my reply has not shown up.... So I have to write it again. :-(

On Sat Jan 18 08​:11​:50 2014\, hmbrand wrote​:

On Sat\, 18 Jan 2014 06​:05​:12 -0800\, "Father Chrysostomos via RT" \perlbug\-followup@&#8203;perl\.org wrote​:

I think the least controversial solution would be to treat { as the beginning of a block when it is the first token in a format argument line (this could be backported to 5.18.3 as well)\, but I want to see what other perl developers think.

that is what it is documented to be

Yes\, indeed. I had not noticed.

I use it in production code\, but have not been able to test that against 5.18. I am very glad someone found it before I did

The minimum action here is to add tests to t/write.t to guarantee it won't happen again

I have fixed this in f60e676307b. Does it have your vote for backporting to 5.18?

--

Father Chrysostomos

p5pRT commented 10 years ago

From @tux

On Sun\, 19 Jan 2014 06​:23​:00 -0800\, "Father Chrysostomos via RT" \perlbug\-followup@&#8203;perl\.org wrote​:

I thought I had replied\, but my reply has not shown up.... So I have to write it again. :-(

On Sat Jan 18 08​:11​:50 2014\, hmbrand wrote​:

On Sat\, 18 Jan 2014 06​:05​:12 -0800\, "Father Chrysostomos via RT" \perlbug\-followup@&#8203;perl\.org wrote​:

I think the least controversial solution would be to treat { as the beginning of a block when it is the first token in a format argument line (this could be backported to 5.18.3 as well)\, but I want to see what other perl developers think.

that is what it is documented to be

Yes\, indeed. I had not noticed.

I use it in production code\, but have not been able to test that against 5.18. I am very glad someone found it before I did

The minimum action here is to add tests to t/write.t to guarantee it won't happen again

I have fixed this in f60e676307b. Does it have your vote for backporting to 5.18?

Yes

-- H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/ using perl5.00307 .. 5.19 porting perl5 on HP-UX\, AIX\, and openSUSE http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/ http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

p5pRT commented 10 years ago

From @tonycoz

On Sun Jan 19 06​:23​:00 2014\, sprout wrote​:

I have fixed this in f60e676307b. Does it have your vote for backporting to 5.18?

It has mine.

Tony

p5pRT commented 10 years ago

From @khwilliamson

I'm marking this as stalled\, to be resolved when we put it into a 5.18 maint release\, or decide not to make such a release. It has been fixed in blead -- Karl Williamson

p5pRT commented 10 years ago

@khwilliamson - Status changed from 'open' to 'stalled'

p5pRT commented 9 years ago

From @cpansprout

The fix was backported to 5.18.4 in commit c38e89e4398a.

--

Father Chrysostomos

p5pRT commented 9 years ago

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

p5pRT commented 9 years ago

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