Insert asserts to paths suspected by Coverity.
authorJarkko Hietaniemi <jhi@iki.fi>
Thu, 29 May 2014 15:37:56 +0000 (11:37 -0400)
committerJarkko Hietaniemi <jhi@iki.fi>
Thu, 29 May 2014 15:49:14 +0000 (11:49 -0400)
Coverity suspects paths like this:

(1)

  p = foo();
  if (!p) p = bar();
  baz(p->q);

since it cannot prove that p is always non-NULL at the dereference.

(2) Or simply something like:

  mg = mg_find(...);
  foo(mg->blah);

Since mg_find() can fail returning NULL.

Adding assert() calls before the dereferences.  Testing with -DDEBUGGING.
Hopefully there are regular smokes doing the same.

[perl #121894]

Fix for Coverity perl5 CIDs 28950,28952..28955,28964,28967,28970..28795,49921:
CID ...: Dereference after null check (FORWARD_NULL)
var_deref_op: Dereferencing null pointer p->q

(TODO: Coverity perl5 CIDs 28962,28968,28969: the same issue, but would
conflict with already in-flight changes, prepare catch-up patch later.)
---
 dist/Data-Dumper/Dumper.xs | 1 +
 dump.c                     | 1 +
 ext/Devel-Peek/Peek.xs     | 1 +
 ext/XS-APItest/APItest.xs  | 1 +
 mg.c                       | 2 ++
 mro.c                      | 4 +++-
 op.c                       | 4 ++++
 perlio.c                   | 1 +
 pp_hot.c                   | 5 +++--
 regcomp.c                  | 3 +++
 regexec.c                  | 5 ++++-
 universal.c                | 2 +-
 util.c                     | 4 +++-
 13 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/dist/Data-Dumper/Dumper.xs b/dist/Data-Dumper/Dumper.xs
index 12c4ebd..23e0cf4 100644
--- a/dist/Data-Dumper/Dumper.xs
+++ b/dist/Data-Dumper/Dumper.xs
@@ -641,6 +641,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
      else {
        sv_pattern = val;
      }
+     assert(sv_pattern);
      rval = SvPV(sv_pattern, rlen);
      rend = rval+rlen;
      slash = rval;
diff --git a/dump.c b/dump.c
index 59be3e0..c2d72fd 100644
--- a/dump.c
+++ b/dump.c
@@ -471,6 +471,7 @@ Perl_sv_peek(pTHX_ SV *sv)
   finish:
     while (unref--)
  sv_catpv(t, ")");
+    /* XXX when is sv ever NULL? */
     if (TAINTING_get && SvTAINTED(sv))
  sv_catpv(t, " [tainted]");
     return SvPV_nolen(t);
diff --git a/ext/Devel-Peek/Peek.xs b/ext/Devel-Peek/Peek.xs
index 679efa5..b20fa94 100644
--- a/ext/Devel-Peek/Peek.xs
+++ b/ext/Devel-Peek/Peek.xs
@@ -450,6 +450,7 @@ PPCODE:
 BOOT:
 {
     CV * const cv = get_cvn_flags("Devel::Peek::Dump", 17, 0);
+    assert(cv);
     cv_set_call_checker(cv, S_ck_dump, (SV *)cv);

     XopENTRY_set(&my_xop, xop_name, "Dump");
diff --git a/ext/XS-APItest/APItest.xs b/ext/XS-APItest/APItest.xs
index a51924d..18ba381 100644
--- a/ext/XS-APItest/APItest.xs
+++ b/ext/XS-APItest/APItest.xs
@@ -2096,6 +2096,7 @@ newCONSTSUB(stash, name, flags, sv)
                break;
         }
         EXTEND(SP, 2);
+        assert(mycv);
         PUSHs( CvCONST(mycv) ? &PL_sv_yes : &PL_sv_no );
         PUSHs((SV*)CvGV(mycv));

diff --git a/mg.c b/mg.c
index 76912bd..7f3339a 100644
--- a/mg.c
+++ b/mg.c
@@ -1675,6 +1675,7 @@ Perl_magic_clearisa(pTHX_ SV *sv, MAGIC *mg)
     same function. */
  mg = mg_find(mg->mg_obj, PERL_MAGIC_isa);

+    assert(mg);
     if (SvTYPE(mg->mg_obj) == SVt_PVAV) { /* multiple stashes */
  SV **svp = AvARRAY((AV *)mg->mg_obj);
  I32 items = AvFILLp((AV *)mg->mg_obj) + 1;
@@ -3437,6 +3438,7 @@ Perl_magic_copycallchecker(pTHX_ SV *sv, MAGIC *mg, SV *nsv,

     sv_magic(nsv, &PL_sv_undef, mg->mg_type, NULL, 0);
     nmg = mg_find(nsv, mg->mg_type);
+    assert(nmg);
     if (nmg->mg_flags & MGf_REFCOUNTED) SvREFCNT_dec(nmg->mg_obj);
     nmg->mg_ptr = mg->mg_ptr;
     nmg->mg_obj = SvREFCNT_inc_simple(mg->mg_obj);
diff --git a/mro.c b/mro.c
index 1b37ca7..ccf4bf4 100644
--- a/mro.c
+++ b/mro.c
@@ -638,12 +638,14 @@ Perl_mro_isa_changed_in(pTHX_ HV* stash)
                       hv_storehek(mroisarev, namehek, &PL_sv_yes);
                 }

-                if((SV *)isa != &PL_sv_undef)
+                if ((SV *)isa != &PL_sv_undef) {
+                    assert(namehek);
                     mro_clean_isarev(
                      isa, HEK_KEY(namehek), HEK_LEN(namehek),
                      HvMROMETA(revstash)->isa, HEK_HASH(namehek),
                      HEK_UTF8(namehek)
                     );
+                }
             }
         }
     }
