This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] add some -Werror pragmas for tilegx


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]