| Message ID | 5723516A.2020606@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> writes: > Hi all, > > I'm getting a warning when building genrecog that 'label' may be used > uninitialised in: > > uint64_t label = 0; > > if (d->test.kind == rtx_test::CODE > && d->if_statement_p (&label) > && label == CONST_INT) > > This is because if_statement_p looks like this: > inline bool > decision::if_statement_p (uint64_t *label) const > { > if (singleton () && first->labels.length () == 1) > { > if (label) > *label = first->labels[0]; > return true; > } > return false; > } > > It's not guaranteed to write label. It is guaranteed to write to label on a true return though, so it looks like a false positive. Is current GCC warning for this or are you using an older host compiler? Thanks, Richard
On 05/02/2016 03:47 AM, Richard Sandiford wrote: > Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> writes: >> Hi all, >> >> I'm getting a warning when building genrecog that 'label' may be used >> uninitialised in: >> >> uint64_t label = 0; >> >> if (d->test.kind == rtx_test::CODE >> && d->if_statement_p (&label) >> && label == CONST_INT) >> >> This is because if_statement_p looks like this: >> inline bool >> decision::if_statement_p (uint64_t *label) const >> { >> if (singleton () && first->labels.length () == 1) >> { >> if (label) >> *label = first->labels[0]; >> return true; >> } >> return false; >> } >> >> It's not guaranteed to write label. > > It is guaranteed to write to label on a true return though, so it looks > like a false positive. Is current GCC warning for this or are you using > an older host compiler? And if it's warning with the current trunk, we should open a BZ with a suitable testcase. DOM/VRP should have detected this and removed the path with the uninitialized use from the CFG, if it didn't then it's a bug worth filing. jeff
On 02/05/16 16:15, Jeff Law wrote: > On 05/02/2016 03:47 AM, Richard Sandiford wrote: >> Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> writes: >>> Hi all, >>> >>> I'm getting a warning when building genrecog that 'label' may be used >>> uninitialised in: >>> >>> uint64_t label = 0; >>> >>> if (d->test.kind == rtx_test::CODE >>> && d->if_statement_p (&label) >>> && label == CONST_INT) >>> >>> This is because if_statement_p looks like this: >>> inline bool >>> decision::if_statement_p (uint64_t *label) const >>> { >>> if (singleton () && first->labels.length () == 1) >>> { >>> if (label) >>> *label = first->labels[0]; >>> return true; >>> } >>> return false; >>> } >>> >>> It's not guaranteed to write label. >> >> It is guaranteed to write to label on a true return though, so it looks >> like a false positive. Is current GCC warning for this or are you using >> an older host compiler? This was after I upgraded my host compiler to GCC 6.1. I saw it with GCC 5.3 as well. > And if it's warning with the current trunk, we should open a BZ with a suitable testcase. DOM/VRP should have detected this and removed the path with the uninitialized use from the CFG, if it didn't then it's a bug worth filing. > After experimenting a bit, I note that the warning goes away when I compile with -O2. In the cross compiler build I'm doing genrecog.c is compiler with -O1, which exhibits the warning. So I suppose DOM/VRP does catch, but only at the appropriate optimisation level. Kyrill > jeff
On 05/03/2016 10:28 AM, Kyrill Tkachov wrote: > After experimenting a bit, I note that the warning goes away when I > compile with -O2. > In the cross compiler build I'm doing genrecog.c is compiler with -O1, > which exhibits the warning. > So I suppose DOM/VRP does catch, but only at the appropriate > optimisation level. That makes sense. Thanks for digging further into this. I think this should be considered a non-issue. Jeff
diff --git a/gcc/genrecog.c b/gcc/genrecog.c index 47e42660fcc854e5da3eba4bee2bb4b06a7352b1..0e62c61a8a756766c12138b51498229f442f44d0 100644 --- a/gcc/genrecog.c +++ b/gcc/genrecog.c @@ -1583,7 +1583,7 @@ simplify_tests (state *s) { for (decision *d = s->first; d; d = d->next) { - uint64_t label; + uint64_t label = 0; /* Convert checks for GET_CODE (x) == CONST_INT and XWINT (x, 0) == N into checks for const_int_rtx[N'], if N is suitably small. */ if (d->test.kind == rtx_test::CODE
Hi all, I'm getting a warning when building genrecog that 'label' may be used uninitialised in: uint64_t label = 0; if (d->test.kind == rtx_test::CODE && d->if_statement_p (&label) && label == CONST_INT) This is because if_statement_p looks like this: inline bool decision::if_statement_p (uint64_t *label) const { if (singleton () && first->labels.length () == 1) { if (label) *label = first->labels[0]; return true; } return false; } It's not guaranteed to write label. This patch initialises label to 0 to fix the warning. Is this the right thing to do? Bootstrapped and tested on aarch64-none-linux-gnu and arm-none-linux-gnueabihf. Thanks, Kyrill 2016-04-29 Kyrylo Tkachov <kyrylo.tkachov@arm.com> * genrecog.c (simplify_tests): Initialize label to 0.