| Message ID | 1448869662-23191-1-git-send-email-hs@denx.de |
|---|---|
| State | Accepted |
| Commit | 1d48ca69e5cb0538b0cae8fa651d2cb2eb97b2cb |
| Delegated to: | Tom Rini |
| Headers | show |
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@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. Regards, Jeroen
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@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? bye, Heiko
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@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.
Hello Heiko, On 30-11-15 11:03, Heiko Schocher wrote: > > 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); >> >> 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, guessing, order of the passes inside the compiler matter, if the inline is done first, and thereafter the sanity check is done, the latter can't detect an uninitialized variable is passed, since it has already been removed. If you do it the other way around, it will warn first since it won't know the latter pass will remove it. If I have to guess, I think that is what is happening. [ of course it can be made a bit smarter, by e.g. looking at the __attribute__((unused)), but more guessing, I think gcc5 just ignores that one ]. Even more funny is the fact that these functions are not macro's to _prevent_ compiler warnings ;) So cc-ing Albert since he wrote it. " /* * Redefine all original macros with static inline functions containing * a simple memory barrier, so that they produce the same instruction * ordering constraints as their original counterparts. * We use static inline functions rather than macros so that we can tell * the compiler to not complain about unused arguments. */ " If we can find a version which doesn't pose other requirements then the linux versions (like flags must be initialized) _and_ they don't warn as well, I guess we can agree that would be preferred. I am not sure to what extend though, some options below to modify local_irq_save itself to prevent the warning.... 4 and 5 are warning free, not sure it is a brilliant thing to do though .... Albert? Regards, Jeroen -------------------------------------------------------------------------------------------- // triggers: "is used uninitialized in this function" with gcc5 static inline void my_argument_maybe_unused1(int flags __attribute__((unused))) { } // triggers -Wunused-variable #define my_argument_maybe_unused2(flags) do {} while(0) // triggers set but not used #define my_argument_maybe_unused3(flags) do { flags = 0; } while(0) // so, lets use flags instead, but clearly unused so it gets optimized out later ;) // no warnings with gcc 5, clang 3.4.... #define my_argument_maybe_unused4(flags) do { int dummy __attribute__((unused)) = sizeof(flags); } while(0) // combined preprocessor / compiler foodoo static inline void _my_argument_maybe_unused5(int *flags __attribute__((unused))) { } #define my_argument_maybe_unused5(flags) do { _my_argument_maybe_unused5(&flags); } while(0) int main() { int i_am_used_in_linux_not_in_uboot1; int i_am_used_in_linux_not_in_uboot2; int i_am_used_in_linux_not_in_uboot3; int i_am_used_in_linux_not_in_uboot4; int i_am_used_in_linux_not_in_uboot5; my_argument_maybe_unused1(i_am_used_in_linux_not_in_uboot1); my_argument_maybe_unused2(i_am_used_in_linux_not_in_uboot2); my_argument_maybe_unused3(i_am_used_in_linux_not_in_uboot3); my_argument_maybe_unused4(i_am_used_in_linux_not_in_uboot4); my_argument_maybe_unused5(i_am_used_in_linux_not_in_uboot5); return 0; } /* arm-linux-gnueabihf-gcc (Linaro GCC 5.1-2015.08) 5.1.1 20150608 arm-linux-gnueabihf-gcc -Wall -O2 warnings.c good, no code generated.. 000102c0 <main>: 102c0: 2000 movs r0, #0 102c2: 4770 bx lr host, clang... guess this fine too, no idea what the nops are though... 00000000004004f0 <main>: 4004f0: 31 c0 xor %eax,%eax 4004f2: c3 retq 4004f3: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) 4004fa: 00 00 00 4004fd: 0f 1f 00 nopl (%rax) */
Hello Tom, On Mon, 30 Nov 2015 11:28:53 -0500, Tom Rini <trini@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@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? > Tom Amicalement,
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@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@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.
On Mon, Nov 30, 2015 at 08:47:42AM +0100, 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@denx.de> I've re-thought this problem. I'm not seeing a better way to work around this problem without further divergence from upstream on these functions, so thanks for doing this! Applied to u-boot/master, thanks!
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) static inline void atomic_sub(int i, volatile atomic_t *v) { - unsigned long flags; + unsigned long flags = 0; local_irq_save(flags); v->counter -= i; @@ -50,7 +50,7 @@ static inline void atomic_sub(int i, volatile atomic_t *v) static inline void atomic_inc(volatile atomic_t *v) { - unsigned long flags; + unsigned long flags = 0; local_irq_save(flags); v->counter += 1; @@ -59,7 +59,7 @@ static inline void atomic_inc(volatile atomic_t *v) static inline void atomic_dec(volatile atomic_t *v) { - unsigned long flags; + unsigned long flags = 0; local_irq_save(flags); v->counter -= 1; @@ -68,7 +68,7 @@ static inline void atomic_dec(volatile atomic_t *v) static inline int atomic_dec_and_test(volatile atomic_t *v) { - unsigned long flags; + unsigned long flags = 0; int val; local_irq_save(flags); @@ -81,7 +81,7 @@ static inline int atomic_dec_and_test(volatile atomic_t *v) static inline int atomic_add_negative(int i, volatile atomic_t *v) { - unsigned long flags; + unsigned long flags = 0; int val; local_irq_save(flags); @@ -94,7 +94,7 @@ static inline int atomic_add_negative(int i, volatile atomic_t *v) static inline void atomic_clear_mask(unsigned long mask, unsigned long *addr) { - unsigned long flags; + unsigned long flags = 0; local_irq_save(flags); *addr &= ~mask; diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h index d479a38..f33efeb 100644 --- a/arch/arm/include/asm/bitops.h +++ b/arch/arm/include/asm/bitops.h @@ -51,7 +51,7 @@ static inline int __test_and_set_bit(int nr, volatile void *addr) static inline int test_and_set_bit(int nr, volatile void * addr) { - unsigned long flags; + unsigned long flags = 0; int out; local_irq_save(flags); @@ -73,7 +73,7 @@ static inline int __test_and_clear_bit(int nr, volatile void *addr) static inline int test_and_clear_bit(int nr, volatile void * addr) { - unsigned long flags; + unsigned long flags = 0; int out; local_irq_save(flags);
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@denx.de> --- arch/arm/include/asm/atomic.h | 14 +++++++------- arch/arm/include/asm/bitops.h | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-)