[PATCH] Possible use before def in fortran/trans-decl.c.

[PATCH] Possible use before def in fortran/trans-decl.c.

[Andrew MacLeod]

[-- Attachment #1: Type: text/plain, Size: 2768 bytes --]

As I'm tweaking installing ranger as the VRP2 pass, I am getting a stage 
2 bootstrap failure now:

In file included from 
/opt/notnfs/amacleod/master/gcc/gcc/fortran/trans-decl.c:28:
/opt/notnfs/amacleod/master/gcc/gcc/tree.h: In function ‘void 
gfc_conv_cfi_to_gfc(stmtblock_t*, stmtblock_t*, tree, tree, gfc_symbol*)’:
/opt/notnfs/amacleod/master/gcc/gcc/tree.h:244:56: error: ‘rank’ may be 
used uninitialized in this function [-Werror=maybe-uninitialized]
   244 | #define TREE_CODE(NODE) ((enum tree_code) (NODE)->base.code)
       | ^~~~
/opt/notnfs/amacleod/master/gcc/gcc/fortran/trans-decl.c:6671:8: note: 
‘rank’ was declared here
  6671 |   tree rank, idx, etype, tmp, tmp2, size_var = NULL_TREE;
       |        ^~~~
cc1plus: all warnings being treated as errors
make[3]: *** [Makefile:1136: fortran/trans-decl.o] Error 1


looking at that function, in the middle I see:

   if (sym->as->rank < 0)
     {
       /* Set gfc->dtype.rank, if assumed-rank.  */
       rank = gfc_get_cfi_desc_rank (cfi);
       gfc_add_modify (&block, gfc_conv_descriptor_rank (gfc_desc), rank);
     }
   else if (!GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (gfc_desc)))
     /* In that case, the CFI rank and the declared rank can differ.  */
     rank = gfc_get_cfi_desc_rank (cfi);
   else
     rank = build_int_cst (signed_char_type_node, sym->as->rank);


so rank is set on all paths here.   However, stepping back a bit, 
earlier in the function I see:

   if (!sym->attr.dimension || !GFC_DESCRIPTOR_TYPE_P (TREE_TYPE 
(gfc_desc)))
     {
       tmp = gfc_get_cfi_desc_base_addr (cfi);
       gfc_add_modify (&block, gfc_desc,
                       fold_convert (TREE_TYPE (gfc_desc), tmp));
       if (!sym->attr.dimension)
         goto done;
     }

The done: label occurs *after* that block of initialization code, and 
bit furtehr down , I see this:

           gfc_add_modify (&loop_body, tmpidx, idx);
           stmtblock_t inner_loop;
           gfc_init_block (&inner_loop);
           tree dim = gfc_create_var (TREE_TYPE (rank), "dim");

I cannot convince myself by looking at the intervening code that this 
can not be executed along this path.  Perhaps someone more familiar with 
the code can check it out.   However, It seems worthwhile to at least 
initialize rank to NULL_TREE, thus we can be more likely to see a trap 
if that path ever gets followed.

And it makes the warning go away :-)

OK?

Andrew

PS as a side note, it would be handy to have the def point *and* the use 
point that might be undefined.   Its a big function and it took me a 
while just to see where a possible use might be.





