Re: LPEG strange code

[Roberto Ierusalimschy <roberto@...>]

> I'm trying to port lpeg 1.0.1 to another scripting language and
> doing so I'm going through almost all lpeg code and of course I'm
> making several mistakes while porting.
> 
> But on one of many segfaults I've got I stumbled at this code in
> lptree.c (the order of the functions was altered here to show then
> in call order), it seems that this code was a copy/paste leftover or
> some code from trying different approaches and forgotten here.
> 
> It seems a kind of weird assert !
> 
> Thanks for your time, attention and great work !
> 
> ...
> 
> /*
> ** Create a new 'ktable' to the pattern at the top of the stack, adding
> ** all elements from pattern 'p' (if not 0) plus element 'idx' to it.
> ** Return index of new element.
> */
> static int addtonewktable (lua_State *L, int p, int idx) {
>   newktable(L, 1);
>   if (p)
>     mergektable(L, p, NULL); ////// <<<<<< Here we are calling
> mergektable with a NULL !!!!!
>   return addtoktable(L, idx);
> }
> 
> ...
> 
> /*
> ** merge 'ktable' from 'stree' at stack index 'idx' into 'ktable'
> ** from tree at the top of the stack, and correct corresponding
> ** tree.
> */
> static void mergektable (lua_State *L, int idx, TTree *stree) { ////
> <<< when called with stree=NULL !!!!
>   int n;
>   lua_getuservalue(L, -1);  /* get ktables */
>   lua_getuservalue(L, idx);
>   n = concattable(L, -1, -2);
>   lua_pop(L, 2);  /* remove both ktables */
>   correctkeys(stree, n); ///<<<<< Here we'll segfault !!!!!!
> }
> ...
> 
> /*
> ** When joining 'ktables', constants from one of the subpatterns must
> ** be renumbered; 'correctkeys' corrects their indices (adding 'n'
> ** to each of them)
> */
> static void correctkeys (TTree *tree, int n) { ///<<<< When tree is
> NULL and n is not zero
>   if (n == 0) return;  /* no correction? */
>  tailcall:
>   switch (tree->tag) { ///<<<<<<   Here we'll segfault !!!!!!!
> <<<<<<< !!!!!!!!!<<<<<<<< !!!!
>     case TOpenCall: case TCall: case TRunTime: case TRule: {
>       if (tree->key > 0)
>         tree->key += n;
>       break;
>     }
>     case TCapture: {
>       if (tree->key > 0 && tree->cap != Carg && tree->cap != Cnum)
>         tree->key += n;
>       break;
>     }
>     default: break;
>   }
>   switch (numsiblings[tree->tag]) {
>     case 1:  /* correctkeys(sib1(tree), n); */
>       tree = sib1(tree); goto tailcall;
>     case 2:
>       correctkeys(sib1(tree), n);
>       tree = sib2(tree); goto tailcall;  /* correctkeys(sib2(tree), n); */
>     default: assert(numsiblings[tree->tag] == 0); break;
>   }
> }
> 

I don't think the case you describe can occur:

1) addtonewktable will call mergektable with a tree at the top of the stack
that has a new ktable (due to the call to newktable).

2) mergektable will call concattable with that tree as its first tree
argument (-1 is the top of the stack).

3) concattable (not listed) returns 0 when the ktable of its first tree
is empty. As this ktable is a new one (see 1), it is empty, so
concattable will return 0.

4) Back in mergektable, n will be 0, so the condition in correctkeys
"When tree is NULL and n is not zero" will never occur.

-- Roberto