summaryrefslogtreecommitdiff
path: root/pp_sys.c
diff options
context:
space:
mode:
authorDavid Mitchell <davem@iabyn.com>2022-11-10 13:04:58 +0000
committerYves Orton <demerphq@gmail.com>2023-02-28 20:53:51 +0800
commit782514efb3d0b2cb91b55f2ac00c89940e973417 (patch)
tree597cafb5909ff63ed80de9630d66e74df3410b85 /pp_sys.c
parent697ec28afb3b2a5eb26f836eaea4cb4071bd71c6 (diff)
downloadperl-782514efb3d0b2cb91b55f2ac00c89940e973417.tar.gz
fix check order for filetest overload
The tryAMAGICftest_MG() macro was doing two checks: 1) seeing whether the filetest operator's arg (*PL_stack_sp) looks to have magic or be a reference, 2) and if the op has children (which will have pushed an arg, unlike (-X _), If both are true, then do full-on magic and/or overload processing. The problem with this is that it checks the arg *before* checking whether there's even an arg. Thus in the case of (-X _), it is actually examining a random SV on the stack (or in the case of nothing on the stack, examining the PL_sv_undef pointer always stored at PL_stack_base[0] as a guard.) It turns out this was harmless - the test for (1) will examine a random (but real) SV and get garbage results, but then the 2nd test will fail anyway, so overloading won't be called. So the fix is to swap the (1) and (2) test order. In addition, I changed the 'has an argument' test from OPf_KIDS to !OPf_REF. These should be mutually exclusive, but the OPf_REF flag formally indicates (-X _), i.e. that no arg has been pushed on the stack. Whether the op has children or not could potentially change in the future, independent of whether it's the (-X _) form. So overall this commit makes no visible functional difference, but may make the code more robust against future changes.
Diffstat (limited to 'pp_sys.c')
-rw-r--r--pp_sys.c6
1 files changed, 4 insertions, 2 deletions
diff --git a/pp_sys.c b/pp_sys.c
index 26a9dda006..348c43a261 100644
--- a/pp_sys.c
+++ b/pp_sys.c
@@ -3108,9 +3108,11 @@ S_ft_return_true(pTHX_ SV *ret) {
#define FT_RETURNUNDEF return S_ft_return_false(aTHX_ &PL_sv_undef)
#define FT_RETURNYES return S_ft_return_true(aTHX_ &PL_sv_yes)
+/* NB: OPf_REF implies '-X _' and thus no arg on the stack */
#define tryAMAGICftest_MG(chr) STMT_START { \
- if ( (SvFLAGS(*PL_stack_sp) & (SVf_ROK|SVs_GMG)) \
- && PL_op->op_flags & OPf_KIDS) { \
+ if ( !(PL_op->op_flags & OPf_REF) \
+ && (SvFLAGS(*PL_stack_sp) & (SVf_ROK|SVs_GMG))) \
+ { \
OP *next = S_try_amagic_ftest(aTHX_ chr); \
if (next) return next; \
} \