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

Wrong assignment in nested assignment together with subroutine ca ll #6932

Closed
p5pRT opened this issue Nov 17, 2003 · 9 comments
Closed

Wrong assignment in nested assignment together with subroutine ca ll #6932

p5pRT opened this issue Nov 17, 2003 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented Nov 17, 2003

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

Searchable as RT24508$

@p5pRT
Copy link
Author

p5pRT commented Nov 17, 2003

From wolf-dietrich.moeller@siemens.com

Wrong assignment in nested assignment together with subroutine call.

Given one assignment with a concatenation on the right hand side.
The first term of this concatenation is an assignment of a concatenation,
enclosed in parentheses.

If the inner assignment concatenates two constant strings or scalar
variables, everything is okay. Example statement​:
$x2 = ($x1 = '##'.'**').'++';

If the inner assignment assigns the concatenation of a string and the result
of a subroutine call, then the parentheses are not honored correctly.
Example statement​:
$x2 = ($x1 = '##'.fct('**')).'++';
The "inner" result variable $x1 has (incorrectly) the value of the "outer"
result varable ($x2).

If a "dummy" empty string is concatenated at the beginning of the outer
concatenation, then the behaviour is correct. Example statement​:
$x2 = ''.($x1 = '##'.fct '**' ).'++';

Working test examples, sample output, and Perl version info see test program
below.

######################### start test program #######################
#!/usr/local/bin/perl
# test-script to show error
# Wolf-Dietrich Moeller, 2003-11-14,
<mailto​:wolf-dietrich.moeller@​siemens.com>
# tested on Perl 5.8.0_806 and 5.8.1_807 Win32 ActiveState,
# also on Perl 5.6.1 under Apache webserver and freeBSD (source
distribution)
# output is (for command line and CGI-script)​:
#######################################################
#1 x1=##** x2=##**++
#2 x1=##**++ x2=##**++
#3 x1=##**++ x2=##**++
#4 x1=##** x2=##**++
#5 x1=##** x2=##**++
# error in lines 2 and 3​: x1 should always equal '##**'
#######################################################
use strict;
binmode STDOUT;
sub fct ($) {
my $c = shift;
return $c
}
my ($x1,$x2);
print "Content-Type​: text/plain\x0D\x0A\x0D\x0A";
#
# compound statement without sub
$x2 = ($x1 = '##'.'**').'++';
print '#1 x1=',$x1,' x2=',$x2,"\x0D\x0A";
#
# compound statement with sub
$x2 = ($x1 = '##'.fct '**' ).'++';
print '#2 x1=',$x1,' x2=',$x2,"\x0D\x0A";
#
# compound statement with sub (parentheses for parameters added)
$x2 = ($x1 = '##'.fct('**')).'++';
print '#3 x1=',$x1,' x2=',$x2,"\x0D\x0A";
#
# (empty) concatenation in front of inner assignment added
$x2 = ''.($x1 = '##'.fct '**' ).'++';
print '#4 x1=',$x1,' x2=',$x2,"\x0D\x0A";
#
# two simple statements
$x1 = '##'.fct('**');
$x2 = $x1.'++';
print '#5 x1=',$x1,' x2=',$x2,"\x0D\x0A";
#
print "# error in lines 2 and 3​: x1 should always equal '##**'\x0D\x0A";
###################### end test program #############################


Dr. Wolf-Dietrich Moeller
Siemens AG, CT IC 3, D-81730 München
Corporate Technology Department Security
Mch P, Tel. +49 89 636-53391, Fax -48000
mailto​:wolf-dietrich.moeller@​siemens.com
Intranet https://security.ct.siemens.de/

@p5pRT
Copy link
Author

p5pRT commented Nov 18, 2003

From japhy@pobox.com

On Nov 17, Moeller Wolf-Dietrich said​:

Wrong assignment in nested assignment together with subroutine call.

Given one assignment with a concatenation on the right hand side.
The first term of this concatenation is an assignment of a concatenation,
enclosed in parentheses.

This only happens with lexicals.

  my ($A, $B);
  $B = ($A = '' . foo()) . "y";
  $D = ($C = '' . foo()) . "y";
  print "$A,$B\n"; # xy,xy
  print "$C,$D\n"; # x,xy

  sub foo { "x" }

--
Jeff "japhy" Pinyan japhy@​pobox.com http​://www.pobox.com/~japhy/
RPI Acacia brother #734 http​://www.perlmonks.org/ http​://www.cpan.org/
<stu> what does y/// stand for? <tenderpuss> why, yansliterate of course.
[ I'm looking for programming work. If you like my work, let me know. ]

@p5pRT
Copy link
Author

p5pRT commented Nov 18, 2003

From @rgarcia

Jeff 'japhy' Pinyan wrote​:

This only happens with lexicals.

my ($A, $B);
$B = ($A = '' . foo()) . "y";
$D = ($C = '' . foo()) . "y";
print "$A,$B\n"; # xy,xy
print "$C,$D\n"; # x,xy

sub foo { "x" }

Ouch.
I can reproduce it with 5.6.1, but not with 5.5.3.
Looks like an optree optimization problem.
Andreas, could you try this below with binsearchaperl ?

#!perl
sub foo { "x" }
my ($A, $B);
$B = ($A = '' . foo()) . "y";
print "$A,$B" eq "x,xy" ? "ok\n" : "not ok\n";
__END__

@p5pRT
Copy link
Author

p5pRT commented Nov 18, 2003

From @iabyn

On Tue, Nov 18, 2003 at 01​:55​:41PM +0100, Rafael Garcia-Suarez wrote​:

Jeff 'japhy' Pinyan wrote​:

This only happens with lexicals.

my ($A, $B);
$B = ($A = '' . foo()) . "y";
$D = ($C = '' . foo()) . "y";
print "$A,$B\n"; # xy,xy
print "$C,$D\n"; # x,xy

sub foo { "x" }

Ouch.
I can reproduce it with 5.6.1, but not with 5.5.3.
Looks like an optree optimization problem.

Both the inner and outer OP_CONCATs have the TEMP target optimised away;
the inner one uses the lexical as the target instead, and the outer one is
STACKED, so it uses the previous result as the target (which happens to be
the lexical).

Andreas, could you try this below with binsearchaperl ?

#!perl
sub foo { "x" }
my ($A, $B);
$B = ($A = '' . foo()) . "y";
print "$A,$B" eq "x,xy" ? "ok\n" : "not ok\n";
__END__

Looks like 3612 judging by these excerpts from the log​:

____________________________________________________________________________

[ 4749] By​: gsar on 2000/01/02 21​:37​:29
  Log​: disable optimization in change#3612 for join() and quotemeta()--this
  removes all the gross hacks for the special cases in that change; fix
  pp_concat() for when TARG == arg (modified version of patch suggested
  by Ilya Zakharevich)
  Branch​: perl
  ! op.c opcode.h opcode.pl pp_hot.c sv.c t/op/lex_assign.t
____________________________________________________________________________
[ 4543] By​: gsar on 1999/11/10 01​:52​:22
  Log​: remove dead branch/infinite looper in change#3612
  Branch​: perl
  ! op.c
____________________________________________________________________________
[ 4415] By​: gsar on 1999/10/20 00​:52​:34
  Log​: disable optimizing troublesome ops in change#3612
  (from Ilya Zakharevich)
  Branch​: perl
  ! Makefile.SH opcode.h opcode.pl
____________________________________________________________________________
[ 4087] By​: gsar on 1999/09/06 18​:06​:06
  Log​: change#3612 is buggy when quotemeta argument matches target
  (hope this is the last of the optimized-OP_SASSIGN bugs)
  From​: Ilya Zakharevich <ilya@​math.ohio-state.edu>
  Date​: Sun, 5 Sep 1999 06​:07​:42 -0400 (EDT)
  Message-Id​: <199909051007.GAA06423@​monk.mps.ohio-state.edu>
  Subject​: Re​: [BUG​: quotemeta]
  Branch​: perl
  ! Changes op.c t/op/lex_assign.t
____________________________________________________________________________
[ 3854] By​: gsar on 1999/08/01 20​:41​:53
  Log​: fixes from Stephen McCamant that address bugs in change#3612
  (the optimization shouldn't be enabled in expressions where
  the variable is introduced), and fix Deparse to grok the
  optimization
  Date​: Thu, 29 Jul 1999 21​:21​:49 -0500 (CDT)
  Message-ID​: <14241.3133.979257.953396@​alias-2.pr.mcs.net>
  Subject​: [PATCH _58] Set OPpTARGET_MY more consistently
  --
  Date​: Thu, 29 Jul 1999 22​:31​:16 -0500 (CDT)
  Message-ID​: <14241.7300.181386.763503@​alias-2.pr.mcs.net>
  Subject​: [PATCH _58] Disable TARGET_MY-ization on variable introduction
  --
  Date​: Fri, 30 Jul 1999 22​:25​:27 -0500 (CDT)
  Message-Id​: <199907310326.VAA24376@​localhost.frii.com>
  Subject​: [PATCH _58, long] B​::Deparse (was Re​: New warning 'Useless use of...')
  Branch​: perl
  ! ext/B/B/Deparse.pm op.c
____________________________________________________________________________
[ 3664] By​: gsar on 1999/07/11 19​:11​:07
  Log​: change#3612 was buggy and failed to build Tk; applied Ilya's
  remedy and related tests via private mail
  Branch​: perl
  ! op.c t/op/lex_assign.t
____________________________________________________________________________
[ 3612] By​: gsar on 1999/07/06 10​:17​:52
  Log​: From​: Ilya Zakharevich <ilya@​math.ohio-state.edu>
  Date​: Sat, 12 Jun 1999 04​:49​:09 -0400 (EDT)
  Message-Id​: <199906120849.EAA26986@​monk.mps.ohio-state.edu>
  Subject​: [PATCH 5.005_57] Optimize away OP_SASSIGN
  Branch​: perl
  ! op.c op.h opcode.h opcode.pl pp.h pp.sym pp_proto.h

--
A walk of a thousand miles begins with a single step...
then continues for another 1,999,999 or so.

@p5pRT
Copy link
Author

p5pRT commented Nov 19, 2003

From @andk

On Tue, 18 Nov 2003 13​:20​:24 +0000, Dave Mitchell <davem@​fdgroup.com> said​:

Andreas, could you try this below with binsearchaperl ?

#!perl
sub foo { "x" }
my ($A, $B);
$B = ($A = '' . foo()) . "y";
print "$A,$B" eq "x,xy" ? "ok\n" : "not ok\n";
__END__

  > Looks like 3612 judging by these excerpts from the log​:

Confirmed.

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Nov 19, 2003

From @rgs

Dave Mitchell wrote​:

my ($A, $B);
$B = ($A = '' . foo()) . "y";
$D = ($C = '' . foo()) . "y";
print "$A,$B\n"; # xy,xy
print "$C,$D\n"; # x,xy

sub foo { "x" }

Ouch.
I can reproduce it with 5.6.1, but not with 5.5.3.
Looks like an optree optimization problem.

Both the inner and outer OP_CONCATs have the TEMP target optimised away;
the inner one uses the lexical as the target instead, and the outer one is
STACKED, so it uses the previous result as the target (which happens to be
the lexical).

I'm tempted to apply the fix below. (Look closer.)
Thoughts ?

Index​: opcode.pl

--- opcode.pl (revision 2824)
+++ opcode.pl (working copy)
@​@​ -537,7 +537,7 @​@​
i_add integer addition (+) ck_null ifsT2 S S
subtract subtraction (-) ck_null IfsT2 S S
i_subtract integer subtraction (-) ck_null ifsT2 S S
-concat concatenation (.) or string ck_concat fsT2 S S
+concat concatenation (.) or string ck_concat fst2 S S
stringify string ck_fun fsT@​ S

left_shift left bitshift (<<) ck_bitop fsT2 S S
End of fix.

@p5pRT
Copy link
Author

p5pRT commented Nov 19, 2003

From @rgs

I warned​:

I'm tempted to apply the fix below. (Look closer.)
Thoughts ?

Temptation applied as :
Change 21752 on 2003/11/19 by rgs@​rgs-home

  Fix bug [perl #24508] Wrong assignment in nested assignment
  together with subroutine call
  Apparently concat still doesn't deal correctly with lexicals
  in all cases. Disable the whole TARGET_MY optimisation for it.
  (and remove the corresponding code from the peephole optimiser.)

It feels safer than adding another piece on the jenga tower.
(I see that similar fixes were already applied for other ops.)

@p5pRT
Copy link
Author

p5pRT commented Nov 23, 2003

@rgs - Status changed from 'new' to 'resolved'

@p5pRT p5pRT closed this as completed Nov 23, 2003
@p5pRT
Copy link
Author

p5pRT commented Dec 31, 2003

From @iabyn

A further note​:

Patch #21752 retracted the assign/concat optimisation, which
contained the bug.
Patch #21951 by adi, re-added the optimisation (ie retracted #21752),
plus added a proper fix for the bug.

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

No branches or pull requests

1 participant