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

Array names starting with 'Inf' can be poorly behaved. #16335

Closed
p5pRT opened this issue Dec 22, 2017 · 11 comments

Comments

@p5pRT
Copy link

commented Dec 22, 2017

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

Searchable as RT132645$

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 22, 2017

From @khwilliamson

I am posting this for Dan Book, who is having trouble getting email to
perlbug through.

The build options and version of perl is immaterial here, except since 5.22.

When the warning "Scalar value @​arrayname[0] better written as
$arrayname[0]"
is triggered (any time an array slice is used with only one element),
this results
in an error if the array's name starts with 'inf'. Code to reproduce​:

use warnings;
my @​infasdf;
my @​x = @​infasdf[0];
1

Error from above code (since 5.22)​: Cannot printf Inf with 'c' at (IRC)
line 3.

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 22, 2017

From @cpansprout

On Fri, 22 Dec 2017 09​:41​:56 -0800, public@​khwilliamson.com wrote​:

I am posting this for Dan Book, who is having trouble getting email to
perlbug through.

The build options and version of perl is immaterial here, except since 5.22.

When the warning "Scalar value @​arrayname[0] better written as
$arrayname[0]"
is triggered (any time an array slice is used with only one element),
this results
in an error if the array's name starts with 'inf'. Code to reproduce​:

use warnings;
my @​infasdf;
my @​x = @​infasdf[0];
1

Error from above code (since 5.22)​: Cannot printf Inf with 'c' at (IRC)
line 3.

$ ../perl.git-copy/Porting/bisect.pl --target=miniperl --start=v5.20.0 --end=v5.22.0 -- ./miniperl -Ilib /tmp/foo
...
3396ed3 is the first bad commit
commit 3396ed3
Author​: Jarkko Hietaniemi <jhi@​iki.fi>
Date​: Sun Jan 25 12​:27​:44 2015 -0500

  infnan​: Simplify inf parsing.
 
  Accept anything beginning with /^inf/i,
  but warn if there's trailing stuff.

:100644 100644 66b08834448704597e416e7f3a024b8329245bd0 7819c60d94adac3f7b96b11ed816cc12d57b1a27 M numeric.c
:040000 040000 85a647618418ee945da09230ba2a172f26331d0c 5a472ae9721446be7d90ffdfe00bfb555e1c3d07 M t
bisect run success
That took 1335 seconds.

--

Father Chrysostomos

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 22, 2017

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

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 22, 2017

From @cpansprout

On Fri, 22 Dec 2017 13​:40​:33 -0800, sprout wrote​:

On Fri, 22 Dec 2017 09​:41​:56 -0800, public@​khwilliamson.com wrote​:

I am posting this for Dan Book, who is having trouble getting email
to
perlbug through.

The build options and version of perl is immaterial here, except
since 5.22.

When the warning "Scalar value @​arrayname[0] better written as
$arrayname[0]"
is triggered (any time an array slice is used with only one element),
this results
in an error if the array's name starts with 'inf'. Code to reproduce​:

use warnings;
my @​infasdf;
my @​x = @​infasdf[0];
1

Error from above code (since 5.22)​: Cannot printf Inf with 'c' at
(IRC)
line 3.

$ ../perl.git-copy/Porting/bisect.pl --target=miniperl --start=v5.20.0
--end=v5.22.0 -- ./miniperl -Ilib /tmp/foo
...
3396ed3 is the first bad commit
commit 3396ed3
Author​: Jarkko Hietaniemi <jhi@​iki.fi>
Date​: Sun Jan 25 12​:27​:44 2015 -0500

infnan​: Simplify inf parsing.

Accept anything beginning with /^inf/i,
but warn if there's trailing stuff.

:100644 100644 66b08834448704597e416e7f3a024b8329245bd0
7819c60d94adac3f7b96b11ed816cc12d57b1a27 M numeric.c
:040000 040000 85a647618418ee945da09230ba2a172f26331d0c
5a472ae9721446be7d90ffdfe00bfb555e1c3d07 M t
bisect run success
That took 1335 seconds.

That commit only changed grok_infnan. I don‘t understand why this (from op.c​:scalar_slice_warning) calls grok_infnan​:

  if (key)
  /* diag_listed_as​: Scalar value @​%s[%s] better written as $%s[%s] */
  Perl_warner(aTHX_ packWARN(WARN_SYNTAX),
  "Scalar value @​%" SVf "%c%s%c better written as $%" SVf
  "%c%s%c",
  SVfARG(name), lbrack, key, rbrack, SVfARG(name),
  lbrack, key, rbrack);

