develooper Front page | perl.perl5.changes | Postings from March 2019

[perl.git] branch blead updated. v5.29.8-65-gca6ebcd640

From:
Dave Mitchell
Date:
March 11, 2019 17:05
Subject:
[perl.git] branch blead updated. v5.29.8-65-gca6ebcd640
Message ID:
E1h3OMc-00009m-Id@git.dc.perl.space
In perl.git, the branch blead has been updated

<https://perl5.git.perl.org/perl.git/commitdiff/ca6ebcd6405d5ab46fb0688c45dc44661698a7c0?hp=ebf06983127fdec6e5f31c7ecc2d3a3ac991cfa2>

- Log -----------------------------------------------------------------
commit ca6ebcd6405d5ab46fb0688c45dc44661698a7c0
Author: David Mitchell <davem@iabyn.com>
Date:   Mon Mar 11 16:18:10 2019 +0000

    Fix leak on syntax error in main prog
    
    t/lib/croak.t was failing several tests under ASan because it was
    running small stand-alone programs with some sort of error in, such as
    
        BEGIN { }
        myfunc 1;
    
    Unlike other code paths (such as S_doeval_compile() for evals),
    Perl_newPROG() - when called for the main body rather than for a
    completed eval - was calling cv_forget_slab() on PL_compcv regardless of
    whether an error was present. That call converts  the compiling CV into
    a compiled one, which disclaims ownership of the slab(s) its ops are
    embedded in. This means that when the CV is freed, ops within the slab
    which aren't embedded within the PL_main_root tree would leak.
    
    Such ops may exist when Perl_newPROG() is reached after one of more
    errors.
    
    The fix is simply to not call cv_forget_slab() if the error count is > 0.

commit 02a9632ac4bf515585a2f25b05b2939de1743ded
Author: David Mitchell <davem@iabyn.com>
Date:   Fri Mar 8 08:40:29 2019 +0000

    fix leak when compiling typed hash deref
    
    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.

-----------------------------------------------------------------------

Summary of changes:
 op.c              | 27 +++++++++++++++++++--------
 t/op/multideref.t | 11 ++++++++++-
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/op.c b/op.c
index 40bc2ef84e..4e49eeeedf 100644
--- a/op.c
+++ b/op.c
@@ -2450,12 +2450,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;
@@ -2505,7 +2506,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);
@@ -3731,7 +3733,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:
@@ -5413,7 +5415,10 @@ Perl_newPROG(pTHX_ OP *o)
         start = LINKLIST(PL_main_root);
 	PL_main_root->op_next = 0;
         S_process_optree(aTHX_ NULL, PL_main_root, start);
-	cv_forget_slab(PL_compcv);
+        if (!PL_parser->error_count)
+            /* on error, leave CV slabbed so that ops left lying around
+             * will eb cleaned up. Else unslab */
+            cv_forget_slab(PL_compcv);
 	PL_compcv = 0;
 
 	/* Register with debugger */
@@ -14691,12 +14696,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
@@ -14704,9 +14710,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 20ba1ca614..12b04536e5 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");
+}
+

-- 
Perl5 Master Repository



nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About