[-- Attachment #2: for.diff --]
[-- Type: text/x-patch, Size: 867 bytes --]

commit ed571a93c54e3967fbf445624e47817be5e333ed
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Thu Oct 21 14:48:20 2021 -0400

    Initialize variable.
    
            gcc/fortran/
            * trans-decl.c (gfc_conv_cfi_to_gfc): Initialize rank to NULL_TREE.

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index de624c82fcf..fe5511b5285 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -6668,7 +6668,7 @@ gfc_conv_cfi_to_gfc (stmtblock_t *init, stmtblock_t *finally,
   stmtblock_t block;
   gfc_init_block (&block);
   tree cfi = build_fold_indirect_ref_loc (input_location, cfi_desc);
-  tree rank, idx, etype, tmp, tmp2, size_var = NULL_TREE;
+  tree idx, etype, tmp, tmp2, size_var = NULL_TREE, rank = NULL_TREE;
   bool do_copy_inout = false;
 
   /* When allocatable + intent out, free the cfi descriptor.  */

Re: [PATCH] Possible use before def in fortran/trans-decl.c.

[Martin Sebor]

On 10/21/21 1:02 PM, Andrew MacLeod via Gcc-patches wrote:
> As I'm tweaking installing ranger as the VRP2 pass, I am getting a stage 
> 2 bootstrap failure now:
> 
> In file included from 
> /opt/notnfs/amacleod/master/gcc/gcc/fortran/trans-decl.c:28:
> /opt/notnfs/amacleod/master/gcc/gcc/tree.h: In function ‘void 
> gfc_conv_cfi_to_gfc(stmtblock_t*, stmtblock_t*, tree, tree, gfc_symbol*)’:
> /opt/notnfs/amacleod/master/gcc/gcc/tree.h:244:56: error: ‘rank’ may be 
> used uninitialized in this function [-Werror=maybe-uninitialized]
>    244 | #define TREE_CODE(NODE) ((enum tree_code) (NODE)->base.code)
>        | ^~~~
> /opt/notnfs/amacleod/master/gcc/gcc/fortran/trans-decl.c:6671:8: note: 
> ‘rank’ was declared here
>   6671 |   tree rank, idx, etype, tmp, tmp2, size_var = NULL_TREE;
>        |        ^~~~
> cc1plus: all warnings being treated as errors
> make[3]: *** [Makefile:1136: fortran/trans-decl.o] Error 1
> 
> 
> looking at that function, in the middle I see:
> 
>    if (sym->as->rank < 0)
>      {
>        /* Set gfc->dtype.rank, if assumed-rank.  */
>        rank = gfc_get_cfi_desc_rank (cfi);
>        gfc_add_modify (&block, gfc_conv_descriptor_rank (gfc_desc), rank);
>      }
>    else if (!GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (gfc_desc)))
>      /* In that case, the CFI rank and the declared rank can differ.  */
>      rank = gfc_get_cfi_desc_rank (cfi);
>    else
>      rank = build_int_cst (signed_char_type_node, sym->as->rank);
> 
> 
> so rank is set on all paths here.   However, stepping back a bit, 
> earlier in the function I see:
> 
>    if (!sym->attr.dimension || !GFC_DESCRIPTOR_TYPE_P (TREE_TYPE 
> (gfc_desc)))
>      {
>        tmp = gfc_get_cfi_desc_base_addr (cfi);
>        gfc_add_modify (&block, gfc_desc,
>                        fold_convert (TREE_TYPE (gfc_desc), tmp));
>        if (!sym->attr.dimension)
>          goto done;
>      }
> 
> The done: label occurs *after* that block of initialization code, and 
> bit furtehr down , I see this:
> 
>            gfc_add_modify (&loop_body, tmpidx, idx);
>            stmtblock_t inner_loop;
>            gfc_init_block (&inner_loop);
>            tree dim = gfc_create_var (TREE_TYPE (rank), "dim");
> 
> I cannot convince myself by looking at the intervening code that this 
> can not be executed along this path.  Perhaps someone more familiar with 
> the code can check it out.   However, It seems worthwhile to at least 
> initialize rank to NULL_TREE, thus we can be more likely to see a trap 
> if that path ever gets followed.
> 
> And it makes the warning go away :-)
> 
> OK?

Initializing variables on declaration is commonly recommended
as a best C/C++ etc. programming practice.  If it silences
a warning and makes the code more readable, who could possibly
say no? ;)

> 
> Andrew
> 
> PS as a side note, it would be handy to have the def point *and* the use 
> point that might be undefined.   Its a big function and it took me a 
> while just to see where a possible use might be.

The use point should be the what the warning points to.  In
the case above it's the result of macro expansion so it less
than helpfully points to the macro definition. I would expect
it to also point to its expansion like in other warnings.
Something must be interfering with it.  I've opened pr102887
to remind us to look into it.

Martin

[COMMITTED] Re: [PATCH] Possible use before def in fortran/trans-decl.c.

[Andrew MacLeod]

