diff mbox series

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

Message ID c5355e12-dfef-b4ab-d2e5-23d066b0d419@redhat.com
State New
Headers show
Series [COMMITTED] Re: [PATCH] Possible use before def in fortran/trans-decl.c. | expand

Commit Message

Andrew MacLeod Oct. 25, 2021, 2:45 p.m. UTC 
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
diff mbox series

Patch

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