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

Assigning number to %SIG should fail #12867

Open
p5pRT opened this issue Mar 21, 2013 · 6 comments
Open

Assigning number to %SIG should fail #12867

p5pRT opened this issue Mar 21, 2013 · 6 comments

Comments

@p5pRT
Copy link

p5pRT commented Mar 21, 2013

Migrated from rt.perl.org#117261 (status was 'open')

Searchable as RT117261$

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2013

From @epa

Created by @epa

The %SIG hash lets you store a code ref, the magic keywords 'DEFAULT' or 'IGNORE', or another string.
In the third case it is taken as the name of a subroutine to call.

However, you can store strings such as '1' which are not legal subroutine names
(except in code that goes to special lengths to mess with the symbol table).
It looks like the current behaviour is for these to be silently ignored, but
it would be better for perl to die or at least warn when this happens. For example

  $SIG{__WARN__} = 1; # should fail somehow

This would be useful because it would catch the mistake of putting code instead of an anonymous sub​:

  $SIG{__WARN__} = say 'something went wrong';

With this proposed change, the return value of say() being 0 or 1 would not be
a legal subroutine name, so perl would not allow it to be stored in $SIG{__WARN__}.

I might also suggest that 'use strict' should disable storing the names of subroutines
in the hash, in the same way it disables treating a subroutine name as a code ref.
But that might break existing code.

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.16.2:

Configured by Red Hat, Inc. at Mon Feb 18 16:52:57 UTC 2013.

Summary of my perl5 (revision 5 version 16 subversion 2) configuration:
   
  Platform:
    osname=linux, osvers=2.6.32-279.19.1.el6.x86_64, archname=x86_64-linux-thread-multi
    uname='linux buildvm-11.phx2.fedoraproject.org 2.6.32-279.19.1.el6.x86_64 #1 smp sat nov 24 14:35:28 est 2012 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Doptimize=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4  -m64 -mtune=generic -Dccdlflags=-Wl,--enable-new-dtags -Dlddlflags=-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4  -m64 -mtune=generic -Wl,-z,relro  -DDEBUGGING=-g -Dversion=5.16.2 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5 -Dsitearch=/usr/local/lib64/perl5 -Dprivlib=/usr/share/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Darchlib=/usr/lib64/perl5 -Dvendorarch=/usr/lib64/perl5/vendor_perl -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Dusedtrace=/usr/bin/dtrace -Duselargefiles -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin -Dusesitecustomize'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.7.2 20121109 (Red Hat 4.7.2-8)', 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='gcc', ldflags =' -fstack-protector'
    libpth=/usr/local/lib64 /lib64 /usr/lib64
    libs=-lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat
    perllibs=-lresolv -lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.16'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,--enable-new-dtags -Wl,-rpath,/usr/lib64/perl5/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -Wl,-z,relro '

Locally applied patches:
    


@INC for perl 5.16.2:
    /home/eda/lib/perl5/
    /usr/local/lib64/perl5
    /usr/local/share/perl5
    /usr/lib64/perl5/vendor_perl
    /usr/share/perl5/vendor_perl
    /usr/lib64/perl5
    /usr/share/perl5
    .


Environment for perl 5.16.2:
    HOME=/home/tradingsystems
    LANG=en_GB.UTF-8
    LANGUAGE (unset)
    LC_COLLATE=C
    LC_CTYPE=en_GB.UTF-8
    LC_MESSAGES=en_GB.UTF-8
    LC_MONETARY=en_GB.UTF-8
    LC_NUMERIC=en_GB.UTF-8
    LC_TIME=en_GB.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/eda/bin:/home/eda/bin:/usr/local/bin:/usr/bin:/sbin:/usr/sbin:/sbin:/usr/sbin
    PERL5LIB=/home/eda/lib/perl5/
    PERL_BADLANG (unset)
    SHELL=/bin/bash


______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
______________________________________________________________________

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2013

From zefram@fysh.org

Ed Avis wrote​:

However, you can store strings such as '1' which are not legal subroutine names
(except in code that goes to special lengths to mess with the symbol table).

Not very special, since "1" does actually meet Perl's identifier syntax
for sigilled contexts​:

  *1 = sub { print "hi"; };
  &1;

A better example would be "!a", the use of which requires braces and
quotes​:

  *{"!a"} = sub { print "hi"; };
  &{"!a"};

Hmm, still not terribly special lengths. Haven't had to touch the
stash directly.

It looks like the current behaviour is for these to be silently ignored,

No, they work​:

$ perl -lwe '*1 = sub { print "hi"; }; $SIG{INT} = 1; kill "INT", 0'
hi

If you don't define the sub, the fault is reported (as a warning) when
an attempt is made to invoke the handler. This is just like what happens
if you don't define a signal handler with a conventional name​:

$ perl -lwe '$SIG{INT} = "foo"; print "x"; kill "INT", 0; print "y"'
x
SIGINT handler "foo" not defined.
y
$ perl -lwe '$SIG{INT} = 1; print "x"; kill "INT", 0; print "y"'
x
SIGINT handler "1" not defined.
y

This substitute handler could sensibly die rather than warn.

it would be better for perl to die or at least warn when this happens.

Currently the behaviour of a string (other than the magic strings)
or of a code ref in %SIG is consistently to call &{$SIG{$signame}}.
It's a type of reference resolution that is used elsewhere in Perl.
To allow some names but not others would be inventing a new type of
reference resolution, with the associated maintenance, documentation,
and teaching burdens. Not appealing.