[-- Attachment #1: Type: text/plain, Size: 2990 bytes --]

On 10/21/21 3:02 PM, Andrew MacLeod wrote:
> As I'm tweaking installing ranger as the VRP2 pass, I am getting a 
> stage 2 bootstrap failure now:
>
> In file included from 
> /opt/notnfs/amacleod/master/gcc/gcc/fortran/trans-decl.c:28:
> /opt/notnfs/amacleod/master/gcc/gcc/tree.h: In function ‘void 
> gfc_conv_cfi_to_gfc(stmtblock_t*, stmtblock_t*, tree, tree, 
> gfc_symbol*)’:
> /opt/notnfs/amacleod/master/gcc/gcc/tree.h:244:56: error: ‘rank’ may 
> be used uninitialized in this function [-Werror=maybe-uninitialized]
>   244 | #define TREE_CODE(NODE) ((enum tree_code) (NODE)->base.code)
>       | ^~~~
> /opt/notnfs/amacleod/master/gcc/gcc/fortran/trans-decl.c:6671:8: note: 
> ‘rank’ was declared here
>  6671 |   tree rank, idx, etype, tmp, tmp2, size_var = NULL_TREE;
>       |        ^~~~
> cc1plus: all warnings being treated as errors
> make[3]: *** [Makefile:1136: fortran/trans-decl.o] Error 1
>
>
> looking at that function, in the middle I see:
>
>   if (sym->as->rank < 0)
>     {
>       /* Set gfc->dtype.rank, if assumed-rank.  */
>       rank = gfc_get_cfi_desc_rank (cfi);
>       gfc_add_modify (&block, gfc_conv_descriptor_rank (gfc_desc), rank);
>     }
>   else if (!GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (gfc_desc)))
>     /* In that case, the CFI rank and the declared rank can differ.  */
>     rank = gfc_get_cfi_desc_rank (cfi);
>   else
>     rank = build_int_cst (signed_char_type_node, sym->as->rank);
>
>
> so rank is set on all paths here.   However, stepping back a bit, 
> earlier in the function I see:
>
>   if (!sym->attr.dimension || !GFC_DESCRIPTOR_TYPE_P (TREE_TYPE 
> (gfc_desc)))
>     {
>       tmp = gfc_get_cfi_desc_base_addr (cfi);
>       gfc_add_modify (&block, gfc_desc,
>                       fold_convert (TREE_TYPE (gfc_desc), tmp));
>       if (!sym->attr.dimension)
>         goto done;
>     }
>
> The done: label occurs *after* that block of initialization code, and 
> bit furtehr down , I see this:
>
>           gfc_add_modify (&loop_body, tmpidx, idx);
>           stmtblock_t inner_loop;
>           gfc_init_block (&inner_loop);
>           tree dim = gfc_create_var (TREE_TYPE (rank), "dim");
>
> I cannot convince myself by looking at the intervening code that this 
> can not be executed along this path.  Perhaps someone more familiar 
> with the code can check it out.   However, It seems worthwhile to at 
> least initialize rank to NULL_TREE, thus we can be more likely to see 
> a trap if that path ever gets followed.
>
> And it makes the warning go away :-)
>
> OK?
>
> Andrew
>
> PS as a side note, it would be handy to have the def point *and* the 
> use point that might be undefined.   Its a big function and it took me 
> a while just to see where a possible use might be.
>
>
>
>
Bootstraps onx86_64-pc-linux-gnu with no regressions.  Pushed.

Andrew


[-- Attachment #2: 0003-Initialize-variable.patch --]
[-- Type: text/x-patch, Size: 973 bytes --]

From 387c665392366a543fb29badaee329533b32abb3 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Thu, 21 Oct 2021 14:48:20 -0400
Subject: [PATCH 3/3] Initialize variable.

	gcc/fortran/
	* trans-decl.c (gfc_conv_cfi_to_gfc): Initialize rank to NULL_TREE.
---
 gcc/fortran/trans-decl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index de624c82fcf..fe5511b5285 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -6668,7 +6668,7 @@ gfc_conv_cfi_to_gfc (stmtblock_t *init, stmtblock_t *finally,
   stmtblock_t block;
   gfc_init_block (&block);
   tree cfi = build_fold_indirect_ref_loc (input_location, cfi_desc);
-  tree rank, idx, etype, tmp, tmp2, size_var = NULL_TREE;
+  tree idx, etype, tmp, tmp2, size_var = NULL_TREE, rank = NULL_TREE;
   bool do_copy_inout = false;
 
   /* When allocatable + intent out, free the cfi descriptor.  */
-- 
2.17.2