The macro body needed two variables to use as temporaries. This meant
lots of callsites with apparently-unused variables. Worse, an
unsuspecting author might use this macro and accidentally corrupt
existing variables called `mg` or `stash` in subtle hard-to-find bugs.
Since we can now use static inline functions this is much neater moved
into such a function, avoiding the risk of breaking those variables.
This allows a debugging perl to be built with the high water mark
checks disabled, or a non-debugging perl to be built with the
high water marks enabled.
This should allow Debian, the reporter for #16607 to build both their
normal perl and debugperl with the same state of high water mark
checks and avoid the mismatch between a debugperl and non-debug
dynamic extension.
Fixes#16607
This macro was originally only used in two places in core (pp_readline
and pp_glob), and nowhere on CPAN.
The last few commits inlined those only two usages, then modified the
functions using that code to be PERL_RC_STACK-aware.
Since the macro is now unused, and is the old obsolete non-PERL_RC_STACK
code, this commit just deletes it.
This long macro is only used in two places (pp_readline and pp_glob).
Expand the contents of this macro directly in those two functions.
This will make it easier to individually unwrap (i.e. remove
PP_wrapped()) those two functions.
Shouldn't be any functional change.
By default it will use I32 for backward compatibility, but you
can select SSize_t during configuration with:
-Accflags=-DPERL_STACK_OFFSET_SSIZET
which may one day become the default.
The actual offsets throughout the code continue to be SSize_t.
An earlier commit added some asserts to the old-style and new-style
stack push/pop functions such as PUSHs() or rpp_popfree_1(). These
assert that the stack is reference-counted or not as appropriate.
These asserts were enabled only if perl was built with all of
DEBUGGING && PERL_RC_STACK && DEBUG_LEAKING_SCALARS.
This commit makes the asserts available under just
DEBUGGING && PERL_RC_STACK
Initially I was worried about the performance implications (you expect
DEBUGGING builds to be slow, but not *too* slow), but they don't seem
too bad. So by making them more likely to be enabled, they're more
likely to help people spot code that needs fixing (e.g. code still doing
POPs when the stack is reference counted and the function hasn't been
wrapped).
On perls built with both -DPERL_RC_STACK and -DDEBUG_LEAKING_SCALARS,
add asserts to the various old-style PUSHs() etc macros and new-style
rpp_push_1() etc functions that they're operating on the right sort of
stack. In particular:
PUSHs() and POPs() should only be used on stacks where rpp_stack_is_rc()
is false, since they don't modify the reference count of the SVs they
are pushing or popping.
Conversely, rpp_push_1(), rpp_popfree_1() etc should only be used on
stacks where rpp_stack_is_rc() is true, since they modify the reference
counts of the SVs they push and pop.
On perls not compiled with PERL_RC_STACK, the rpp_ functions don't
modify the reference counts, but on such builds the rpp_stack_is_rc()
assertions in the rpp_ functions are disabled, so it all works out
still.
These new inline functions are supposed to be near-identical
replacements for these macros:
PUSHSTACKi(type) push_stackinfo(type)
POPSTACK() pop_stackinfo()
SWITCHSTACK(from,to) switch_argstack(to) // assumes (from == PL_curstack)
except that they don't require dSP to be in scope (they operate on
PL_stack_sp rather than sp) and their names more clearly specify what
sort of stack they manipulate.
The macros are now thin wrappers around the functions. I've kept most of
their uses in core for now as they are still used in places with dSP
present.
In the presence of PERL_RC_STACK, wrap most pp functions with a function
that calls the real pp function with a (non-reference-counted) copy of
the stack arguments. On return, the wrapper bumps up the reference
count of the returned list and, as appropriate, shifts them down the
stack.
This allows a pp function which is not aware of a reference-counted
stack to be called in a reference-counted stack environment.
About 250 such functions are wrappable; the remaining 50 or so need
individual handling and will be dealt with in the following few commits.
This is a temporary band-aid which slows down calls to pp() functions.
It is expected that over time these functions will be unwrapped and
updated to to understand a reference-counted stack. But by temporarily
wrapping them now, it allows perl to be (experimentally) shifted over to
a reference-counted stack without having to review and modify all 30K
lines of pp code first.
(Except currently, the wrapper assumes the stack is still not
reference-counted in the presence of PERL_XXX_TMP_NORC. This will be
turned off in a few commits' time.)
Note also that pp_wrap() is not yet complete. It simulates wrapping by
making a copy of the args on entry to the wrapped function and shifting
any returned values back down. But as well as being temporarily
inhibited from actually modifying reference counts by PERL_XXX_TMP_NORC,
there are extra considerations which will be taken account of by the
"Allow stacks to be reference-counted" commit coming later.
This set of PUSH-style macros takes a string literal argument and pushes
it to the stack, optionally mortalizing it and/or extending the stack.
Previously, the best alternative was
mPUSHp("literal", 7);
which required the author to visually count the number of characters in
the string literal (7 in this case). These new macros fit the similar
pattern familiar to many functions such as `newSVpvs`, which takes a
string literal and counts it directly.
This should be being used only in core, as its only use is for autodoc.
Change the flag name to be more mnemonic, freeing up its current name
for another use.
This just detabifies to get rid of the mixed tab/space indentation.
Applying consistent indentation and dealing with other tabs are another issue.
Done with `expand -i`.
* vutil.* left alone, it's part of version.
* Left regen managed files alone for now.
This feature allows documentation destined for perlapi or perlintern to
be split into sections of related functions, no matter where the
documentation source is. Prior to this commit the line had to contain
the exact text of the title of the section. Now it can be a $variable
name that autodoc.pl expands to the title. It still has to be an exact
match for the variable in autodoc, but now, the expanded text can be
changed in autodoc alone, without other files needing to be updated at
the same time.
This is being done only for those functions that don't have a guard
preventing them from being seen outside of the Perl core.
Talking to Tony Cook, we agreed that this was a good idea for two
reasons:
1) The 'Perl_' prefix does not pollute XS caller's name space. The 'S_'
one could be argued that it doesn't much either, but it does more so
than 'Perl_', and the next reason is the clincher:
2) It allows us to change our minds about whether a function should be
static inline or not, without affecting callers who use the Perl_
form, which they would be accustomed to anyway if they're using the
full name form.
This makes various fixes to the text that is used to generate the
documentation. The dominant change is to add the 'n' flag to indicate
that the macro takes no arguments. A couple should have been marked
with a D (for deprecated) flag, and a couple were missing parameters,
and a couple were missing return values.
These were spotted by using Devel::PPPort on them.
This indicates to not output the macro with parentheses for parameters.
Currently that doesn't happen anyway, but a future commit will change
things so this is required (so that a bug can be fixed)
I added this flag a few years ago when I revamped the overload macros
tryAMAGICbin() etc. It allowed two different classes of macros to
share the same functions (Perl_try_amagic_un/Perl_try_amagic_bin)
by indicating what type of action is required.
However, the last few commits have made those two functions able to
robustly always determine whether its an assign-type action
($x op= $y or $lex = $x op $x) or a plain set-result-on-stack operation
($x op $y).
So eliminate this flag.
Note that this makes the ops which have the AMGf_set flag hard-coded
infinitesimally slower, since Perl_try_amagic_bin no longer skips the
checks for assign-ness. But compared with the overhead of having
already called the overload method, this is is trivial.
On the plus side, it makes the code smaller and easier to understand.
This macro is defined as
(PL_op->op_flags & OPf_STACKED)
and indicates, for ops which support it, that the mutator-variant of the
op is present (e.g. $x += 1).
This macro was mainly used as an arg for the old-style overloading
macros (tryAMAGICbin()) which were eliminated several years ago.
This commit removes its vestigial usage, and instead tests OPf_STACKED
directly at each location, along with adding a comment about the
significance of the flag.
This removes one item of obfuscation from the overloading code.
There is one potentially functional change in this commit:
Perl_try_amagic_bin() was sometimes testing for OPf_STACKED without
first checking that it had been called with the AMGf_assign flag (which
indicates that this op supports a mutator variant). With this commit, it
now checks first, so this is theoretically a bug fix. In practice that
section of code was never reached without AMGf_assign always being set
anyway.
On debugging builds only, add a mechanism for checking pp function calls
for insufficient stack extending. It works by:
* make the runops loop set a high-water-mark (HWM) variable equal to
PL_stack_sp just before calling each pp function;
* make EXTEND() etc update this HWM;
* on return from the pp function, panic if PL_stack_sp is > HWM.
This detects whether pp functions are pushing more items onto the stack
than they are requesting space for.
There's a possibility of false positives if the code is doing weird stuff
like direct manipulation of stacks via PL_curstack, SWITCHSTACK() etc.
It's also possible that one pp function "knows" that a previous pp
function will have already grown the stack enough. Currently the only
place in core that seems to do this is pp_enteriter, which allocates 1
stack slot so that pp_iter doesn't have to check each time it returns
&PL_sv_yes/no. To accommodate this, the new macro EXTEND_SKIP() has been
added, that tells perl that it's safely skipping an EXTEND() here.
C++11 requires space between the end of a string literal and a macro, so
that a feature can unambiguously be added to the language. Starting in
g++ 6.2, the compiler emits a warning when there isn't a space
(presumably so that future versions can support C++11). Unfortunately
there are many such instances in the perl core. This commit fixes
those, including those in ext/, but individual commits will be used for
the other modules, those in dist/ and cpan/.
This commit also inserts space at the end of a macro before a string
literal, even though that is not deprecated, and removes useless ""
literals following a macro (instead of inserting a blank). The result
is easier to read, making the macro stand out, and be clearer as to the
intention.
Code and modules included with the Perl core need to be compilable using
C++. This is so that perl can be embedded in C++ programs. (Actually,
only the hdr files need to be so compilable, but it would be hard to
test that just the hdrs are compilable.) So we need to accommodate
changes to the C++ language.
Use static inline functions to avoid having different code paths
for GCC and non-GCC. INCMARK is unused on CPAN and only used as
a statement in core, so it can become a statement.
The value of gimme stored in the context stack is U8.
Make all other uses in the main core consistent with this.
My primary motivation on this was that the new function cx_pushblock(),
which I gave a 'U8 gimme' parameter, was generating warnings where callers
were passing I32 gimme vars to it. Rather than play whack-a-mole, it
seemed simpler to just uniformly use U8 everywhere.
Porting/bench.pl shows a consistent reduction of about 2 instructions on
the loop and sub benchmarks, so this change isn't harming performance.
The debugging variants of POPMARK() etc: use %IVdf rather than %d
to avoid compiler warnings when sizeof(IV) != sizeof(int);
also, make these macros trigger only under -Dsv rather than just -Ds
(-v is the verbose variant).
Most integers are small, so in most cases it won't be set.
The other option would be to always clear it, but that increases the
amount of inline code for a rare case.
v5.23.3-305-g6cae08a introduced debugging variants of INCMARK/POPMARK, and
replaced a number of "PL_markstack_ptr--;" with "POPMARK;" etc.
This spews a bunch of "value computed is not used" warnings; so
add some "(void)"s.
Also indent the new definitions of INCMARK/POPMARK correctly.
These macros are used by many pp functions to set TARG to an int or float
value and put it on the stack. The macros use a call to sv_setiv()
or similar.
However, there is a good chance that the target is ether a lexical var
or a PADTMP that has been assigned just an IV/NV in the past, in which
case its body is likely to still be of type SVt_IV/SVt_NV. If this is
the case, we can set its value much more efficiently.
Update those setting macros to first check if the target has a simple
body, and if so, set directly.
This makes quite a significant performance difference on code that does
a lot of arithmetic, at the cost of a larger binary (0.36% on my Linux
x86_64 system).
It also allows you (at compile time) to skip testing for taint if you
know that the value can't be tainted (e.g. pp_add() when both args are
non-magical and so can't have taint magic attached). The next commit will
make use of this.
Includes a couple of efficiency tweaks suggested by Daniel Dragan.
This commit fixes various issues around stack_grow() and its
two main wrappers, EXTEND() and MEXTEND(). In particular it behaves
very badly on systems with 32-bit pointers but 64-bit ints.
One noticeable effect of this is commit is that various usages of EXTEND()
etc will now start to give compiler warnings - usually because they're
passing an unsigned N arg when it should be signed. This may indicate
a logic error in the caller's code which needs fixing. This commit causes
several such warnings to appear in core code, which will be fixed in the
next commit.
Essentially there are several potential false negatives in this basic
code:
if (PL_stack_max - p < (SSize_t)(n))
stack_grow(sp,p,(SSize_t)(n));
where it incorrectly skips the call to stack_grow() and then the caller
tramples over the end of the stack because it assumes that it has in fact
been extended. The value of N passed to stack_grow() can also potentially
get truncated or wrapped.
Note that the N arg of stack_grow() is SSize_t and EXTEND()'s N arg is
documented as SSize_t. In earlier times, they were both ints.
Significantly, this means that they are both signed, and always have been.
In detail, the problems and their solutions are:
1) N is a signed value: if negative, it could be an indication of a
caller's invalid logic or wrapping in the caller's code. This should
trigger a panic. Make it so by adding an extra test to EXTEND() to
always call stack_grow if negative, then add a check and panic in
stack_grow() (and other places too). This extra test will be constant
folded when EXTEND() is called with a literal N.
2) If the caller passes an unsigned value of N, then the comparison is
between a signed and an unsigned value, leading to potential
wrap-around. Casting N to SSize_t merely hides any compiler warnings,
thus failing to alert the caller to a problem with their code. In
addition, where sizeof(N) > sizeof(SSize_t), the cast may truncate N,
again leading to false negatives. The solution is to remove the cast,
and let the caller deal with any compiler warnings that result.
3) Similarly, casting stack_grow()'s N arg can hide any warnings issued by
e.g. -Wconversion. So remove it. It still does the wrong thing if the
caller uses a non-signed type (usually a panic in stack_grow()), but
coders have slightly more chance of spotting issues at compile time
now.
4) If sizeof(N) > sizeof(SSize_t), then the N arg to stack_grow() may get
truncated or sign-swapped. Add a test for this (basically that N is too
big to fit in a SSize_t); for simplicity, in this case just set N to
-1 so that stack_grow() panics shortly afterwards. In platforms where
this can't happen, the test is constant folded away.
With all these changes, the macro now looks in essence like:
if ( n < 0 || PL_stack_max - p < n)
stack_grow(sp,p,
(sizeof(n) > sizeof(SSize_t) && ((SSize_t)(n) != n) ? -1 : n));
- linkify references to sections
- use EXTEND(SP, n) and PUSHs() instead of XPUSHs() where applicable
and update prose to match
- add POPu, POPul and POPpbytex to the "complete list of POP macros"
and clarify the documentation for some of the existing entries, and
a note about side-effects
- add API documentation for POPu and POPul
- use ERRSV more efficiently
- approaches to thread-safety storage of SVs.
- minor updates
An empty cpan/.dir-locals.el stops Emacs using the core defaults for
code imported from CPAN.
Committer's work:
To keep t/porting/cmp_version.t and t/porting/utils.t happy, $VERSION needed
to be incremented in many files, including throughout dist/PathTools.
perldelta entry for module updates.
Add two Emacs control files to MANIFEST; re-sort MANIFEST.
For: RT #124119.
