| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In a construct like
my ($x,$y) = @_
the pushmark/padsv/padsv is already optimised into a single padrange
op. This commit makes the OPf_SPECIAL flag on the padrange op indicate
that in addition, @_ should be pushed onto the stack, skipping an
additional pushmark/gv[*_]/rv2sv combination.
So in total (including the earlier padrange work), the above construct
goes from being
3 <0> pushmark s
4 <$> gv(*_) s
5 <1> rv2av[t3] lK/1
6 <0> pushmark sRM*/128
7 <0> padsv[$x:1,2] lRM*/LVINTRO
8 <0> padsv[$y:1,2] lRM*/LVINTRO
9 <2> aassign[t4] vKS
to
3 <0> padrange[$x:1,2; $y:1,2] l*/LVINTRO,2 ->4
4 <2> aassign[t4] vKS
|
| |
|
|
|
|
|
| |
$ ./miniperl -Ilib -w -e 'warn $$; while(1){eval {push @-, ""}}'
Croak first instead of creating a new element to store in the
array first and letting av_store croak.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Various pieces of code were creating an SV and then assigning to it
from a value that might be magical. If the source scalar is magical,
it could die when magic is called, leaking the scalar that would have
been assigned to.
So we call get-magic before creating the new scalar, and then use a
non-magical assignment.
Also, anonhash and anonlist were doing nothing to protect the aggre-
gate if an argument should die on FETCH, resulting in a leak.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When commit 9f621bb00 made length(undef) return undef, it also made it
return undef for objects with string overloading that returns undef.
But stringifying as undef is a contradiction in terms, and this makes
length inconsistent with defined, which returns true for such objects.
Changing this allows is to simplify pp_length, as we can now call
sv_len_utf8 on the argument unconditionally (except under the bytes
pragma). sv_len_utf8 is now careful not to record caches on magical
or overloaded scalars (any non-PV, in fact).
Note that sv_len is now just a wrapper around SvPV_const, so we use
SvPV_const_nomg, as there is no equivalent sv_len_nomg.
|
| |
|
|
|
| |
Since commit 6fc9266916, the if (SvFAKE) check under the SVt_PVGV case
in pp_undef has been redundant. And PVBMs are no longer GVs.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If it checks the UTF8 flag first, it might be looking at a stale flag,
resulting in malformed UTF8. Both tests added produced malformed utf8
strings before this commit.
Simply moving this:
if (!DO_UTF8(sv))
sv_utf8_upgrade(sv);
after the stringification is not enough to fix this, as the string
retrieved will be out of date after we do an upgrade. To avoid
stringifying twice, we use SvPV_force if there is a replacement. This
means rearranging if() blocks a little.
The use of SvPV_force also means that string overloading is no longer
called twice on the target scalar. This rearrangement also means
that targets upgraded to utf8 are no longer exempt from the refer-
ence warning. (Oh, and the test for that warning was not testing any-
thing in its no warnings test, because the target was no longer a ref-
erence; so I corrected the test.)
|
| |
|
|
|
| |
Typeglobs and references can change their UTF8ness upon string-
ification.
|
| |
|
|
|
|
|
|
|
|
|
| |
The easiest way to fix this was to move the special handling out of
the regexp engine. Now a flag is set on the split op itself for
this case. A real regexp is still created, as that is the most
convenient way to propagate locale settings, and it prevents the
need to rework pp_split to handle a null regexp.
This also means that custom regexp plugins no longer need to handle
split specially (which they all do currently).
|
| |
|
|
|
|
|
|
|
| |
See: https://rt.perl.org/rt3/Ticket/Display.html?id=113930#txn-1153156
By using find_runcv, we can revert d2c8bf052f. This may not be the
best tradeoff in the long run, as it makes code using experimental my
subs (my experimental subs?) slower. But at least we avoid slowing
down existing code.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
my subs are cloned on scope entry. To make closures work, a stub
stored in the pad (and closed over elsewhere) is cloned into.
But we need somewhere to store the prototype from which the clone is
made. I was attaching the prototype via magic to the stub in the pad,
since the pad is available at run time, but not the pad names.
That leads to lots of little games all over the place to make sure
the prototype isn’t lost when the pad is swiped on scope exit
(SAVEt_CLEARSV in scope.c). We also run the risk of losing it if an
XS module replaces the sub with another.
Instead, we should be storing it with the pad name. The previous com-
mit made the pad names available at run time, so we can move it there
(still stuffed inside a magic box) and delete much code.
This does mean that newMYSUB cannot rely on the behaviour of non-clon-
able subs that close over variables (or subs) immediately. Previ-
ously, we would dig through outer scopes to find the stub in cases
like this:
sub y {
my sub foo;
sub x {
sub {
sub foo { ... }
}
}
}
We would stop at x, which happens to have y’s stub in its pad, so
that’s no problem.
If we attach it to the pad name, we definitely have to dig past x to
get to the pad name in y’s pad.
Usually, immediate closures do not store the parent pad index, since
it will never be used. But now we do need to use it, so we modify the
code in pad.c:S_pad_findlex to set it always for my/state.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The pad slot for a my sub now holds a stub with a prototype CV
attached to it by proto magic.
The prototype is cloned on scope entry. The stub in the pad is used
when cloning, so any code that references the sub before scope entry
will be able to see that stub become defined, making these behave
similarly:
our $x;
BEGIN { $x = \&foo }
sub foo { }
our $x;
my sub foo { }
BEGIN { $x = \&foo }
Constants are currently not cloned, but that may cause bugs in
pad_push. I’ll have to look into that.
On scope exit, lexical CVs go through leave_scope’s SAVEt_CLEARSV sec-
tion, like lexical variables. If the sub is referenced elsewhere, it
is abandoned, and its proto magic is stolen and attached to a new stub
stored in the pad. If the sub is not referenced elsewhere, it is
undefined via cv_undef.
To clone my subs on scope entry, we create a sequence of introcv and
clonecv ops. See the huge comment in block_end that explains why we
need two separate ops for each CV.
To allow my subs to be defined in inner subs (my sub foo; sub { sub
foo {} }), pad_add_name_pvn and S_pad_findlex now upgrade the entry
for a my sub to a CV to begin with, so that fake entries added to pads
(fake entries are those that reference outer pads) can share the same
CV. Otherwise newMYSUB would have to add the CV to every pad that
closes over the ‘my sub’ declaration. newMYSUB no longer throws away
the initial value replacing it with a new one.
Prototypes are not currently visible to sub calls at compile time,
because the lexer sees the empty stub. A future commit will
solve that.
When I added name heks to CV’s I made mistakes in a few places, by not
turning on the CVf_NAMED flag, or by not clearing the field when free-
ing the hek. Those code paths were not exercised enough by state
subs, so the problems did not show up till now. So this commit fixes
those, too.
One of the tests in lexsub.t, involving foreach loops, was incorrect,
and has been fixed. Another test has been added to the end for a par-
ticular case of state subs closing over my subs that I broke when ini-
tially trying to get sibling my subs to close over each other, before
I had separate introcv and clonecv ops.
|
| |
|
|
|
|
| |
This will be used for cloning a ‘my’ sub on scope entry.
I was going to use pp_padcv for this, but it would end up having a
top-level if/else.
|
| |
|
|
|
| |
This will be used for introducing ‘my’ subs on scope entry, by turning
off the stale flag.
|
| |
|
|
| |
This will allow named lexical subs to exist independent of GVs.
|
| |
|
|
|
|
|
|
|
|
|
| |
State subs can now be referenced and called. Most of the tests in
lexsub.t are now passing. I noticed mistakes in a couple of the
tests and corrected them. In doing so I got an assertion failure
during compilation, so the tests in question I wrapped in a skipped
string eval.
State subs are now mostly working, but there are a few things to
clean up still.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
The rules for matching whether an above-Latin1 code point are now saved
in a macro generated from a trie by regen/regcharclass.pl, and these are
now used by pp.c to test these cases. This allows removal of a wrapper
subroutine, and also there is no need for dynamic loading at run-time
into a swash.
This macro is about as big as I'm comfortable compiling in, but it
saves the building of a hash that can grow over time, and removes a
subroutine and interpreter variables. Indeed, performance benchmarks
show that it is about the same speed as a hash, but it does not require
having to load the rules in from disk the first time it is used.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
(no functional changes).
1. Remove some dead code from pp_split; it's protected by an assert
that it could never be called.
2. Simplify the flags settings for the call to CALLREGEXEC() in
pp_substcont: on subsequent matches we always set REXEC_NOT_FIRST,
which forces the regex engine not to copy anyway, so passing the
REXEC_COPY_STR is pointless, as is the conditional code to set it.
3. (whitespace change): split a conditional expression over 2 lines
for easier reading.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When a pattern matches, and that pattern contains captures (or $`, $&, $'
or /p are present), a copy is made of the whole original string, so
that $1 et al continue to hold the correct value even if the original
string is subsequently modified. This can have severe performance
penalties; for example, this code causes a 1Mb buffer to be allocated,
copied and freed a million times:
$&;
$x = 'x' x 1_000_000;
1 while $x =~ /(.)/g;
This commit changes this so that, where possible, only the needed
substring of the original string is copied: in the above case, only a
1-byte buffer is copied each time. Also, it now reuses or reallocs the
buffer, rather than freeing and mallocing each time.
Now that PL_sawampersand is a 3-bit flag indicating separately whether
$`, $& and $' have been seen, they each contribute only their own
individual penalty; which ones have been seen will limit the extent to
which we can avoid copying the whole buffer.
Note that the above code *without* the $& is not currently slow, but only
because the copying is artificially disabled to avoid the performance hit.
The next but one commit will remove that hack, meaning that it will still
be fast, but will now be correct in the presence of a modified original
string.
We achieve this by by adding suboffset and subcoffset fields to the
existing subbeg and sublen fields of a regex, to indicate how many bytes
and characters have been skipped from the logical start of the string till
the physical start of the buffer. To avoid copying stuff at the end, we
just reduce sublen. For example, in this:
"abcdefgh" =~ /(c)d/
subbeg points to a malloced buffer containing "c\0"; sublen == 1,
and suboffset == 2 (as does subcoffset).
while if $& has been seen,
subbeg points to a malloced buffer containing "cd\0"; sublen == 2,
and suboffset == 2.
If in addition $' has been seen, then
subbeg points to a malloced buffer containing "cdefgh\0"; sublen == 6,
and suboffset == 2.
The regex engine won't do this by default; there are two new flag bits,
REXEC_COPY_SKIP_PRE and REXEC_COPY_SKIP_POST, which in conjunction with
REXEC_COPY_STR, request that the engine skip the start or end of the
buffer (it will still copy in the presence of the relevant $`, $&, $',
/p).
Only pp_match has been enhanced to use these extra flags; substitution
can't easily benefit, since the usual action of s///g is to copy the
whole string first time round, then perform subsequent matching iterations
against the copy, without further copying. So you still need to copy most
of the buffer.
|
| |
|
|
|
| |
The work around involves a runtime check and substituting OP pointers based
on the result. The substitution fails if the optree is mapped read-only.
|
| |
|
|
|
| |
By calling get-magic twice, it could cause its string buffer to be
reallocated, resulting in incorrect and random return values.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since 6ea72b3a1, rv2hv and padhv have had the ability to return boo-
leans in scalar context, instead of bucket stats, if flagged the right
way. sub { %hash || ... } is optimised to take advantage of this. If
the || is in unknown context at compile time, the %hash is flagged as
being maybe a true boolean. When flagged that way, it returns a bool-
ean if block_gimme() returns G_VOID.
If rv2hv and padhv can already do this, then we don’t need the
boolkeys op any more. We can just flag the rv2hv to return a boolean.
In all the cases where boolkeys was used, we know at compile time that
it is true boolean context, so we add a new flag for that.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In %hash || $foo, the %hash is in scalar context, so it has to iterate
through the buckets to produce statistics on bucket usage.
If the || is in void context, the value returned by hash is only ever
used as a boolean (as || doesn’t have to return it). We already opti-
mise it by adding a boolkeys op when it is known at compile time that
|| will be in void context.
In sub { %hash || $foo } it is not known at compile time that it will
be in void context, so it wasn’t optimised.
This commit optimises it by flagging the %hash at compile time as
being possibly in ‘true boolean’ context. When that flag is set,
the rv2hv and padhv ops call block_gimme() to see whether || is in
void context.
This speeds things up signficantly. Here is what I got after optimis-
ing rv2hv but before doing padhv:
$ time ./miniperl -e '%hash = 1..10000; sub { %hash || 1 }->() for 1..100000'
real 0m0.179s
user 0m0.101s
sys 0m0.005s
$ time ./miniperl -e 'my %hash = 1..10000; sub { %hash || 1 }->() for 1..100000'
real 0m5.446s
user 0m2.419s
sys 0m0.015s
(That example is slightly misleading because of the closure, but the
closure version takes 1 sec. when optimised.)
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The boolkeys optimisation (867fa1e2da1) was only applying to an and
(or if) in void context. If an if occurs as the last thing in a sub-
routine, the void context is not know at compile time so the optimisa-
tion does not apply.
In the case of || (to which the boolkeys optimisation also applies),
we can’t optimise it in non-void context, because someone might be
writing $bucket_info = %hash || '0/0';
In the case of &&, we can optimise it, even in non-void context,
because a true value will always be discarded in %hash && foo.
The false value it returns for an empty hash is always the int-
eger 0. That would change if we simply applied boolkeys to
my $ret = %hash && foo; because boolkeys return &PL_sv_no (the dualvar
you get from !1). But since boolkeys’ return value is never directly
visible to perl code, we can safely change that.
|
| |
|
|
|
| |
If it’s going to consume and return exactly one item, it doesn’t need
to decrement and increment the stack pointer.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
CVs close over their outer CVs. So, when you write:
my $x = 52;
sub foo {
sub bar {
sub baz {
$x
}
}
}
baz’s CvOUTSIDE pointer points to bar, bar’s CvOUTSIDE points to foo,
and foo’s to the main cv.
When the inner reference to $x is looked up, the CvOUTSIDE chain is
followed, and each sub’s pad is looked at to see if it has an $x.
(This happens at compile time.)
It can happen that bar is undefined and then redefined:
undef &bar;
eval 'sub bar { my $x = 34 }';
After this, baz will still refer to the main cv’s $x (52), but, if baz
had ‘eval '$x'’ instead of just $x, it would see the new bar’s $x.
(It’s not really a new bar, as its refaddr is the same, but it has a
new body.)
This particular case is harmless, and is obscure enough that we could
define it any way we want, and it could still be considered correct.
The real problem happens when CVs are cloned.
When a CV is cloned, its name pad already contains the offsets into
the parent pad where the values are to be found. If the outer CV
has been undefined and redefined, those pad offsets can be com-
pletely bogus.
Normally, a CV cannot be cloned except when its outer CV is running.
And the outer CV cannot have been undefined without also throwing
away the op that would have cloned the prototype.
But formats can be cloned when the outer CV is not running. So it
is possible for cloned formats to close over bogus entries in a new
parent pad.
In this example, \$x gives us an array ref. It shows ARRAY(0xbaff1ed)
instead of SCALAR(0xdeafbee):
sub foo {
my $x;
format =
@
($x,warn \$x)[0]
.
}
undef &foo;
eval 'sub foo { my @x; write }';
foo
__END__
And if the offset that the format’s pad closes over is beyond the end
of the parent’s new pad, we can even get a crash, as in this case:
eval
'sub foo {' .
'{my ($a,$b,$c,$d,$e,$f,$g,$h,$i,$j,$k,$l,$m,$n,$o,$p,$q,$r,$s,$t,$u)}'x999
. q|
my $x;
format =
@
($x,warn \$x)[0]
.
}
|;
undef &foo;
eval 'sub foo { my @x; my $x = 34; write }';
foo();
__END__
So now, instead of using CvROOT to identify clones of
CvOUTSIDE(format), we use the padlist ID instead. Padlists don’t
actually have an ID, so we give them one. Any time a sub is cloned,
the new padlist gets the same ID as the old. The format needs to
remember what its outer sub’s padlist ID was, so we put that in the
padlist struct, too.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This removes most register declarations in C code (and accompanying
documentation) in the Perl core. Retained are those in the ext
directory, Configure, and those that are associated with assembly
language.
See:
http://stackoverflow.com/questions/314994/whats-a-good-example-of-register-variable-usage-in-c
which says, in part:
There is no good example of register usage when using modern compilers
(read: last 10+ years) because it almost never does any good and can do
some bad. When you use register, you are telling the compiler "I know
how to optimize my code better than you do" which is almost never the
case. One of three things can happen when you use register:
The compiler ignores it, this is most likely. In this case the only
harm is that you cannot take the address of the variable in the
code.
The compiler honors your request and as a result the code runs slower.
The compiler honors your request and the code runs faster, this is the least likely scenario.
Even if one compiler produces better code when you use register, there
is no reason to believe another will do the same. If you have some
critical code that the compiler is not optimizing well enough your best
bet is probably to use assembler for that part anyway but of course do
the appropriate profiling to verify the generated code is really a
problem first.
|
| |
|
|
|
| |
Now that gmagical svs use the OK flags the same way as muggles,
things like SvPOK || (SvGMAGICAL && SvPOKp) are no longer necessary.
|
| |
|
|
|
| |
I think I added the if (OP_TRANSR) block to the wrong spot, because the
mortal scalar created just before it is only used for y/// without /r.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In restore_magic(), which is called after any magic processing, all of
the public OK flags have been shifted into the private OK flags. Thus
the lack of an appropriate public OK flags was used to trigger both get
magic and required conversions. This scheme did not cover ROK, however,
so all properly written code had to make sure mg_get was called the right
number of times anyway. Meanwhile the private OK flags gained a second
purpose of marking converted but non-authoritative values (e.g. the IV
conversion of an NV), and the inadequate flag shift mechanic broke this
in some cases.
This patch removes the shift mechanic for magic flags, thus exposing (and
fixing) some improper usage of magic SVs in which mg_get() was not called
correctly. It also has the side effect of making magic get functions
specifically set their SVs to undef if that is desired, as the new behavior
of empty get functions is to leave the value unchanged. This is a feature,
as now get magic that does not modify its value, e.g. tainting, does not
have to be special cased.
The changes to cpan/ here are only temporary, for development only, to
keep blead working until upstream applies them (or something like them).
Thanks to Rik and Father C for review input.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This makes the negation operator under the integer pragma (i_int) use
the same logic for determining whether to do string negation as the
regular negation operator.
Before, this did not happen at all under the integer pragma, except
for barewords, resulting in strange inconsistencies:
$ perl -le 'use integer; print -foo'
-foo
$ perl -le 'use integer; print -"foo"'
0
The code for string negation is now in a static routine in pp.c and is
used by both types of negation.
|
| |
|
|
| |
Also, prevent a crash when SvCUR is used on a non-PV.
|
| | |
|
| |
|
|
|
|
| |
The two branches of this were almost identical. Not only is it unnec-
essary to repeat the code, but it is error-prone, as bug fixes have to
be done in both branches.
|
| |
|
|
|
|
|
|
|
| |
The XS API allows hash entries to be created with null values. perl
itself uses these internally, too.
Though they should rarely be seen by Perl code, Perl ops should still
be able to handle them without crashing (by croaking). I fixed helem
and hslice in 5.16 (commit 746f6409), but missed localised deletions.
|
| | |
|
| |
|
|
|
|
|
|
|
| |
It was returning U+FFFD for negative numbers, unless they
were strings.
The SvOK is to avoid double uninit warnings. The !SvIsUV is for
efficiency. We don’t need to coerce it to an NV if it is a UV
already, because we know it won’t pass that test.
|
| |
|
|
|
| |
It was returning U+FFFD for negative numbers, but only for non-magical
variables.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Ops that don’t allow arbitrarily long argument lists are flagged at compile-time with the number of children.
Coresubs have to call the same op with differing numbers of arguments,
so they push nulls on to the stack to make up the number of items the
pp function expects to pop.
Commit f914a6829 stopped popping the null of the stack.
Before:
$ ./perl -le 'print &CORE::srand'
1240765685
After:
$ ./perl -le 'print &CORE::srand'
Bus error
List assignment does the same thing, and makes it easier to
write a test.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Commit bee7c5743fa appears to have fixed this. But what it does is
barely significant:
diff --git a/sv.c b/sv.c
index b96f7c1..a4994f5 100644
--- a/sv.c
+++ b/sv.c
@@ -9525,6 +9525,11 @@ Perl_sv_bless(pTHX_ SV *const sv, HV *const stash)
SvUPGRADE(tmpRef, SVt_PVMG);
SvSTASH_set(tmpRef, MUTABLE_HV(SvREFCNT_inc_simple(stash)));
+ if (Gv_AMG(stash))
+ SvAMAGIC_on(sv);
+ else
+ (void)SvAMAGIC_off(sv);
+
if(SvSMAGICAL(tmpRef))
if(mg_find(tmpRef, PERL_MAGIC_ext) || mg_find(tmpRef, PERL_MAGIC_uvar))
mg_set(tmpRef);
The crash can still be triggered another way. Instead of a blessing,
we need to modify a method (to turn on the potentially-overloaded
flag) and then use an operator that respects overloading. This exam-
ple crashes before and after bee7c5743fa:
eval 'sub Sample::foo {}';
"".bless {},'Sample';
delete local $Sample::{ '()' };
It is the recalculation of overload caches before a localised deletion
that causes the crash. And it only happens when the '()' key does
not exist.
Actually, it turns out that S_delete_local doesn’t behave correctly
for rmagical aggregates, except for %ENV:
$ ./perl -Ilib -MDevel::Peek -e 'delete local $ISA[0]'
Bus error
$ ./perl -XIlib -MDevel::Peek -e '??; delete local $::{foo}'
Bus error
It’s this line, which occurs twice in pp.c:S_do_delete_local, which
is at fault:
const bool can_preserve = SvCANEXISTDELETE(osv)
|| mg_find((const SV *)osv, PERL_MAGIC_env);
When can_preserve is true, the ‘preeminent’ variable is set based on
whether the element exists. Otherwise it is set to true.
Why the term ‘preeminent’ was chosen I don’t know, but in this case it
means that the element already exists, so it has to be restored after-
wards. We can’t just do save_delete.
The code for saving a hash element assumes it is non-null, and crashes
otherwise.
The logic for setting can_preserve is wrong. SvCANEXISTDELETE returns
true for non-magical variables and for variables with those tie meth-
ods implemented. For magical variables that are not tied, it returns
the wrong answer. PERL_MAGIC_env seems to have been added as an
exception, to keep it working. But other magical aggregates were not
accounted for.
This logic was copied from other functions (aslice, hslice, etc.),
which are similarly buggy, but they don’t crash:
$ ./perl -Ilib -le ' { local $::{foo} } print exists $::{foo}'
$ ./perl -Ilib -le 'm??; { local $::{foo} } print exists $::{foo}'
1
In all these cases, it is SvCANEXISTDELETE that is buggy. So this
commit fixes it and adds tests for all the code paths that use it.
Now no exception needs to be made for PERL_MAGIC_env.
|
| |
|
|
| |
to make sure it really is never reached.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since we already have a check further down to see whether a string
begins with an identifier or sign, and since looks_like_number
was added for strings representing negative numbers, move the
looks_like_number down to where we already know the string
begins with '-'.
This is a micro-optimisation, but it also makes the code more
straightforward (to me at least).
This happens to let magical integers-as-strings fall down to code that
they used not to reach, so that has to change to account.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
-$1 was treating $1 as a float even if the string consisted of an
integer, due to incorrect flag checks. It was doing the same things
with tied variables returning str+int dualvars.
Simply checking whether the privates flags consist solely of SVp_IOK
(which works for tie variables returning pure integers--so I wasn’t
entirely wrong in adding that logic a few commits ago), isn’t suffi-
cient. For gmagical variables that have already had get-magic called
on them, the private flags are equivalent to public flags for other
variables.
|
