[Mesa-dev] [PATCH] glcpp: Fix attempts to expand recursive macros infinitely (bug #32835).

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/15/2011 12:03 PM, Carl Worth wrote:
> The 095-recursive-define test case was triggering infinite recursion
> with the following test case:
> 
> 	#define A(a, b) B(a, b)
> 	#define C A(0, C)
> 	C
> 
> Here's what as happening:
> 
>   1. "C" was pushed onto the active list to expand the C node
> 
>   2. While expanding the "0" argument, the active list would be
>      emptied by the code at the end of _glcpp_parser_expand_token_list
> 
>   3. When expanding the "C" argument, the active list was now empty,
>      so lather, rinse, repeat.
> 
> We fix this by adjusting the final popping at the end of
> _glcpp_parser_expand_token_list to never pop more nodes then this
> particular invocation had pushed itself. This is as simple as saving
> the original state of the active list, and then interrupting the
> popping when we reach this same state.
> 
> With this fix, all of the glcpp-test tests now pass.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=32835
Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> ---
> 
> As mentioned above, I've tested with the glcpp-test tests. But I
> haven't run piglit with this change. If someone could do that while
> reviewing the patch, it would be most appreciated.
> 
>  src/glsl/glcpp/glcpp-parse.y |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
> index fdc9467..6f15e85 100644
> --- a/src/glsl/glcpp/glcpp-parse.y
> +++ b/src/glsl/glcpp/glcpp-parse.y
> @@ -1563,6 +1563,7 @@ _glcpp_parser_expand_token_list (glcpp_parser_t *parser,
>  	token_node_t *node_prev;
>  	token_node_t *node, *last = NULL;
>  	token_list_t *expansion;
> +	active_list_t *active_initial = parser->active;
>  
>  	if (list == NULL)
>  		return;
> @@ -1617,7 +1618,10 @@ _glcpp_parser_expand_token_list (glcpp_parser_t *parser,
>  		node = node_prev ? node_prev->next : list->head;
>  	}
>  
> -	while (parser->active)
> +	/* Remove any lingering effects of this invocation on the
> +	 * active list. That is, pop until the list looks like it did
> +	 * at the beginning of this function. */
> +	while (parser->active && parser->active != active_initial)
>  		_parser_active_list_pop (parser);
>  
>  	list->non_space_tail = list->tail;

The funny thing is that I looked at this code for a long time.  I was
very confused about why there was all this effort to put a marker in the
stack only to ignore it later.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk2o2awACgkQX1gOwKyEAw9cxwCgkerjv0Vm3JBMtVq9ifWMWmGn
YyIAnRL1S+T2aHmMPW69qeeEHoN1IeLb
=Xtle
-----END PGP SIGNATURE-----