diff --git a/op.c b/op.c
index 796cb03..79621ce 100644
--- a/op.c
+++ b/op.c
@@ -2907,6 +2907,7 @@ S_my_kid(pTHX_ OP *o, OP *attrs, OP **imopsp)
      S_cant_declare(aTHX_ o);
  } else if (attrs) {
      GV * const gv = cGVOPx_gv(cUNOPo->op_first);
+     assert(PL_parser);
      PL_parser->in_my = FALSE;
      PL_parser->in_my_stash = NULL;
      apply_attrs(GvSTASH(gv),
@@ -2929,6 +2930,7 @@ S_my_kid(pTHX_ OP *o, OP *attrs, OP **imopsp)
     else if (attrs && type != OP_PUSHMARK) {
  HV *stash;

+        assert(PL_parser);
  PL_parser->in_my = FALSE;
  PL_parser->in_my_stash = NULL;

@@ -10229,6 +10231,7 @@ Perl_ck_split(pTHX_ OP *o)
  op_append_elem(OP_SPLIT, o, newDEFSVOP());

     kid = kid->op_sibling;
+    assert(kid);
     scalar(kid);

     if (!kid->op_sibling)
@@ -10902,6 +10905,7 @@ Perl_cv_set_call_checker(pTHX_ CV *cv, Perl_call_checker ckfun, SV *ckobj)
  MAGIC *callmg;
  sv_magic((SV*)cv, &PL_sv_undef, PERL_MAGIC_checkcall, NULL, 0);
  callmg = mg_find((SV*)cv, PERL_MAGIC_checkcall);
+ assert(callmg);
  if (callmg->mg_flags & MGf_REFCOUNTED) {
      SvREFCNT_dec(callmg->mg_obj);
      callmg->mg_flags &= ~MGf_REFCOUNTED;
diff --git a/perlio.c b/perlio.c
index d4c43d0..e6ff9e4 100644
--- a/perlio.c
+++ b/perlio.c
@@ -2225,6 +2225,7 @@ PerlIOBase_dup(pTHX_ PerlIO *f, PerlIO *o, CLONE_PARAMS *param, int flags)
  PerlIO_funcs * const self = PerlIOBase(o)->tab;
  SV *arg = NULL;
  char buf[8];
+ assert(self);
  PerlIO_debug("PerlIOBase_dup %s f=%p o=%p param=%p\n",
       self ? self->name : "(Null)",
       (void*)f, (void*)o, (void*)param);
diff --git a/pp_hot.c b/pp_hot.c
index 2cccc48..9c9d1e9 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -3078,6 +3078,7 @@ S_method_common(pTHX_ SV* meth, U32* hashp)
  const HE* const he = hv_fetch_ent(stash, meth, 0, *hashp);
  if (he) {
      gv = MUTABLE_GV(HeVAL(he));
+     assert(stash);
      if (isGV(gv) && GvCV(gv) &&
  (!GvCVGEN(gv) || GvCVGEN(gv)
                   == (PL_sub_generation + HvMROMETA(stash)->cache_gen)))
@@ -3085,9 +3086,9 @@ S_method_common(pTHX_ SV* meth, U32* hashp)
  }
     }

+    assert(stash || packsv);
     gv = gv_fetchmethod_sv_flags(stash ? stash : MUTABLE_HV(packsv),
-              meth, GV_AUTOLOAD | GV_CROAK);
-
+                                 meth, GV_AUTOLOAD | GV_CROAK);
     assert(gv);

     return isGV(gv) ? MUTABLE_SV(GvCV(gv)) : MUTABLE_SV(gv);
diff --git a/regcomp.c b/regcomp.c
index eaee604..3d49827 100644
--- a/regcomp.c
+++ b/regcomp.c
@@ -2007,6 +2007,7 @@ S_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
     });

     re_trie_maxbuff = get_sv(RE_TRIE_MAXBUF_NAME, 1);
