This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))
- From: Bernd Schmidt <bschmidt at redhat dot com>
- To: David Malcolm <dmalcolm at redhat dot com>, David Edelsohn <dje dot gcc at gmail dot com>
- Cc: Jeffrey Law <law at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Biener <richard dot guenther at gmail dot com>, Dodji Seketeli <dodji at redhat dot com>
- Date: Mon, 16 Nov 2015 22:34:14 +0100
- Subject: Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))
- Authentication-results: sourceware.org; auth=none
- References: <CAGWvnynj92T8T_U8P7x1SPyQpiW=_1jtUWhMxSn+aiVo+Ffkbw at mail dot gmail dot com> <1447561939 dot 19594 dot 27 dot camel at surprise> <1447707000 dot 19594 dot 64 dot camel at surprise>
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