| Message ID | 20201103015228.2250547-2-kuhn.chenqun@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | fix uninitialized variable warning | expand |
On 11/3/20 2:52 AM, Chen Qun wrote: > The compiler cannot determine whether the return values of the xtensa_operand_is_register(isa, opc, opnd) > and xtensa_operand_is_visible(isa, opc, opnd) functions are the same. > So,it assumes that 'rf' is not assigned, but it's used. > > The compiler showed warning: > target/xtensa/translate.c: In function ‘disas_xtensa_insn’: > target/xtensa/translate.c:985:43: warning: ‘rf’ may be used uninitialized in this function [-Wmaybe-uninitialized] > 985 | arg[vopnd].num_bits = xtensa_regfile_num_bits(isa, rf); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Add a default value for 'rf' to prevented the warning. > > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> > --- > Cc: Max Filippov <jcmvbkbc@gmail.com> > --- > target/xtensa/translate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c > index 944a157747..eea851bbe7 100644 > --- a/target/xtensa/translate.c > +++ b/target/xtensa/translate.c > @@ -953,7 +953,7 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc) > > for (opnd = vopnd = 0; opnd < opnds; ++opnd) { > void **register_file = NULL; > - xtensa_regfile rf; > + xtensa_regfile rf = -1; NAck (code smells). Deferring to Max, but possible fix: -- >8 -- @@ -953,10 +953,9 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc) for (opnd = vopnd = 0; opnd < opnds; ++opnd) { void **register_file = NULL; - xtensa_regfile rf; + xtensa_regfile rf = xtensa_operand_regfile(isa, opc, opnd); if (xtensa_operand_is_register(isa, opc, opnd)) { - rf = xtensa_operand_regfile(isa, opc, opnd); register_file = dc->config->regfile[rf]; if (rf == dc->config->a_regfile) { ---
On Mon, Nov 2, 2020 at 5:52 PM Chen Qun <kuhn.chenqun@huawei.com> wrote: > > The compiler cannot determine whether the return values of the xtensa_operand_is_register(isa, opc, opnd) > and xtensa_operand_is_visible(isa, opc, opnd) functions are the same. It doesn't have to because 1) they definitely are not the same, but 2) it doesn't matter. > So,it assumes that 'rf' is not assigned, but it's used. The assumption is wrong. rf is used under the 'if (register_file)' condition and register_file is initialized to NULL and then set to something non-NULL based on the value of rf here: 958 if (xtensa_operand_is_register(isa, opc, opnd)) { 959 rf = xtensa_operand_regfile(isa, opc, opnd); 960 register_file = dc->config->regfile[rf]; > The compiler showed warning: > target/xtensa/translate.c: In function ‘disas_xtensa_insn’: > target/xtensa/translate.c:985:43: warning: ‘rf’ may be used uninitialized in this function [-Wmaybe-uninitialized] > 985 | arg[vopnd].num_bits = xtensa_regfile_num_bits(isa, rf); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Add a default value for 'rf' to prevented the warning. I don't see it doing default build with gcc 8.3. But then I don't see -Wmaybe-uninitialized in the compiler command line either. > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> > --- > Cc: Max Filippov <jcmvbkbc@gmail.com> > --- > target/xtensa/translate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c > index 944a157747..eea851bbe7 100644 > --- a/target/xtensa/translate.c > +++ b/target/xtensa/translate.c > @@ -953,7 +953,7 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc) > > for (opnd = vopnd = 0; opnd < opnds; ++opnd) { > void **register_file = NULL; > - xtensa_regfile rf; > + xtensa_regfile rf = -1; Please use XTENSA_UNDEFINED instead if you still think this is worth changing.
> -----Original Message----- > From: Max Filippov [mailto:jcmvbkbc@gmail.com] > Sent: Tuesday, November 3, 2020 5:22 PM > To: Chenqun (kuhn) <kuhn.chenqun@huawei.com> > Cc: qemu-devel <qemu-devel@nongnu.org>; QEMU Trivial > <qemu-trivial@nongnu.org>; Zhanghailiang > <zhang.zhanghailiang@huawei.com>; ganqixin <ganqixin@huawei.com>; Euler > Robot <euler.robot@huawei.com> > Subject: Re: [PATCH 1/6] target/xtensa: fix uninitialized variable warning > > On Mon, Nov 2, 2020 at 5:52 PM Chen Qun <kuhn.chenqun@huawei.com> > wrote: > > > > The compiler cannot determine whether the return values of the > > xtensa_operand_is_register(isa, opc, opnd) and > xtensa_operand_is_visible(isa, opc, opnd) functions are the same. > > It doesn't have to because 1) they definitely are not the same, but > 2) it doesn't matter. > > > So,it assumes that 'rf' is not assigned, but it's used. > > The assumption is wrong. rf is used under the 'if (register_file)' > condition and register_file is initialized to NULL and then set to something > non-NULL based on the value of rf here: > Hi Max, Yeah, your analysis is correct. This rf is used only when register_file is non-NULL. When this condition is met, the rf must have been assigned a value. The GCC 9.3 compilation I use contains the Wmaybe-uninitialized parameter by default. It cannot recognize this complex logic judgment. This warning may be frequently encountered by developers who compile this part of code. > 958 if (xtensa_operand_is_register(isa, opc, opnd)) { > 959 rf = xtensa_operand_regfile(isa, opc, opnd); > 960 register_file = dc->config->regfile[rf]; > > > The compiler showed warning: > > target/xtensa/translate.c: In function ‘disas_xtensa_insn’: > > target/xtensa/translate.c:985:43: warning: ‘rf’ may be used uninitialized in > this function [-Wmaybe-uninitialized] > > 985 | arg[vopnd].num_bits = > xtensa_regfile_num_bits(isa, rf); > > | > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > Add a default value for 'rf' to prevented the warning. > > I don't see it doing default build with gcc 8.3. But then I don't see > -Wmaybe-uninitialized in the compiler command line either. > Maybe it's available after GCC9, or some CFLAG configuration. The -Wmaybe-uninitialized parameter has this description： "These warnings are only possible in optimizing compilation, because otherwise GCC does not keep track of the state of variables." From：https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#Warning-Options I have tried to configure only "-O2 -fexceptions" for the CFLAG on GCC9, and this warning will occur. > > Reported-by: Euler Robot <euler.robot@huawei.com> > > Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> > > --- > > Cc: Max Filippov <jcmvbkbc@gmail.com> > > --- > > target/xtensa/translate.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c > > index 944a157747..eea851bbe7 100644 > > --- a/target/xtensa/translate.c > > +++ b/target/xtensa/translate.c > > @@ -953,7 +953,7 @@ static void disas_xtensa_insn(CPUXtensaState *env, > > DisasContext *dc) > > > > for (opnd = vopnd = 0; opnd < opnds; ++opnd) { > > void **register_file = NULL; > > - xtensa_regfile rf; > > + xtensa_regfile rf = -1; > > Please use XTENSA_UNDEFINED instead if you still think this is worth changing. > I don't think it's wrong, it's just a bit of trouble for the compiler :) Thanks, Chen Qun
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c index 944a157747..eea851bbe7 100644 --- a/target/xtensa/translate.c +++ b/target/xtensa/translate.c @@ -953,7 +953,7 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc) for (opnd = vopnd = 0; opnd < opnds; ++opnd) { void **register_file = NULL; - xtensa_regfile rf; + xtensa_regfile rf = -1; if (xtensa_operand_is_register(isa, opc, opnd)) { rf = xtensa_operand_regfile(isa, opc, opnd);
The compiler cannot determine whether the return values of the xtensa_operand_is_register(isa, opc, opnd) and xtensa_operand_is_visible(isa, opc, opnd) functions are the same. So,it assumes that 'rf' is not assigned, but it's used. The compiler showed warning: target/xtensa/translate.c: In function ‘disas_xtensa_insn’: target/xtensa/translate.c:985:43: warning: ‘rf’ may be used uninitialized in this function [-Wmaybe-uninitialized] 985 | arg[vopnd].num_bits = xtensa_regfile_num_bits(isa, rf); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Add a default value for 'rf' to prevented the warning. Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> --- Cc: Max Filippov <jcmvbkbc@gmail.com> --- target/xtensa/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)