[PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))

Bernd Schmidt bschmidt@redhat.com
Mon Nov 16 21:34:00 GMT 2015


On 11/16/2015 09:50 PM, David Malcolm wrote:
> The root cause is uninitialized data.  Specifically, the C parser's
> struct c_expr gained a "src_range" field, and it turns out there are a
> few places where I wasn't initializing this when returning c_expr
> instances on the stack, and in some cases the values could get used.

> I'm working on a followup to fix the remaining places I identified via
> review of the source.

The patch is mostly OK IMO and should be installed to fix the problems, 
but I think there are a few more things to consider.

Should c_expr perhaps acquire a constructor so that this problem is 
avoided in the future? The whole thing seems somewhat error-prone.

> @@ -4278,9 +4278,11 @@ c_parser_braced_init (c_parser *parser, tree type, bool nested_p)
>         obstack_free (&braced_init_obstack, NULL);
>         return ret;
>       }
> +  location_t close_loc = c_parser_peek_token (parser)->location;

It looks like we're peeking the token twice here (via a 
c_parser_token_is_not call above the quoted code). Probably not too 
expensive but maybe we can avoid it.

>   	case RID_VA_ARG:
> -	  c_parser_consume_token (parser);
> +	  {
> +	    location_t start_loc = loc;

Does this really have to be indented in an extra braced block? Please 
fix if not.


Bernd



More information about the Gcc-patches mailing list