This would be useful because it would catch the mistake of putting code
instead of an anonymous sub​:

It would only catch it if the code evaluates to something that doesn't
meet the permitted identifier syntax. And it would only be useful,
catching a mistake that's not otherwise obvious, if the code has no
visible side effects (a "say" executing during startup should be pretty
obvious), and if the signal handler is never actually invoked during
testing (for invocation would trigger the "handler not defined" warning).

Also, how often do you make this mistake? Once per person is probably
enough to learn, for those who were confused to begin with about executing
code now versus executing it later.

I might also suggest that 'use strict' should disable storing the names
of subroutines in the hash, in the same way it disables treating a
subroutine name as a code ref.

Such a stricture makes some sense, but not under "use strict" and,
er, not really for %SIG. "use strict" is lexically scoped, but the
interpretation of the string as a sub name doesn't occur within the
assigner's lexical scope​: it happens magically within the core when
the signal is being handled. We don't want to make "use strict" have
a global effect. Maybe you could have an explicit global stricture,
but that would break the reasonable expectations of existing modules.
Or you could imagine somehow applying the stricture on assignments
within a declaring lexical scope, but because %SIG is presented as
a variable, and assignment to hash elements is rather well defined,
assignments through references could result in code assigning to %SIG
without knowing specifically that the lvalue is a %SIG element.

Maybe you want to wrap up assignment to %SIG in a subroutine that checks
that the value is a code ref. If you never assign to it directly,
your bug can't arise.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2013

From @epa

By 'code that goes to special lengths to mess with the symbol table'
I meant exactly what you suggest​: storing things directly in typeglobs.
I do contend that creating a subroutine called '1' is not something any
ordinary Perl program will do, and if someone is expert enough to
mess with the symbol table directly they're also expert enough to
store a code ref in %SIG.

It looks like the current behaviour is for these to be silently ignored,

No, they work​:

$ perl -lwe '*1 = sub { print "hi"; }; $SIG{INT} = 1; kill "INT", 0'
hi

Ah, the case I tested was this​:

$ perl -E '$SIG{__WARN__} = "1"; warn'
Warning​: something's wrong at -e line 1.

I had expected to get an error 'undefined subroutine main​::1 called'
but instead perl silently ignored the $SIG setting and called the
normal warning routine. However, it appears this is not specific
to numbers​: any nonexistent subroutine is also silently ignored,
and turning warnings on doesn't help​:

% perl -E 'use warnings; $SIG{__WARN__} = "a"; warn'
Warning​: something's wrong at -e line 1.

So that's a separate issue​: nonexistent subroutine names should fail,
ideally with an error, but since that might break existing code, a
warning would be safer.

If you don't define the sub, the fault is reported (as a warning) when
an attempt is made to invoke the handler.

What you wrote is true for $SIG{INT} but appears not so for __WARN__
and __DIE__.

Currently the behaviour of a string (other than the magic strings)
or of a code ref in %SIG is consistently to call &{$SIG{$signame}}.
It's a type of reference resolution that is used elsewhere in Perl.
To allow some names but not others would be inventing a new type of
reference resolution, with the associated maintenance, documentation,
and teaching burdens. Not appealing.

I don't suggest that. I think there should be a warning at the point
of assignment to $SIG{x}, not when the signal happens. So if you really,
really want to (a) make a subroutine called '123' and (b) have it called
as a signal handler and (c) specify the signal handler as a string rather
than a coderef, you can still do that, and temporarily put 'no warnings'
at the point of assignment to $SIG{x}.

This would be useful because it would catch the mistake of putting code
instead of an anonymous sub​:

Also, how often do you make this mistake?

Pretty often if my own limited record is to be taken as representative ;-p.
The fact that this mistake is specifically warned against in the docs
suggests that it's reasonably common. Providing an easily understood
warning would save programmers a bit of head-scratching.

I think that a check on assignment to $SIG{x} (that the value is not a string
starting with a digit) would help catch some mistakes made both by
beginniners and relative experts, would not have any false positives in
practice, and would be straightforward to implement. I might be wrong
about the implementation but IMHO the other two considerations argue
for adding a warning here.

--
Ed Avis <eda@​waniasset.com>

______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http​://www.symanteccloud.com
______________________________________________________________________

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2013

From zefram@fysh.org

Ed Avis wrote​:

                              if someone is expert enough to

mess with the symbol table directly they're also expert enough to
store a code ref in %SIG.

I'd rather not tie together these disparate aspects of Perl programming.
(And assigning to a typeglob is still not direct symbol table access.)

$ perl -E '$SIG{__WARN__} = "1"; warn'
Warning​: something's wrong at -e line 1.

Aha, this is indeed a problem. If $SIG{__WARN__} or $SIG{__DIE__} is a
string that does not name an extant sub, some error should be reported,
as it is in the equivalent situation for *signal* handlers in %SIG.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2013

From @epa

I don't mean to cast any judgements on typeglob assignment or anything like that.
My point is simply that assigning a raw number to $SIG{x} is in practice always a mistake.
And a fairly common one IME.

A warning would indeed have a false positive if someone really wanted to make
a numeric subroutine and put it in %SIG by name rather than by reference.
But that is one reason why I suggest a warning rather than a hard error - we accept
that a warning only indicates code which is very likely to be wrong (or become wrong
in a future release) and can be turned off if wanted.

--
Ed Avis <eda@​waniasset.com>

______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http​://www.symanteccloud.com
______________________________________________________________________

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

No branches or pull requests

2 participants