develooper Front page | perl.perl5.porters | Postings from April 2006

Last 2 uninspected Coverity bugs are in regexec.c

Thread Next
From:
Nicholas Clark
Date:
April 16, 2006 12:23
Subject:
Last 2 uninspected Coverity bugs are in regexec.c
Message ID:
20060416192254.GL32132@plum.flirble.org
Coverity is spotting 2 possible problems in regexec.c.

Firstly, it sees the potential for an assignment of NULL to next:

  2589		if (next == scan)
  2590		    next = NULL;
  2591	
  2592		switch (OP(scan)) {

and for the right (well, wrong) value of scan you can get here:

  3527		case CURLYX: {
  3528			/* No need to save/restore up to this paren */
  3529			I32 parenfloor = scan->flags;
  3530	
  3531			if (OP(PREVOPER(next)) == NOTHING) /* LONGJMP */
  3532			    next += ARG(next);

where OP() will dereference next. Which it assumes could be NULL.

Secondly it sees this NULL assignment:

  2516	    st->cc = NULL;
  2517	    /* Note that nextchr is a byte even in UTF */
  2518	    nextchr = UCHARAT(locinput);
  2519	    scan = prog;
  2520	    while (scan != NULL) {

and that you can end up here:

  3560		case WHILEM: {
  3561			/*
  3562			 * This is really hard to understand, because after we match
  3563			 * what we're trying to match, we must make sure the rest of
  3564			 * the REx is going to match for sure, and to do that we have
  3565			 * to go back UP the parse tree by recursing ever deeper.  And
  3566			 * if it fails, we have to reset our parent's current state
  3567			 * that we can try again after backing off.
  3568			 */
  3569	
  3570			st->u.whilem.lastloc = st->cc->u.curlyx.lastloc; /* Detection of 0-len. */


Although for some reason it chooses a route via this NULL assignment:

  2588		next = scan + NEXT_OFF(scan);
  2589		if (next == scan)
  2590		    next = NULL;


and through this case statement

  3437		case IFTHEN:
  3438		    PL_reg_leftiter = PL_reg_maxiter;		/* Void cache */
  3439		    if (st->sw)
  3440			next = NEXTOPER(NEXTOPER(scan));

this else block

  3441		    else {
  3442			next = scan + ARG(scan);

and into this if.

  3443			if (OP(next) == IFTHEN) /* Fake one. */
  3444			    next = NEXTOPER(NEXTOPER(next));
  3445		    }

and went round the main switch statement a second time to get there.


The regexp code is a bit convoluted. Is it onto something, or is it merely
confused?

Nicholas Clark

PS Coverity sees each of these bugs twice, because the same code is in the
   re module. It's also not resynced perl for a day or two, because quite a
   few of the marked bugs are now fixed.

Thread Next


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