Skip to content

Commit

Permalink
fix 'for reverse @array' bug on AIX
Browse files Browse the repository at this point in the history
RT #133558

Due to what appears to be a compiler bug on AIX (or perhaps it's
undefined behaviour which happens to work on other platforms), this line
of code in pp_iter():

    inc = 1 - (PL_op->op_private & OPpITER_REVERSED);

was setting inc to 4294967295 rather than to the expected -1 (inc was a
64-bit signed long).

Fix it with a couple of judicious (IV) casts (which ought to be a NOOP).
  • Loading branch information
iabyn committed Oct 17, 2018
1 parent 747c94e commit d6139ec
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
4 changes: 2 additions & 2 deletions pp_hot.c
Expand Up @@ -3932,7 +3932,7 @@ PP(pp_iter)
case CXt_LOOP_LIST: /* for (1,2,3) */

assert(OPpITER_REVERSED == 2); /* so inc becomes -1 or 1 */
inc = 1 - (PL_op->op_private & OPpITER_REVERSED);
inc = (IV)1 - (IV)(PL_op->op_private & OPpITER_REVERSED);
ix = (cx->blk_loop.state_u.stack.ix += inc);
if (UNLIKELY(inc > 0
? ix > cx->blk_oldsp
Expand All @@ -3947,7 +3947,7 @@ PP(pp_iter)
case CXt_LOOP_ARY: /* for (@ary) */

av = cx->blk_loop.state_u.ary.ary;
inc = 1 - (PL_op->op_private & OPpITER_REVERSED);
inc = (IV)1 - (IV)(PL_op->op_private & OPpITER_REVERSED);
ix = (cx->blk_loop.state_u.ary.ix += inc);
if (UNLIKELY(inc > 0
? ix > AvFILL(av)
Expand Down
16 changes: 15 additions & 1 deletion t/op/for.t
Expand Up @@ -5,7 +5,7 @@ BEGIN {
require "./test.pl";
}

plan(124);
plan(126);

# A lot of tests to check that reversed for works.

Expand Down Expand Up @@ -664,3 +664,17 @@ is(fscope(), 1, 'return via loop in sub');
}
is($foo, "outside", "RT #123994 array outside");
}

# RT #133558 'reverse' under AIX was causing loop to terminate
# immediately, probably due to compiler bug

{
my @a = qw(foo);
my @b;
push @b, $_ for (reverse @a);
is "@b", "foo", " RT #133558 reverse array";

@b = ();
push @b, $_ for (reverse 'bar');
is "@b", "bar", " RT #133558 reverse list";
}

0 comments on commit d6139ec

Please sign in to comment.