[U-Boot] [PATCH] arm, ubifs: fix gcc5.x compiler warning

[Tom Rini]

On Tue, Dec 01, 2015 at 08:56:54AM +0100, Albert ARIBAUD wrote:
> Hello Tom,
> 
> On Mon, 30 Nov 2015 11:28:53 -0500, Tom Rini <trini at konsulko.com> wrote:
> > On Mon, Nov 30, 2015 at 11:03:53AM +0100, Heiko Schocher wrote:
> > > Hello Jeroen,
> > > 
> > > Am 30.11.2015 um 10:20 schrieb Jeroen Hofstee:
> > > >Hello Heiko,
> > > >
> > > >On 30-11-15 08:47, Heiko Schocher wrote:
> > > >>compiling U-Boot for openrd_base_defconfig with
> > > >>gcc 5.x shows the following warning:
> > > >>
> > > >>   CC      fs/ubifs/super.o
> > > >>In file included from fs/ubifs/ubifs.h:35:0,
> > > >>                  from fs/ubifs/super.c:37:
> > > >>fs/ubifs/super.c: In function 'atomic_inc':
> > > >>./arch/arm/include/asm/atomic.h:55:2: warning: 'flags' is used uninitialized in this function
> > > >>[-Wuninitialized]
> > > >>   local_irq_save(flags);
> > > >>   ^
> > > >>fs/ubifs/super.c: In function 'atomic_dec':
> > > >>./arch/arm/include/asm/atomic.h:64:2: warning: 'flags' is used uninitialized in this function
> > > >>[-Wuninitialized]
> > > >>   local_irq_save(flags);
> > > >>   ^
> > > >>   CC      fs/ubifs/sb.o
> > > >>[...]
> > > >>   CC      fs/ubifs/lpt.o
> > > >>In file included from include/linux/bitops.h:123:0,
> > > >>                  from include/common.h:20,
> > > >>                  from include/ubi_uboot.h:17,
> > > >>                  from fs/ubifs/ubifs.h:37,
> > > >>                  from fs/ubifs/lpt.c:35:
> > > >>fs/ubifs/lpt.c: In function 'test_and_set_bit':
> > > >>./arch/arm/include/asm/bitops.h:57:2: warning: 'flags' is used uninitialized in this function
> > > >>[-Wuninitialized]
> > > >>   local_irq_save(flags);
> > > >>   ^
> > > >>   CC      fs/ubifs/lpt_commit.o
> > > >>In file included from include/linux/bitops.h:123:0,
> > > >>                  from include/common.h:20,
> > > >>                  from include/ubi_uboot.h:17,
> > > >>                  from fs/ubifs/ubifs.h:37,
> > > >>                  from fs/ubifs/lpt_commit.c:26:
> > > >>fs/ubifs/lpt_commit.c: In function 'test_and_set_bit':
> > > >>./arch/arm/include/asm/bitops.h:57:2: warning: 'flags' is used uninitialized in this function
> > > >>[-Wuninitialized]
> > > >>   local_irq_save(flags);
> > > >>   ^
> > > >>   CC      fs/ubifs/scan.o
> > > >>   CC      fs/ubifs/lprops.o
> > > >>   CC      fs/ubifs/tnc.o
> > > >>In file included from include/linux/bitops.h:123:0,
> > > >>                  from include/common.h:20,
> > > >>                  from include/ubi_uboot.h:17,
> > > >>                  from fs/ubifs/ubifs.h:37,
> > > >>                  from fs/ubifs/tnc.c:30:
> > > >>fs/ubifs/tnc.c: In function 'test_and_set_bit':
> > > >>./arch/arm/include/asm/bitops.h:57:2: warning: 'flags' is used uninitialized in this function
> > > >>[-Wuninitialized]
> > > >>   local_irq_save(flags);
> > > >>   ^
> > > >>   CC      fs/ubifs/tnc_misc.o
> > > >>
> > > >>Fix it.
> > > >>
> > > >>Signed-off-by: Heiko Schocher <hs at denx.de>
> > > >>---
> > > >>
> > > >>  arch/arm/include/asm/atomic.h | 14 +++++++-------
> > > >>  arch/arm/include/asm/bitops.h |  4 ++--
> > > >>  2 files changed, 9 insertions(+), 9 deletions(-)
> > > >>
> > > >>diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h
> > > >>index 34c07fe..9b79506 100644
> > > >>--- a/arch/arm/include/asm/atomic.h
> > > >>+++ b/arch/arm/include/asm/atomic.h
> > > >>@@ -32,7 +32,7 @@ typedef struct { volatile int counter; } atomic_t;
> > > >>  static inline void atomic_add(int i, volatile atomic_t *v)
> > > >>  {
> > > >>-    unsigned long flags;
> > > >>+    unsigned long flags = 0;
> > > >>      local_irq_save(flags);
> > > >>      v->counter += i;
> > > >>@@ -41,7 +41,7 @@ static inline void atomic_add(int i, volatile atomic_t *v)
> > > >
> > > >Since flags is an "out" argument, something else must be wrong.
> > > >There should be no need to initialize it, since local_irq_save should
> > > >do that afaik.
> > > 
> > > yes, you are right, it should be, but gcc 5.x seems to have problems
> > > with it ... compiled code size for the openrd_base config is same with
> > > my patch ...
> > > 
> > > Hmm... for the openrd_base compile local_irq_save() is used from:
> > > arch/arm/thumb1/include/asm/proc-armv/system.h
> > > 
> > > with:
> > > static inline void local_irq_save(
> > >         unsigned long flags __attribute__((unused)))
> > > {
> > >         __asm__ __volatile__ ("" : : : "memory");
> > > }
> > > 
> > > flasg marked as unused ... seems correct to me, but I have
> > > no idea, why gcc 5.x prints a warning ... any ideas?
> > 
> > Well, gcc does get more vigerous in its checking now and yeah, it feels
> > like it's flagging false positives.   In this case I think the answer is
> > that we need to nop out the various calls a bit harder on ARM.  Glancing
> > at the kernel, I think for thumb1 we should just do what we do for
> > non-thumb, or translate that into thumb1 only code.
> 
> Not sure I'm following what you mean, both about nop-ing out and about
> thumb-1. Can you clarify?

With respect to nop-ing out, we don't do interrupts, which is why the
thumb-1 version of local_irq_save(), etc, are not incorrect.  But the
normal case of these macros, in U-Boot, is not nop-ing them out, but
having the linux kernel version of the macro.

With respect to thumb-1, we need to have those real macros around (and
maybe re-structure things slightly again to easily share that code) as
gcc-5.x is not happy with what we're doing here and warning.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151202/10075cac/attachment.sig>