This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] add some -Werror pragmas for tilegx
- From: Chris Metcalf <cmetcalf at ezchip dot com>
- To: Paul Eggert <eggert at cs dot ucla dot edu>, GLIBC Devel <libc-alpha at sourceware dot org>
- Date: Mon, 29 Dec 2014 10:28:13 -0500
- Subject: Re: [PATCH] add some -Werror pragmas for tilegx
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp dot mailfrom=cmetcalf at ezchip dot com;
- References: <549DF701 dot 606 at ezchip dot com> <549E0204 dot 80605 at cs dot ucla dot edu>
On 12/26/2014 7:49 PM, Paul Eggert wrote:
Chris Metcalf wrote:
+ /* Using the tilegx gcc 4.8.2 yields uninitialized warnings
+ here (on the "else if" line) and below. Compiling the same
+ source code with the same gcc version on x86_64 does not
+ yield the warning, so it is something subtle. */
+ DIAG_PUSH_NEEDS_COMMENT;
+ DIAG_IGNORE_NEEDS_COMMENT(4.8, "-Wmaybe-uninitialized");
}
else if (br_elem->type == COLL_SYM)
{
+ DIAG_POP_NEEDS_COMMENT;
Can you fix this in a way that doesn't feel like ifdefs that are improperly nested with respect to the actual code? E.g., by moving the "br_elem->type == COLL_SYM" expression into a static function, and playing with the pragmas only inside that function's body?
Unfortunately, no; restructuring in that way just means that the call site to the static function is tagged as a maybe-uninitialized warning instead.
Noodling around in the code a bit more for the two warnings, the common issue is the call to parse_bracket_element, which takes a bracket_elem_t pointer and fills in the object properly, setting both the union type indicator ("type") and the appropriate member of the enclosed union ("char ch", "wchar_t wch", or "char *name", basically). However, the code currently relies on a fragile-seeming test where if one of the other arguments to the function is a token with a particular type, it calls on down to parse_bracket_symbol, which assumes that the "name" field is valid, even though the "type" has never been set.
It turns out that just doing what feels like the more robust thing, namely setting the type to "COLL_SYM" when we set the symbol value to "name", eliminates the warnings. I can't imagine there is any noticeable performance issue associated with this, and it makes the warning go away.
Any objections to committing this fix?
diff --git a/posix/regcomp.c b/posix/regcomp.c
index 897fe276a3fa..ca438c8cf8c8 100644
--- a/posix/regcomp.c
+++ b/posix/regcomp.c
@@ -3114,6 +3114,7 @@ parse_bracket_exp (re_string_t *regexp, re_dfa_t *dfa, re_token_t *token,
re_token_t token2;
start_elem.opr.name = start_name_buf;
+ start_elem.type = COLL_SYM;
ret = parse_bracket_element (&start_elem, regexp, token, token_len, dfa,
syntax, first_round);
if (BE (ret != REG_NOERROR, 0))
@@ -3157,6 +3158,7 @@ parse_bracket_exp (re_string_t *regexp, re_dfa_t *dfa, re_token_t *token,
if (is_range_exp == 1)
{
end_elem.opr.name = end_name_buf;
+ end_elem.type = COLL_SYM;
ret = parse_bracket_element (&end_elem, regexp, &token2, token_len2,
dfa, syntax, 1);
if (BE (ret != REG_NOERROR, 0))
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com