From 057b890a6d3201a44afd68c840f3a76d4f508d91 Mon Sep 17 00:00:00 2001
From: David Mitchell <davem@iabyn.com>
Date: Fri, 8 Mar 2019 08:40:29 +0000
Subject: [PATCH] fix leak when compiling typed hash deref
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
In something like
my Foo $h;
$h->{bad_key}
perl will croak if package Foo defines valid %FIELDS and bad_key isn't
one of them. This croak happens during the second pass in
S_maybe_multideref(), which is trying to convert $h->{bad_key} into a
single multideref op. Since the aux buffer is allocated at the end of
the first pass, the buffer leaks.
The fix is to do the check in the first pass, which has been done by
adding an extra boolean flag to S_check_hash_fields_and_hekify(),
indicating whether to just check or actually do it.
Petr Písař: Ported to 5.18.1 from
02a9632ac4bf515585a2f25b05b2939de1743ded.
Signed-off-by: Petr Písař <ppisar@redhat.com>
---
op.c | 22 +++++++++++++++-------
t/op/multideref.t | 11 ++++++++++-
2 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/op.c b/op.c
index 67da715..af3c448 100644
--- a/op.c
+++ b/op.c
@@ -2416,12 +2416,13 @@ S_modkids(pTHX_ OP *o, I32 type)
/* for a helem/hslice/kvslice, if its a fixed hash, croak on invalid
* const fields. Also, convert CONST keys to HEK-in-SVs.
- * rop is the op that retrieves the hash;
+ * rop is the op that retrieves the hash;
* key_op is the first key
+ * real if false, only check (and possibly croak); don't update op
*/
STATIC void
-S_check_hash_fields_and_hekify(pTHX_ UNOP *rop, SVOP *key_op)
+S_check_hash_fields_and_hekify(pTHX_ UNOP *rop, SVOP *key_op, int real)
{
PADNAME *lexname;
GV **fields;
@@ -2471,7 +2472,8 @@ S_check_hash_fields_and_hekify(pTHX_ UNOP *rop, SVOP *key_op)
if ( !SvIsCOW_shared_hash(sv = *svp)
&& SvTYPE(sv) < SVt_PVMG
&& SvOK(sv)
- && !SvROK(sv))
+ && !SvROK(sv)
+ && real)
{
SSize_t keylen;
const char * const key = SvPV_const(sv, *(STRLEN*)&keylen);
@@ -3648,7 +3650,7 @@ S_finalize_op(pTHX_ OP* o)
check_keys:
if (o->op_private & OPpLVAL_INTRO || rop->op_type != OP_RV2HV)
rop = NULL;
- S_check_hash_fields_and_hekify(aTHX_ rop, key_op);
+ S_check_hash_fields_and_hekify(aTHX_ rop, key_op, 1);
break;
}
case OP_NULL:
@@ -14605,12 +14607,13 @@ S_maybe_multideref(pTHX_ OP *start, OP *orig_o, UV orig_action, U8 hints)
* the extra hassle for those edge cases */
break;
- if (pass) {
+ {
UNOP *rop = NULL;
OP * helem_op = o->op_next;
ASSUME( helem_op->op_type == OP_HELEM
- || helem_op->op_type == OP_NULL);
+ || helem_op->op_type == OP_NULL
+ || pass == 0);
if (helem_op->op_type == OP_HELEM) {
rop = (UNOP*)(((BINOP*)helem_op)->op_first);
if ( helem_op->op_private & OPpLVAL_INTRO
@@ -14618,9 +14621,14 @@ S_maybe_multideref(pTHX_ OP *start, OP *orig_o, UV orig_action, U8 hints)
)
rop = NULL;
}
- S_check_hash_fields_and_hekify(aTHX_ rop, cSVOPo);
+ /* on first pass just check; on second pass
+ * hekify */
+ S_check_hash_fields_and_hekify(aTHX_ rop, cSVOPo,
+ pass);
+ }
#ifdef USE_ITHREADS
+ if (pass) {
/* Relocate sv to the pad for thread safety */
op_relocate_sv(&cSVOPo->op_sv, &o->op_targ);
arg->pad_offset = o->op_targ;
diff --git a/t/op/multideref.t b/t/op/multideref.t
index 20ba1ca..12b0453 100644
--- a/t/op/multideref.t
+++ b/t/op/multideref.t
@@ -18,7 +18,7 @@ BEGIN {
use warnings;
use strict;
-plan 63;
+plan 64;
# check that strict refs hint is handled
@@ -233,3 +233,12 @@ sub defer {}
is $x[qw(rt131627)->$*], 11, 'RT #131627: $a[qw(var)->$*]';
}
+# this used to leak - run the code for ASan to spot any problems
+{
+ package Foo;
+ our %FIELDS = ();
+ my Foo $f;
+ eval q{ my $x = $f->{c}; };
+ ::pass("S_maybe_multideref() shouldn't leak on croak");
+}
+
--
2.20.1