+    assert(re_trie_maxbuff);
     if (!SvIOK(re_trie_maxbuff)) {
         sv_setiv(re_trie_maxbuff, RE_TRIE_MAXBUF_INIT);
     }
@@ -14920,6 +14921,7 @@ S_set_ANYOF_arg(pTHX_ RExC_state_t* const pRExC_state,
  av_store(av, 0, (runtime_defns)
  ? SvREFCNT_inc(runtime_defns) : &PL_sv_undef);
  if (swash) {
+     assert(cp_list);
      av_store(av, 1, swash);
      SvREFCNT_dec_NN(cp_list);
  }
@@ -16609,6 +16611,7 @@ S_dumpuntil(pTHX_ const regexp *r, const regnode *start, const regnode *node,
         last= plast;

     while (PL_regkind[op] != END && (!last || node < last)) {
+        assert(node);
  /* While that wasn't END last time... */
  NODE_ALIGN(node);
  op = OP(node);
diff --git a/regexec.c b/regexec.c
index 362390b..ffab4f2 100644
--- a/regexec.c
+++ b/regexec.c
@@ -7010,6 +7010,8 @@ no_silent:
                 sv_commit = &PL_sv_yes;
             sv_yes_mark = &PL_sv_no;
         }
+        assert(sv_err);
+        assert(sv_mrk);
         sv_setsv(sv_err, sv_commit);
         sv_setsv(sv_mrk, sv_yes_mark);
     }
@@ -7620,6 +7622,7 @@ Perl__get_regclass_nonbitmap_data(pTHX_ const regexp *prog,
                     *only_utf8_locale_ptr = ary[2];
                 }
                 else {
+                    assert(only_utf8_locale_ptr);
                     *only_utf8_locale_ptr = NULL;
                 }

@@ -7641,7 +7644,7 @@ Perl__get_regclass_nonbitmap_data(pTHX_ const regexp *prog,
      }
      else if (doinit && ((si && si != &PL_sv_undef)
                                  || (invlist && invlist != &PL_sv_undef))) {
-
+ assert(si);
  sw = _core_swash_init("utf8", /* the utf8 package */
        "", /* nameless */
        si,
diff --git a/universal.c b/universal.c
index a29696d..65e02df 100644
--- a/universal.c
+++ b/universal.c
@@ -67,7 +67,7 @@ S_isa_lookup(pTHX_ HV *stash, const char * const name, STRLEN len, U32 flags)
     if (our_stash) {
  HEK *canon_name = HvENAME_HEK(our_stash);
  if (!canon_name) canon_name = HvNAME_HEK(our_stash);
-
+ assert(canon_name);
  if (hv_common(isa, NULL, HEK_KEY(canon_name), HEK_LEN(canon_name),
        HEK_FLAGS(canon_name),
        HV_FETCH_ISEXISTS, NULL, HEK_HASH(canon_name))) {
diff --git a/util.c b/util.c
index b90abe5..cd0afb6 100644
--- a/util.c
+++ b/util.c
@@ -850,15 +850,17 @@ Perl_fbm_instr(pTHX_ unsigned char *big, unsigned char *bigend, SV *littlestr, U

     {
  const MAGIC *const mg = mg_find(littlestr, PERL_MAGIC_bm);
- const unsigned char * const table = (const unsigned char *) mg->mg_ptr;
  const unsigned char *oldlittle;

+ assert(mg);
+
  --littlelen; /* Last char found by table lookup */

  s = big + littlelen;
  little += littlelen; /* last char */
  oldlittle = little;
  if (s < bigend) {
+     const unsigned char * const table = (const unsigned char *) mg->mg_ptr;
      I32 tmp;

    top2:
--
1.8.5.2 (Apple Git-48)

13 files changed:
dist/Data-Dumper/Dumper.xs
dump.c
ext/Devel-Peek/Peek.xs
ext/XS-APItest/APItest.xs
mg.c
mro.c
op.c
perlio.c
pp_hot.c
regcomp.c
regexec.c
universal.c
util.c

index 5bc3428..e98c6d7 100644 (file)
@@ -642,6 +642,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
            else {
              sv_pattern = val;
            }
+           assert(sv_pattern);
            rval = SvPV(sv_pattern, rlen);
            rend = rval+rlen;
            slash = rval;
diff --git a/dump.c b/dump.c
index 59be3e0..c2d72fd 100644 (file)
--- a/dump.c
+++ b/dump.c
@@ -471,6 +471,7 @@ Perl_sv_peek(pTHX_ SV *sv)
   finish:
     while (unref--)
        sv_catpv(t, ")");
+    /* XXX when is sv ever NULL? */
     if (TAINTING_get && SvTAINTED(sv))
        sv_catpv(t, " [tainted]");
     return SvPV_nolen(t);
index 679efa5..b20fa94 100644 (file)
@@ -450,6 +450,7 @@ PPCODE:
 BOOT:
 {
     CV * const cv = get_cvn_flags("Devel::Peek::Dump", 17, 0);
+    assert(cv);
     cv_set_call_checker(cv, S_ck_dump, (SV *)cv);
 
     XopENTRY_set(&my_xop, xop_name, "Dump");
index fb5e366..1e750c3 100644 (file)
@@ -2095,6 +2095,7 @@ newCONSTSUB(stash, name, flags, sv)
                break;
         }
         EXTEND(SP, 2);
+        assert(mycv);
         PUSHs( CvCONST(mycv) ? &PL_sv_yes : &PL_sv_no );
         PUSHs((SV*)CvGV(mycv));
 
diff --git a/mg.c b/mg.c
index 76912bd..7f3339a 100644 (file)
--- a/mg.c
+++ b/mg.c
@@ -1675,6 +1675,7 @@ Perl_magic_clearisa(pTHX_ SV *sv, MAGIC *mg)
           same function. */
        mg = mg_find(mg->mg_obj, PERL_MAGIC_isa);
 
+    assert(mg);
     if (SvTYPE(mg->mg_obj) == SVt_PVAV) { /* multiple stashes */
        SV **svp = AvARRAY((AV *)mg->mg_obj);
        I32 items = AvFILLp((AV *)mg->mg_obj) + 1;
@@ -3437,6 +3438,7 @@ Perl_magic_copycallchecker(pTHX_ SV *sv, MAGIC *mg, SV *nsv,
 
     sv_magic(nsv, &PL_sv_undef, mg->mg_type, NULL, 0);
     nmg = mg_find(nsv, mg->mg_type);
+    assert(nmg);
     if (nmg->mg_flags & MGf_REFCOUNTED) SvREFCNT_dec(nmg->mg_obj);
     nmg->mg_ptr = mg->mg_ptr;
     nmg->mg_obj = SvREFCNT_inc_simple(mg->mg_obj);
diff --git a/mro.c b/mro.c
index 1b37ca7..ccf4bf4 100644 (file)
--- a/mro.c
+++ b/mro.c
@@ -638,12 +638,14 @@ Perl_mro_isa_changed_in(pTHX_ HV* stash)
                       hv_storehek(mroisarev, namehek, &PL_sv_yes);
                 }
 
-                if((SV *)isa != &PL_sv_undef)
+                if ((SV *)isa != &PL_sv_undef) {
+                    assert(namehek);
                     mro_clean_isarev(
                      isa, HEK_KEY(namehek), HEK_LEN(namehek),
                      HvMROMETA(revstash)->isa, HEK_HASH(namehek),
                      HEK_UTF8(namehek)
                     );
+                }
             }
         }
     }
diff --git a/op.c b/op.c
index 75aae39..266ac8c 100644 (file)
--- a/op.c
+++ b/op.c
@@ -2912,6 +2912,7 @@ S_my_kid(pTHX_ OP *o, OP *attrs, OP **imopsp)
            S_cant_declare(aTHX_ o);
        } else if (attrs) {
            GV * const gv = cGVOPx_gv(cUNOPo->op_first);
+           assert(PL_parser);
            PL_parser->in_my = FALSE;
            PL_parser->in_my_stash = NULL;
            apply_attrs(GvSTASH(gv),
@@ -2934,6 +2935,7 @@ S_my_kid(pTHX_ OP *o, OP *attrs, OP **imopsp)
     else if (attrs && type != OP_PUSHMARK) {
        HV *stash;
 
+        assert(PL_parser);
        PL_parser->in_my = FALSE;
        PL_parser->in_my_stash = NULL;
 
@@ -10238,6 +10240,7 @@ Perl_ck_split(pTHX_ OP *o)
        op_append_elem(OP_SPLIT, o, newDEFSVOP());
 
     kid = kid->op_sibling;
+    assert(kid);
     scalar(kid);
 
     if (!kid->op_sibling)
@@ -10912,6 +10915,7 @@ Perl_cv_set_call_checker(pTHX_ CV *cv, Perl_call_checker ckfun, SV *ckobj)
        MAGIC *callmg;
        sv_magic((SV*)cv, &PL_sv_undef, PERL_MAGIC_checkcall, NULL, 0);
        callmg = mg_find((SV*)cv, PERL_MAGIC_checkcall);
+       assert(callmg);
        if (callmg->mg_flags & MGf_REFCOUNTED) {
            SvREFCNT_dec(callmg->mg_obj);
            callmg->mg_flags &= ~MGf_REFCOUNTED;
index 4dc3ec3..2ce8ac1 100644 (file)
--- a/perlio.c
+++ b/perlio.c
@@ -2227,6 +2227,7 @@ PerlIOBase_dup(pTHX_ PerlIO *f, PerlIO *o, CLONE_PARAMS *param, int flags)
        PerlIO_funcs * const self = PerlIOBase(o)->tab;
        SV *arg = NULL;
        char buf[8];
+       assert(self);
        PerlIO_debug("PerlIOBase_dup %s f=%p o=%p param=%p\n",
                     self ? self->name : "(Null)",
                     (void*)f, (void*)o, (void*)param);
index 2cccc48..9c9d1e9 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -3078,6 +3078,7 @@ S_method_common(pTHX_ SV* meth, U32* hashp)
        const HE* const he = hv_fetch_ent(stash, meth, 0, *hashp);
        if (he) {
            gv = MUTABLE_GV(HeVAL(he));
+           assert(stash);
            if (isGV(gv) && GvCV(gv) &&
                (!GvCVGEN(gv) || GvCVGEN(gv)
                   == (PL_sub_generation + HvMROMETA(stash)->cache_gen)))
@@ -3085,9 +3086,9 @@ S_method_common(pTHX_ SV* meth, U32* hashp)
        }
     }
 
+    assert(stash || packsv);
     gv = gv_fetchmethod_sv_flags(stash ? stash : MUTABLE_HV(packsv),
-                                    meth, GV_AUTOLOAD | GV_CROAK);
-
+                                 meth, GV_AUTOLOAD | GV_CROAK);
     assert(gv);
 
     return isGV(gv) ? MUTABLE_SV(GvCV(gv)) : MUTABLE_SV(gv);
index 94dfd55..e3a01b6 100644 (file)
--- a/regcomp.c
+++ b/regcomp.c
@@ -2007,6 +2007,7 @@ S_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
     });
 
     re_trie_maxbuff = get_sv(RE_TRIE_MAXBUF_NAME, 1);
+    assert(re_trie_maxbuff);
     if (!SvIOK(re_trie_maxbuff)) {
         sv_setiv(re_trie_maxbuff, RE_TRIE_MAXBUF_INIT);
     }
@@ -14923,6 +14924,7 @@ S_set_ANYOF_arg(pTHX_ RExC_state_t* const pRExC_state,
        av_store(av, 0, (runtime_defns)
                        ? SvREFCNT_inc(runtime_defns) : &PL_sv_undef);
        if (swash) {
+           assert(cp_list);
            av_store(av, 1, swash);
            SvREFCNT_dec_NN(cp_list);
        }
@@ -16612,6 +16614,7 @@ S_dumpuntil(pTHX_ const regexp *r, const regnode *start, const regnode *node,
         last= plast;
 
     while (PL_regkind[op] != END && (!last || node < last)) {
+        assert(node);
        /* While that wasn't END last time... */
        NODE_ALIGN(node);
        op = OP(node);
index 7d6827a..d2994d4 100644 (file)
--- a/regexec.c
+++ b/regexec.c
@@ -7010,6 +7010,8 @@ no_silent:
                 sv_commit = &PL_sv_yes;
             sv_yes_mark = &PL_sv_no;
         }
+        assert(sv_err);
+        assert(sv_mrk);
         sv_setsv(sv_err, sv_commit);
         sv_setsv(sv_mrk, sv_yes_mark);
     }
@@ -7620,6 +7622,7 @@ Perl__get_regclass_nonbitmap_data(pTHX_ const regexp *prog,
                     *only_utf8_locale_ptr = ary[2];
                 }
                 else {
+                    assert(only_utf8_locale_ptr);
                     *only_utf8_locale_ptr = NULL;
                 }
 
@@ -7641,7 +7644,7 @@ Perl__get_regclass_nonbitmap_data(pTHX_ const regexp *prog,
            }
            else if (doinit && ((si && si != &PL_sv_undef)
                                  || (invlist && invlist != &PL_sv_undef))) {
-
+               assert(si);
                sw = _core_swash_init("utf8", /* the utf8 package */
                                      "", /* nameless */
                                      si,
index a29696d..65e02df 100644 (file)
@@ -67,7 +67,7 @@ S_isa_lookup(pTHX_ HV *stash, const char * const name, STRLEN len, U32 flags)
     if (our_stash) {
        HEK *canon_name = HvENAME_HEK(our_stash);
        if (!canon_name) canon_name = HvNAME_HEK(our_stash);
-
+       assert(canon_name);
        if (hv_common(isa, NULL, HEK_KEY(canon_name), HEK_LEN(canon_name),
                      HEK_FLAGS(canon_name),
                      HV_FETCH_ISEXISTS, NULL, HEK_HASH(canon_name))) {
diff --git a/util.c b/util.c
index ad007b9..42926b3 100644 (file)
--- a/util.c
+++ b/util.c
@@ -850,15 +850,17 @@ Perl_fbm_instr(pTHX_ unsigned char *big, unsigned char *bigend, SV *littlestr, U
 
     {
        const MAGIC *const mg = mg_find(littlestr, PERL_MAGIC_bm);
-       const unsigned char * const table = (const unsigned char *) mg->mg_ptr;
        const unsigned char *oldlittle;
 
+       assert(mg);
+
        --littlelen;                    /* Last char found by table lookup */
 
        s = big + littlelen;
        little += littlelen;            /* last char */
        oldlittle = little;
        if (s < bigend) {
+           const unsigned char * const table = (const unsigned char *) mg->mg_ptr;
            I32 tmp;
 
          top2: