Bug 74762

Summary: [9/10/11 Regression] missing uninitialized warning (C++, parenthesized expr, TREE_NO_WARNING)
Product: gcc Reporter: Martin Liška <marxin>
Component: c++Assignee: Martin Sebor <msebor>
Status: RESOLVED FIXED    
Severity: normal CC: daniel.kruegler, dimhen, egallager, jakub, manu, msebor, webrown.cpp
Priority: P2 Keywords: diagnostic, patch
Version: 7.0   
Target Milestone: 12.0   
Host: Target:
Build: Known to work: 4.6.4
Known to fail: 10.2.0, 11.1.0, 7.3.0, 8.3.0, 9.2.0 Last reconfirmed: 2021-04-16 00:00:00
Bug Depends on:    
Bug Blocks: 24639, 99251    

Description Martin Liška 2016-08-12 11:29:55 UTC
$ cat tc2.c
struct tree2;
struct tree_vector2
{
  tree2 *elts[1];
};
struct tree2
{
  struct
  {
    tree_vector2 vector;
  } u;
};
int
const_with_all_bytes_same (tree2 *val)
{
  int i;
  const_with_all_bytes_same ((val->u.vector.elts[i]));

  return 1;
}

$ g++ tc2.c -Wuninitialized -c
(empty)

$ clang++ tc2.c -Wuninitialized -c
clang-3.8: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
tc2.c:17:50: warning: variable 'i' is uninitialized when used here [-Wuninitialized]
  const_with_all_bytes_same ((val->u.vector.elts[i]));
                                                 ^
tc2.c:16:8: note: initialize the variable 'i' to silence this warning
  int i;
       ^
        = 0
1 warning generated.

The test-case used to be catch in 4.6.x era.
Comment 1 Patrick Palka 2016-08-12 12:23:21 UTC
Looks like warn_uninit() suppresses the warning on 'i' because the TREE_NO_WARNING flag is set on the ARRAY_REF by the C++ FE (in finish_parenthesized_expr(), for an unrelated purpose).
Comment 2 Manuel López-Ibáñez 2016-08-12 12:37:59 UTC
A simpler testcase without infinite recursion:

struct tree2;
struct tree_vector2
{
  tree2 *elts[1];
};
struct tree2
{
  struct
  {
    tree_vector2 vector;
  } u;
};
tree2 *
const_with_all_bytes_same (tree2 *val)
{
  int i;
  return ((val->u.vector.elts[i]));
}

And I would say this is a C++ problem, but unfortunately, it is a problem throughout GCC of abusing TREE_NO_WARNING.
Comment 3 Jakub Jelinek 2016-08-12 13:37:50 UTC
Started most likely with r177667, r177670 doesn't warn, r177661 does.
Comment 4 Jakub Jelinek 2016-08-12 13:39:00 UTC
Started most likely with r177667, r177670 doesn't warn, r177661 does.
Comment 5 Manuel López-Ibáñez 2016-08-12 14:58:36 UTC
(In reply to Patrick Palka from comment #1)
> Looks like warn_uninit() suppresses the warning on 'i' because the
> TREE_NO_WARNING flag is set on the ARRAY_REF by the C++ FE (in
> finish_parenthesized_expr(), for an unrelated purpose).

I wonder if a quick work-around for this case in particular is to check for:

  if (TREE_CODE (expr) == MODIFY_EXPR)

like the C parser does:

	  /* A parenthesized expression.  */
	  location_t loc_open_paren = c_parser_peek_token (parser)->location;
	  c_parser_consume_token (parser);
	  expr = c_parser_expression (parser);
	  if (TREE_CODE (expr.value) == MODIFY_EXPR)
	    TREE_NO_WARNING (expr.value) = 1;
	  if (expr.original_code != C_MAYBE_CONST_EXPR)
	    expr.original_code = ERROR_MARK;
	  /* Don't change EXPR.ORIGINAL_TYPE.  */
	  location_t loc_close_paren = c_parser_peek_token (parser)->location;
	  set_c_expr_source_range (&expr, loc_open_paren, loc_close_paren);
	  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
				     "expected %<)%>");

Then the bug will only trigger for code such as ((lhs=rhs)), which is what TREE_NO_WARNING is trying to avoid warning for.
Comment 6 Patrick Palka 2016-08-13 23:44:23 UTC
(In reply to Manuel López-Ibáñez from comment #5)
> (In reply to Patrick Palka from comment #1)
> > Looks like warn_uninit() suppresses the warning on 'i' because the
> > TREE_NO_WARNING flag is set on the ARRAY_REF by the C++ FE (in
> > finish_parenthesized_expr(), for an unrelated purpose).
> 
> I wonder if a quick work-around for this case in particular is to check for:
> 
>   if (TREE_CODE (expr) == MODIFY_EXPR)
> 
> like the C parser does:
> 
> 	  /* A parenthesized expression.  */
> 	  location_t loc_open_paren = c_parser_peek_token (parser)->location;
> 	  c_parser_consume_token (parser);
> 	  expr = c_parser_expression (parser);
> 	  if (TREE_CODE (expr.value) == MODIFY_EXPR)
> 	    TREE_NO_WARNING (expr.value) = 1;
> 	  if (expr.original_code != C_MAYBE_CONST_EXPR)
> 	    expr.original_code = ERROR_MARK;
> 	  /* Don't change EXPR.ORIGINAL_TYPE.  */
> 	  location_t loc_close_paren = c_parser_peek_token (parser)->location;
> 	  set_c_expr_source_range (&expr, loc_open_paren, loc_close_paren);
> 	  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
> 				     "expected %<)%>");
> 
> Then the bug will only trigger for code such as ((lhs=rhs)), which is what
> TREE_NO_WARNING is trying to avoid warning for.

Unfortunately it looks like the purpose of setting TREE_NO_WARNING in finish_parenthesized_expr() is not limited to assignments within conditionals, it's also important for the -Wparentheses warning about confusing precedence (e.g. warn for "a + b << c" but not for "a + (b << c)")
Comment 7 Jakub Jelinek 2016-11-11 11:01:16 UTC
We really need fine grained TREE_NO_WARNING, will see if I manage to implement something for stage3.
Comment 8 Jakub Jelinek 2017-10-10 13:25:14 UTC
GCC 5 branch is being closed
Comment 9 Martin Sebor 2018-01-15 21:31:53 UTC
*** Bug 82609 has been marked as a duplicate of this bug. ***
Comment 10 Martin Sebor 2018-01-15 21:45:51 UTC
Author: msebor
Date: Mon Jan 15 21:45:06 2018
New Revision: 256709

URL: https://gcc.gnu.org/viewcvs?rev=256709&root=gcc&view=rev
Log:
PR testsuite/83869 - c-c++-common/attr-nonstring-3.c fails starting with r256683

testsuite/CHangeLog:
	* c-c++-common/attr-nonstring-3.c: Work around bug c++/74762.


Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/attr-nonstring-3.c
Comment 11 Martin Sebor 2018-03-29 15:16:05 UTC
*** Bug 85119 has been marked as a duplicate of this bug. ***
Comment 12 Jakub Jelinek 2018-10-26 10:07:58 UTC
GCC 6 branch is being closed
Comment 13 Richard Biener 2019-11-14 07:49:09 UTC
The GCC 7 branch is being closed, re-targeting to GCC 8.4.
Comment 14 Jakub Jelinek 2020-03-04 09:40:44 UTC
GCC 8.4.0 has been released, adjusting target milestone.
Comment 15 Martin Sebor 2021-04-16 19:44:08 UTC
No change in GCC 11.
Comment 16 Jakub Jelinek 2021-05-14 09:48:24 UTC
GCC 8 branch is being closed.
Comment 18 Richard Biener 2021-06-01 08:08:05 UTC
GCC 9.4 is being released, retargeting bugs to GCC 9.5.
Comment 19 Eric Gallager 2021-06-26 15:15:18 UTC
does the TREE_NO_WARNING overhaul that Martin Sebor did affect this?
Comment 20 Martin Sebor 2021-06-28 20:13:03 UTC
Yes, this was indeed resolved by r12-1804 and related.  One of the last patches in the series (yet to be committed) adds a test for this to the test suite:
https://gcc.gnu.org/pipermail/gcc-patches/2021-June/571984.html
Comment 21 GCC Commits 2021-06-28 21:20:37 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

https://gcc.gnu.org/g:40c64c9ea565230817f08b5e66a30a1c94ec880c

commit r12-1861-g40c64c9ea565230817f08b5e66a30a1c94ec880c
Author: Martin Sebor <msebor@redhat.com>
Date:   Mon Jun 28 15:14:50 2021 -0600

    Regression tests for TREE_NO_WARNING enhancement to warning groups [PR74765, PR74762].
    
    PR middle-end/74765 - missing uninitialized warning (parenthesis, TREE_NO_WARNING abuse)
    PR middle-end/74762 - [9/10/11/12 Regression] missing uninitialized warning (C++, parenthesized expr, TREE_NO_WARNING)
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/uninit-pr74762.C: New test.
            * g++.dg/warn/uninit-pr74765.C: Same.