--

Father Chrysostomos

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 22, 2017

From @jkeenan

On Fri, 22 Dec 2017 17​:41​:56 GMT, public@​khwilliamson.com wrote​:

I am posting this for Dan Book, who is having trouble getting email to
perlbug through.

The build options and version of perl is immaterial here, except since 5.22.

When the warning "Scalar value @​arrayname[0] better written as
$arrayname[0]"
is triggered (any time an array slice is used with only one element),
this results
in an error if the array's name starts with 'inf'. Code to reproduce​:

use warnings;
my @​infasdf;
my @​x = @​infasdf[0];
1

Error from above code (since 5.22)​: Cannot printf Inf with 'c' at (IRC)
line 3.

Bisection points to​:

#####
3396ed3 is the first bad commit
commit 3396ed3
Author​: Jarkko Hietaniemi <jhi@​iki.fi>
Date​: Sun Jan 25 12​:27​:44 2015 -0500

  infnan​: Simplify inf parsing.
 
  Accept anything beginning with /^inf/i,
  but warn if there's trailing stuff.
#####
--
James E Keenan (jkeenan@​cpan.org)

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 23, 2017

From @cpansprout

On Fri, 22 Dec 2017 14​:01​:29 -0800, jkeenan wrote​:

On Fri, 22 Dec 2017 17​:41​:56 GMT, public@​khwilliamson.com wrote​:

I am posting this for Dan Book, who is having trouble getting email
to
perlbug through.

The build options and version of perl is immaterial here, except
since 5.22.

When the warning "Scalar value @​arrayname[0] better written as
$arrayname[0]"
is triggered (any time an array slice is used with only one element),
this results
in an error if the array's name starts with 'inf'. Code to reproduce​:

use warnings;
my @​infasdf;
my @​x = @​infasdf[0];
1

Error from above code (since 5.22)​: Cannot printf Inf with 'c' at
(IRC)
line 3.

Bisection points to​:

#####
3396ed3 is the first bad commit
commit 3396ed3
Author​: Jarkko Hietaniemi <jhi@​iki.fi>
Date​: Sun Jan 25 12​:27​:44 2015 -0500

infnan​: Simplify inf parsing.

Accept anything beginning with /^inf/i,
but warn if there's trailing stuff.
#####

If I change the array name to @​inf, then it points to me​:

354b74a is the first bad commit
commit 354b74a
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Sat Sep 27 06​:48​:04 2014 -0700

  [perl #12285] Fix str vs num inf/nan treatment
 
  sprintf, pack and chr were treating 0+"Inf" and "Inf" differently,
  even though they have the same string and numeric values.
 
  pack was also croaking for 0+"Inf" passed to a string format.

That commit corrected much wrong logic, but missed one bit. The result was that any time perl’s printf mechanism is called at the C level (not directly from Perl code) with a format containing %s followed by a numeric format, it will croak if the string is ‘inf’.

$ ./perl -Ilib -we 'sub in; @​{in}'
Ambiguous use of @​{in} resolved to @​in at -e line 1.
Useless use of a variable in void context at -e line 1.
$ ./perl -Ilib -we 'sub inf; @​{inf}'
Cannot printf Inf with 'c' at -e line 1.

--

Father Chrysostomos

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 23, 2017

From zefram@fysh.org

Fixed in commit 0498098.

-zefram

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 23, 2017

@iabyn - Status changed from 'open' to 'pending release'

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 23, 2017

From @cpansprout

On Sat, 23 Dec 2017 01​:14​:38 -0800, zefram@​fysh.org wrote​:

Fixed in commit 0498098.

You ninjaed me. :-) I think the code is still fragile. The problem is that every other code path checks args for truth. But this one path, added by Jarkko in 540a63d, checks argsv for truth instead of !args.

--

Father Chrysostomos

@p5pRT

This comment has been minimized.

Copy link
Author

commented Jun 23, 2018

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release yesterday of Perl 5.28.0, this and 185 other issues have been
resolved.

Perl 5.28.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.28.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT

This comment has been minimized.

Copy link
Author

commented Jun 23, 2018

@khwilliamson - Status changed from 'pending release' to 'resolved'

@p5pRT p5pRT closed this Jun 23, 2018
@p5pRT p5pRT added the Severity Low label Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.