linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kernel/bounds: Provide prototype for foo
@ 2018-09-21 14:22 Kieran Bingham
  2018-09-21 14:45 ` Greg Kroah-Hartman
  2018-10-05  8:33 ` [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning Arnd Bergmann
  0 siblings, 2 replies; 17+ messages in thread
From: Kieran Bingham @ 2018-09-21 14:22 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-renesas-soc, Kieran Bingham, stable, Thomas Gleixner,
	Kate Stewart, Greg Kroah-Hartman, Philippe Ombredanne, open list

kernel/bounds.c is recompiled on every build, and shows the following
warning when compiling with W=1:

  CC      kernel/bounds.s
linux/kernel/bounds.c:16:6: warning: no previous prototype for ‘foo’ [-Wmissing-prototypes]
 void foo(void)
      ^~~

Provide a prototype to satisfy the compiler.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: stable@vger.kernel.org

---
I compile all of my incremental builds with W=1, which allows me to know
instantly if I add a new compiler warning in code I generate.

This warning always comes up and seems trivial to clean up.
---
 kernel/bounds.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/bounds.c b/kernel/bounds.c
index c373e887c066..60136d937800 100644
--- a/kernel/bounds.c
+++ b/kernel/bounds.c
@@ -13,6 +13,8 @@
 #include <linux/log2.h>
 #include <linux/spinlock_types.h>
 
+void foo(void);
+
 void foo(void)
 {
 	/* The enum constants to put into include/generated/bounds.h */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] kernel/bounds: Provide prototype for foo
  2018-09-21 14:22 [PATCH] kernel/bounds: Provide prototype for foo Kieran Bingham
@ 2018-09-21 14:45 ` Greg Kroah-Hartman
  2018-09-21 15:58   ` Kieran Bingham
  2018-10-05  8:33 ` [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning Arnd Bergmann
  1 sibling, 1 reply; 17+ messages in thread
From: Greg Kroah-Hartman @ 2018-09-21 14:45 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: Andrew Morton, linux-renesas-soc, stable, Thomas Gleixner,
	Kate Stewart, Philippe Ombredanne, open list

On Fri, Sep 21, 2018 at 03:22:33PM +0100, Kieran Bingham wrote:
> kernel/bounds.c is recompiled on every build, and shows the following
> warning when compiling with W=1:

Don't do that, you will get a lot of warnings that really don't make
much sense.  Like this one :)

> 
>   CC      kernel/bounds.s
> linux/kernel/bounds.c:16:6: warning: no previous prototype for ‘foo’ [-Wmissing-prototypes]
>  void foo(void)
>       ^~~
> 
> Provide a prototype to satisfy the compiler.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: stable@vger.kernel.org
> 
> ---
> I compile all of my incremental builds with W=1, which allows me to know
> instantly if I add a new compiler warning in code I generate.
> 
> This warning always comes up and seems trivial to clean up.
> ---
>  kernel/bounds.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/bounds.c b/kernel/bounds.c
> index c373e887c066..60136d937800 100644
> --- a/kernel/bounds.c
> +++ b/kernel/bounds.c
> @@ -13,6 +13,8 @@
>  #include <linux/log2.h>
>  #include <linux/spinlock_types.h>
>  
> +void foo(void);
> +
>  void foo(void)

This file is a userspace tool that is used to later generate the
include/generated/bounds.h file.

If you really want to track this down and fix it properly, put the
prototype in the .c file that ends up calling this function.  That's a
fun task to dig through the build system to find :)

good luck!

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] kernel/bounds: Provide prototype for foo
  2018-09-21 14:45 ` Greg Kroah-Hartman
@ 2018-09-21 15:58   ` Kieran Bingham
  2018-09-21 16:03     ` Kieran Bingham
  0 siblings, 1 reply; 17+ messages in thread
From: Kieran Bingham @ 2018-09-21 15:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Andrew Morton, linux-renesas-soc, stable, Thomas Gleixner,
	Kate Stewart, Philippe Ombredanne, open list

Hi Greg,

Thank you for quick response!

On 21/09/18 15:45, Greg Kroah-Hartman wrote:
> On Fri, Sep 21, 2018 at 03:22:33PM +0100, Kieran Bingham wrote:
>> kernel/bounds.c is recompiled on every build, and shows the following
>> warning when compiling with W=1:

So it turns out after a bit more checking that my statement above was a
small lie :)

My local build scripts were *causing* kernel/bounds.s to be rebuilt on
every build, which is why this stood out to me - because of two
competing compiles for the kernel image, and the dtb.

One with W=1 and the other without... (kbuild detected different flags,
and thus rebuilt the common objects)

And that's why I saw this warning on every build... and thought this was
a hot-path.


> Don't do that, you will get a lot of warnings that really don't make
> much sense.  Like this one :)


I know - but I can ignore all on my first build, then on incremental
builds where only files I change are compiled - it's much quieter :)

I see it as a benefit to compile *my* code with a higher warning level,
to prevent /me/ adding further warnings.


I realise of course this patch is just pandering to the compiler to shut
it up, as this is essentially an 'unused' dummy function from it's
perspective.

But in this instance, it's because the output is being compiled to an
assembly output (kernel/bounds.s) which is then later parsed, so there
isn't anywhere else to define the prototype, and the object code is only
referenced from the assembly output.


>>
>>   CC      kernel/bounds.s
>> linux/kernel/bounds.c:16:6: warning: no previous prototype for ‘foo’ [-Wmissing-prototypes]
>>  void foo(void)
>>       ^~~
>>
>> Provide a prototype to satisfy the compiler.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> Cc: stable@vger.kernel.org
>>
>> ---
>> I compile all of my incremental builds with W=1, which allows me to know
>> instantly if I add a new compiler warning in code I generate.
>>
>> This warning always comes up and seems trivial to clean up.
>> ---
>>  kernel/bounds.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/kernel/bounds.c b/kernel/bounds.c
>> index c373e887c066..60136d937800 100644
>> --- a/kernel/bounds.c
>> +++ b/kernel/bounds.c
>> @@ -13,6 +13,8 @@
>>  #include <linux/log2.h>
>>  #include <linux/spinlock_types.h>
>>  
>> +void foo(void);
>> +
>>  void foo(void)
> 
> This file is a userspace tool that is used to later generate the
> include/generated/bounds.h file.

Well more accurately it is a file compiled directly to assembly which is
then later parsed to help generate the bounds.h file. It's not itself a
userspace tool, nothing executes this code...

It's just a compilation object to allow utilisation of the preprocessor
and compiler in ways that couldn't be done otherwise as far as I
understand it.


> If you really want to track this down and fix it properly, put the
> prototype in the .c file that ends up calling this function.  That's a
> fun task to dig through the build system to find :)

This is the only location.

The compilation output from V=1 (https://paste.debian.net/1043598/ for
the full output) shows the command generating this warning as:

  (with many flags redacted for readability)

aarch64-linux-gnu-gcc -Wp,-MD,kernel/.bounds.s.d  -nostdinc -Wall
-Wstrict-prototypes -DKBUILD_BASENAME='"bounds"'
-DKBUILD_MODNAME='"bounds"'  -fverbose-asm -S -o kernel/bounds.s
kernel/bounds.c



I still feel this patch has 'some' merit with the inaccurate leading
statement regarding this being output on every build removed from the
commit log.


What do you think ?

Worth a v2 with commit message fixed?
Or should I just drop this ?


> good luck!
> 
> greg k-h


--
Cheers

Kieran

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] kernel/bounds: Provide prototype for foo
  2018-09-21 15:58   ` Kieran Bingham
@ 2018-09-21 16:03     ` Kieran Bingham
  0 siblings, 0 replies; 17+ messages in thread
From: Kieran Bingham @ 2018-09-21 16:03 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Andrew Morton, linux-renesas-soc, stable, Thomas Gleixner,
	Kate Stewart, Philippe Ombredanne, open list

On 21/09/18 16:58, Kieran Bingham wrote:
> Hi Greg,
> 
> Thank you for quick response!

Ahem; s/for quick/for your quick/

I hate re-reading mails after I hit send and spotting things like this.
Sigh :-)

Regards
--
Kieran

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-09-21 14:22 [PATCH] kernel/bounds: Provide prototype for foo Kieran Bingham
  2018-09-21 14:45 ` Greg Kroah-Hartman
@ 2018-10-05  8:33 ` Arnd Bergmann
  2018-10-05  8:47   ` Kieran Bingham
                     ` (3 more replies)
  1 sibling, 4 replies; 17+ messages in thread
From: Arnd Bergmann @ 2018-10-05  8:33 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-renesas-soc, Kieran Bingham, stable, linux-kernel,
	linux-kbuild, yamada.masahiro, Arnd Bergmann

Building any configuration with 'make W=1' produces a warning:

kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]

When also passing -Werror, this prevents us from building any
other files. Nobody ever calls the function, but we can't make
it 'static' either since we want the compiler output.

Calling it 'main' instead however avoids the warning, because gcc
does not insist on having a declaration for main.

Reported-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: stable@vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I have run into this problem several times before, and thought I had
sent a fix at some point. Looking in the archives, I came across
the suggested fix from Kieran, so I'm following up on that here.
---
 kernel/bounds.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/bounds.c b/kernel/bounds.c
index c373e887c066..9795d75b09b2 100644
--- a/kernel/bounds.c
+++ b/kernel/bounds.c
@@ -13,7 +13,7 @@
 #include <linux/log2.h>
 #include <linux/spinlock_types.h>
 
-void foo(void)
+int main(void)
 {
 	/* The enum constants to put into include/generated/bounds.h */
 	DEFINE(NR_PAGEFLAGS, __NR_PAGEFLAGS);
@@ -23,4 +23,6 @@ void foo(void)
 #endif
 	DEFINE(SPINLOCK_SIZE, sizeof(spinlock_t));
 	/* End of constants */
+
+	return 0;
 }
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-05  8:33 ` [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning Arnd Bergmann
@ 2018-10-05  8:47   ` Kieran Bingham
  2018-10-05  8:52   ` David Laight
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Kieran Bingham @ 2018-10-05  8:47 UTC (permalink / raw)
  To: Arnd Bergmann, Andrew Morton
  Cc: linux-renesas-soc, stable, linux-kernel, linux-kbuild, yamada.masahiro

Hi Arnd,

On 05/10/18 09:33, Arnd Bergmann wrote:
> Building any configuration with 'make W=1' produces a warning:
> 
> kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]
> 

s/warnign/warning/ but I don't think that's too important :D

> When also passing -Werror, this prevents us from building any
> other files. Nobody ever calls the function, but we can't make
> it 'static' either since we want the compiler output.
> 
> Calling it 'main' instead however avoids the warning, because gcc
> does not insist on having a declaration for main.

Aha - even better! (and annoyingly fairly obvious, from the
Why-didn't-I-think-of-that department).

> Reported-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> ---
> I have run into this problem several times before, and thought I had
> sent a fix at some point. Looking in the archives, I came across
> the suggested fix from Kieran, so I'm following up on that here.
> ---
>  kernel/bounds.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bounds.c b/kernel/bounds.c
> index c373e887c066..9795d75b09b2 100644
> --- a/kernel/bounds.c
> +++ b/kernel/bounds.c
> @@ -13,7 +13,7 @@
>  #include <linux/log2.h>
>  #include <linux/spinlock_types.h>
>  
> -void foo(void)
> +int main(void)
>  {
>  	/* The enum constants to put into include/generated/bounds.h */
>  	DEFINE(NR_PAGEFLAGS, __NR_PAGEFLAGS);
> @@ -23,4 +23,6 @@ void foo(void)
>  #endif
>  	DEFINE(SPINLOCK_SIZE, sizeof(spinlock_t));
>  	/* End of constants */
> +
> +	return 0;
>  }
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-05  8:33 ` [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning Arnd Bergmann
  2018-10-05  8:47   ` Kieran Bingham
@ 2018-10-05  8:52   ` David Laight
  2018-10-05  9:07     ` Arnd Bergmann
  2018-10-06 20:31   ` Masahiro Yamada
  2018-10-06 21:18   ` Miguel Ojeda
  3 siblings, 1 reply; 17+ messages in thread
From: David Laight @ 2018-10-05  8:52 UTC (permalink / raw)
  To: 'Arnd Bergmann', Andrew Morton
  Cc: linux-renesas-soc, Kieran Bingham, stable, linux-kernel,
	linux-kbuild, yamada.masahiro

From: Arnd Bergmann
> Sent: 05 October 2018 09:33
> 
> Building any configuration with 'make W=1' produces a warning:
> 
> kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]
> 
> When also passing -Werror, this prevents us from building any
> other files. Nobody ever calls the function, but we can't make
> it 'static' either since we want the compiler output.
> 
> Calling it 'main' instead however avoids the warning, because gcc
> does not insist on having a declaration for main.

Ugg.
main() might be special in other ways too.
It wouldn't surprise me if some linkers don't do special stuff for it.

What is wrong with just putting and extra "void foo(void);" before
the function?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-05  8:52   ` David Laight
@ 2018-10-05  9:07     ` Arnd Bergmann
  2018-10-05  9:27       ` Kieran Bingham
  0 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2018-10-05  9:07 UTC (permalink / raw)
  To: David Laight
  Cc: Andrew Morton, Linux-Renesas, kieran.bingham+renesas, # 3.4.x,
	Linux Kernel Mailing List, Linux Kbuild mailing list,
	Masahiro Yamada

On Fri, Oct 5, 2018 at 10:52 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Arnd Bergmann
> > Sent: 05 October 2018 09:33
> >
> > Building any configuration with 'make W=1' produces a warning:
> >
> > kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]
> >
> > When also passing -Werror, this prevents us from building any
> > other files. Nobody ever calls the function, but we can't make
> > it 'static' either since we want the compiler output.
> >
> > Calling it 'main' instead however avoids the warning, because gcc
> > does not insist on having a declaration for main.
>
> Ugg.
> main() might be special in other ways too.
> It wouldn't surprise me if some linkers don't do special stuff for it.
>
> What is wrong with just putting and extra "void foo(void);" before
> the function?

Greg objected to that on the basis that we don't want declarations
in .c files -- they should be in a shared header:

https://lkml.org/lkml/2018/9/21/735

I don't see what could go wrong here with calling it main(), after
all we are just interested in the assembler output, not even
creating an object file.

      Arnd

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-05  9:07     ` Arnd Bergmann
@ 2018-10-05  9:27       ` Kieran Bingham
  0 siblings, 0 replies; 17+ messages in thread
From: Kieran Bingham @ 2018-10-05  9:27 UTC (permalink / raw)
  To: Arnd Bergmann, David Laight
  Cc: Andrew Morton, Linux-Renesas, # 3.4.x, Linux Kernel Mailing List,
	Linux Kbuild mailing list, Masahiro Yamada

On 05/10/18 10:07, Arnd Bergmann wrote:
> On Fri, Oct 5, 2018 at 10:52 AM David Laight <David.Laight@aculab.com> wrote:
>>
>> From: Arnd Bergmann
>>> Sent: 05 October 2018 09:33
>>>
>>> Building any configuration with 'make W=1' produces a warning:
>>>
>>> kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]
>>>
>>> When also passing -Werror, this prevents us from building any
>>> other files. Nobody ever calls the function, but we can't make
>>> it 'static' either since we want the compiler output.
>>>
>>> Calling it 'main' instead however avoids the warning, because gcc
>>> does not insist on having a declaration for main.
>>
>> Ugg.
>> main() might be special in other ways too.
>> It wouldn't surprise me if some linkers don't do special stuff for it.

I worried about this but didn't think it would be too much of an issue.
But perhaps we should check...

<compile bounds.s in both configurations> as bounds.s.foo and bounds.s.main:


 diff -Nurp bounds.s.*
--- bounds.s.foo        2018-10-05 10:20:53.269941404 +0100
+++ bounds.s.main       2018-10-05 10:20:31.375891260 +0100
@@ -108,11 +108,12 @@

        .global _mcount
 #NO_APP
+       .section        .text.startup,"ax",@progbits
        .align  2
        .p2align 3,,7
-       .global foo
-       .type   foo, %function
-foo:
+       .global main
+       .type   main, %function
+main:
        stp     x29, x30, [sp, -16]!    //,,,
        add     x29, sp, 0      //,,
 // /home/linuxembedded/iob/renesas/vsp1/sources/linux/kernel/bounds.c:17: {
@@ -139,10 +140,11 @@ foo:

 .ascii "->SPINLOCK_SIZE 56 sizeof(spinlock_t)" //
 // 0 "" 2
-// /home/linuxembedded/iob/renesas/vsp1/sources/linux/kernel/bounds.c:26: }
+// /home/linuxembedded/iob/renesas/vsp1/sources/linux/kernel/bounds.c:28: }
 #NO_APP
+       mov     w0, 0   //,
        ldp     x29, x30, [sp], 16      //,,,
        ret
-       .size   foo, .-foo
+       .size   main, .-main
        .ident  "GCC: (Ubuntu/Linaro 7.3.0-16ubuntu3) 7.3.0"
        .section        .note.GNU-stack,"",@progbits



compiled with aarch64-linux-gnu-gcc, and with no debug enabled.

Other than the entry point rename (and section name) and the return
value being added, I can't see anything problematic here.

And as far as I know - this file gets processed after to extract
definitions which should be independent. This file is not executed or
further compiled as far as I am aware.

--
Kieran



>>
>> What is wrong with just putting and extra "void foo(void);" before
>> the function?
> 
> Greg objected to that on the basis that we don't want declarations
> in .c files -- they should be in a shared header:
> 
> https://lkml.org/lkml/2018/9/21/735
> 
> I don't see what could go wrong here with calling it main(), after
> all we are just interested in the assembler output, not even
> creating an object file.
> 
>       Arnd
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-05  8:33 ` [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning Arnd Bergmann
  2018-10-05  8:47   ` Kieran Bingham
  2018-10-05  8:52   ` David Laight
@ 2018-10-06 20:31   ` Masahiro Yamada
  2018-10-06 21:18   ` Miguel Ojeda
  3 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2018-10-06 20:31 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, Linux-Renesas, kieran.bingham+renesas, stable,
	Linux Kernel Mailing List, Linux Kbuild mailing list

On Fri, Oct 5, 2018 at 5:34 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Building any configuration with 'make W=1' produces a warning:
>
> kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]
>
> When also passing -Werror, this prevents us from building any
> other files. Nobody ever calls the function, but we can't make
> it 'static' either since we want the compiler output.
>
> Calling it 'main' instead however avoids the warning, because gcc
> does not insist on having a declaration for main.
>
> Reported-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---


Applied to kbuild/fixes
with 's/warnign/warning/'

Thanks!




> I have run into this problem several times before, and thought I had
> sent a fix at some point. Looking in the archives, I came across
> the suggested fix from Kieran, so I'm following up on that here.
> ---
>  kernel/bounds.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bounds.c b/kernel/bounds.c
> index c373e887c066..9795d75b09b2 100644
> --- a/kernel/bounds.c
> +++ b/kernel/bounds.c
> @@ -13,7 +13,7 @@
>  #include <linux/log2.h>
>  #include <linux/spinlock_types.h>
>
> -void foo(void)
> +int main(void)
>  {
>         /* The enum constants to put into include/generated/bounds.h */
>         DEFINE(NR_PAGEFLAGS, __NR_PAGEFLAGS);
> @@ -23,4 +23,6 @@ void foo(void)
>  #endif
>         DEFINE(SPINLOCK_SIZE, sizeof(spinlock_t));
>         /* End of constants */
> +
> +       return 0;
>  }
> --
> 2.18.0
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-05  8:33 ` [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning Arnd Bergmann
                     ` (2 preceding siblings ...)
  2018-10-06 20:31   ` Masahiro Yamada
@ 2018-10-06 21:18   ` Miguel Ojeda
  2018-10-06 21:58     ` Masahiro Yamada
  3 siblings, 1 reply; 17+ messages in thread
From: Miguel Ojeda @ 2018-10-06 21:18 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, linux-renesas-soc, kieran.bingham+renesas, stable,
	linux-kernel, linux-kbuild, Masahiro Yamada

On Fri, Oct 5, 2018 at 10:35 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Building any configuration with 'make W=1' produces a warning:
>
> kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]
>
> When also passing -Werror, this prevents us from building any
> other files. Nobody ever calls the function, but we can't make
> it 'static' either since we want the compiler output.
>
> Calling it 'main' instead however avoids the warning, because gcc
> does not insist on having a declaration for main.

I think marking the function as static __used should do the trick and
would be less confusing.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-06 21:18   ` Miguel Ojeda
@ 2018-10-06 21:58     ` Masahiro Yamada
  2018-10-06 22:06       ` Masahiro Yamada
  2018-10-06 22:07       ` Miguel Ojeda
  0 siblings, 2 replies; 17+ messages in thread
From: Masahiro Yamada @ 2018-10-06 21:58 UTC (permalink / raw)
  To: miguel.ojeda.sandonis
  Cc: Arnd Bergmann, Andrew Morton, Linux-Renesas,
	kieran.bingham+renesas, stable, Linux Kernel Mailing List,
	Linux Kbuild mailing list

Hi Miguel,


On Sun, Oct 7, 2018 at 6:18 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Fri, Oct 5, 2018 at 10:35 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > Building any configuration with 'make W=1' produces a warning:
> >
> > kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]
> >
> > When also passing -Werror, this prevents us from building any
> > other files. Nobody ever calls the function, but we can't make
> > it 'static' either since we want the compiler output.
> >
> > Calling it 'main' instead however avoids the warning, because gcc
> > does not insist on having a declaration for main.
>
> I think marking the function as static __used should do the trick and
> would be less confusing.




I tried __used, but I still see the warning.


masahiro@grover:~/ref/linux$ git diff
diff --git a/kernel/bounds.c b/kernel/bounds.c
index c373e88..aee0101 100644
--- a/kernel/bounds.c
+++ b/kernel/bounds.c
@@ -13,7 +13,7 @@
 #include <linux/log2.h>
 #include <linux/spinlock_types.h>

-void foo(void)
+void __used foo(void)
 {
        /* The enum constants to put into include/generated/bounds.h */
        DEFINE(NR_PAGEFLAGS, __NR_PAGEFLAGS);
masahiro@grover:~/ref/linux$ make W=1  prepare
  CC      kernel/bounds.s
kernel/bounds.c:16:13: warning: no previous prototype for ‘foo’
[-Wmissing-prototypes]
 void __used foo(void)
             ^
  CC      arch/x86/kernel/asm-offsets.s





-- 
Best Regards
Masahiro Yamada

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-06 21:58     ` Masahiro Yamada
@ 2018-10-06 22:06       ` Masahiro Yamada
  2018-10-08 10:00         ` Kieran Bingham
  2018-10-06 22:07       ` Miguel Ojeda
  1 sibling, 1 reply; 17+ messages in thread
From: Masahiro Yamada @ 2018-10-06 22:06 UTC (permalink / raw)
  To: miguel.ojeda.sandonis
  Cc: Arnd Bergmann, Andrew Morton, Linux-Renesas,
	kieran.bingham+renesas, stable, Linux Kernel Mailing List,
	Linux Kbuild mailing list

On Sun, Oct 7, 2018 at 6:58 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> Hi Miguel,
>
>
> On Sun, Oct 7, 2018 at 6:18 AM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > On Fri, Oct 5, 2018 at 10:35 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > Building any configuration with 'make W=1' produces a warning:
> > >
> > > kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]
> > >
> > > When also passing -Werror, this prevents us from building any
> > > other files. Nobody ever calls the function, but we can't make
> > > it 'static' either since we want the compiler output.
> > >
> > > Calling it 'main' instead however avoids the warning, because gcc
> > > does not insist on having a declaration for main.
> >
> > I think marking the function as static __used should do the trick and
> > would be less confusing.
>
>
>
>
> I tried __used, but I still see the warning.
>
>
> masahiro@grover:~/ref/linux$ git diff
> diff --git a/kernel/bounds.c b/kernel/bounds.c
> index c373e88..aee0101 100644
> --- a/kernel/bounds.c
> +++ b/kernel/bounds.c
> @@ -13,7 +13,7 @@
>  #include <linux/log2.h>
>  #include <linux/spinlock_types.h>
>
> -void foo(void)
> +void __used foo(void)
>  {
>         /* The enum constants to put into include/generated/bounds.h */
>         DEFINE(NR_PAGEFLAGS, __NR_PAGEFLAGS);
> masahiro@grover:~/ref/linux$ make W=1  prepare
>   CC      kernel/bounds.s
> kernel/bounds.c:16:13: warning: no previous prototype for ‘foo’
> [-Wmissing-prototypes]
>  void __used foo(void)
>              ^
>   CC      arch/x86/kernel/asm-offsets.s



Sorry, I forgot to add 'static'.

Adding both static and __used worked for me,
and I like the idea.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-06 21:58     ` Masahiro Yamada
  2018-10-06 22:06       ` Masahiro Yamada
@ 2018-10-06 22:07       ` Miguel Ojeda
  1 sibling, 0 replies; 17+ messages in thread
From: Miguel Ojeda @ 2018-10-06 22:07 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Arnd Bergmann, Andrew Morton, linux-renesas-soc,
	kieran.bingham+renesas, stable, linux-kernel, linux-kbuild

Hi Masahiro,

On Sat, Oct 6, 2018 at 11:59 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Sun, Oct 7, 2018 at 6:18 AM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > I think marking the function as static __used should do the trick and
> > would be less confusing.
>
> I tried __used, but I still see the warning.

It has to be static __used, not only __used! :-)

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-06 22:06       ` Masahiro Yamada
@ 2018-10-08 10:00         ` Kieran Bingham
  2018-10-08 14:32           ` Masahiro Yamada
  0 siblings, 1 reply; 17+ messages in thread
From: Kieran Bingham @ 2018-10-08 10:00 UTC (permalink / raw)
  To: Masahiro Yamada, miguel.ojeda.sandonis
  Cc: Arnd Bergmann, Andrew Morton, Linux-Renesas, stable,
	Linux Kernel Mailing List, Linux Kbuild mailing list

On 06/10/18 23:06, Masahiro Yamada wrote:
> On Sun, Oct 7, 2018 at 6:58 AM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>>
>> Hi Miguel,
>>
>>
>> On Sun, Oct 7, 2018 at 6:18 AM Miguel Ojeda
>> <miguel.ojeda.sandonis@gmail.com> wrote:
>>>
>>> On Fri, Oct 5, 2018 at 10:35 AM Arnd Bergmann <arnd@arndb.de> wrote:
>>>>
>>>> Building any configuration with 'make W=1' produces a warning:
>>>>
>>>> kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]
>>>>
>>>> When also passing -Werror, this prevents us from building any
>>>> other files. Nobody ever calls the function, but we can't make
>>>> it 'static' either since we want the compiler output.
>>>>
>>>> Calling it 'main' instead however avoids the warning, because gcc
>>>> does not insist on having a declaration for main.
>>>
>>> I think marking the function as static __used should do the trick and
>>> would be less confusing.
>>
>>
>>
>>
>> I tried __used, but I still see the warning.
>>
>>
>> masahiro@grover:~/ref/linux$ git diff
>> diff --git a/kernel/bounds.c b/kernel/bounds.c
>> index c373e88..aee0101 100644
>> --- a/kernel/bounds.c
>> +++ b/kernel/bounds.c
>> @@ -13,7 +13,7 @@
>>  #include <linux/log2.h>
>>  #include <linux/spinlock_types.h>
>>
>> -void foo(void)
>> +void __used foo(void)
>>  {
>>         /* The enum constants to put into include/generated/bounds.h */
>>         DEFINE(NR_PAGEFLAGS, __NR_PAGEFLAGS);
>> masahiro@grover:~/ref/linux$ make W=1  prepare
>>   CC      kernel/bounds.s
>> kernel/bounds.c:16:13: warning: no previous prototype for ‘foo’
>> [-Wmissing-prototypes]
>>  void __used foo(void)
>>              ^
>>   CC      arch/x86/kernel/asm-offsets.s
> 
> 
> 
> Sorry, I forgot to add 'static'.
> 
> Adding both static and __used worked for me,
> and I like the idea.
> 

Aha - I'd also tried converting to static in my earlier attempts, but
didn't realise we had __used!

updating as "static __used" causes the following diff:

diff -Nurp bounds.s.foo bounds.s.static-used
--- bounds.s.foo        2018-10-05 10:20:53.269941404 +0100
+++ bounds.s.static-used        2018-10-08 10:51:18.079309049 +0100
@@ -110,7 +110,6 @@
 #NO_APP
        .align  2
        .p2align 3,,7
-       .global foo
        .type   foo, %function
 foo:
        stp     x29, x30, [sp, -16]!    //,,,


I'd say this is a pretty good alternative fix - however I see Arnd's
version is already on it's way though akpm's tree...

https://ozlabs.org/~akpm/mmots/broken-out/kbuild-fix-kernel-boundsc-w%3D1-warning.patch

Anyway, as long as one of the variants gets there I'll be happy :)

--
Regards

Kieran

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-08 10:00         ` Kieran Bingham
@ 2018-10-08 14:32           ` Masahiro Yamada
  2018-10-08 14:41             ` Geert Uytterhoeven
  0 siblings, 1 reply; 17+ messages in thread
From: Masahiro Yamada @ 2018-10-08 14:32 UTC (permalink / raw)
  To: kieran.bingham+renesas
  Cc: Miguel Ojeda, Arnd Bergmann, Andrew Morton, Linux-Renesas,
	stable, Linux Kernel Mailing List, Linux Kbuild mailing list

On Mon, Oct 8, 2018 at 7:01 PM Kieran Bingham
<kieran.bingham+renesas@ideasonboard.com> wrote:
>
> On 06/10/18 23:06, Masahiro Yamada wrote:
> > On Sun, Oct 7, 2018 at 6:58 AM Masahiro Yamada
> > <yamada.masahiro@socionext.com> wrote:
> >>
> >> Hi Miguel,
> >>
> >>
> >> On Sun, Oct 7, 2018 at 6:18 AM Miguel Ojeda
> >> <miguel.ojeda.sandonis@gmail.com> wrote:
> >>>
> >>> On Fri, Oct 5, 2018 at 10:35 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >>>>
> >>>> Building any configuration with 'make W=1' produces a warning:
> >>>>
> >>>> kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]
> >>>>
> >>>> When also passing -Werror, this prevents us from building any
> >>>> other files. Nobody ever calls the function, but we can't make
> >>>> it 'static' either since we want the compiler output.
> >>>>
> >>>> Calling it 'main' instead however avoids the warning, because gcc
> >>>> does not insist on having a declaration for main.
> >>>
> >>> I think marking the function as static __used should do the trick and
> >>> would be less confusing.
> >>
> >>
> >>
> >>
> >> I tried __used, but I still see the warning.
> >>
> >>
> >> masahiro@grover:~/ref/linux$ git diff
> >> diff --git a/kernel/bounds.c b/kernel/bounds.c
> >> index c373e88..aee0101 100644
> >> --- a/kernel/bounds.c
> >> +++ b/kernel/bounds.c
> >> @@ -13,7 +13,7 @@
> >>  #include <linux/log2.h>
> >>  #include <linux/spinlock_types.h>
> >>
> >> -void foo(void)
> >> +void __used foo(void)
> >>  {
> >>         /* The enum constants to put into include/generated/bounds.h */
> >>         DEFINE(NR_PAGEFLAGS, __NR_PAGEFLAGS);
> >> masahiro@grover:~/ref/linux$ make W=1  prepare
> >>   CC      kernel/bounds.s
> >> kernel/bounds.c:16:13: warning: no previous prototype for ‘foo’
> >> [-Wmissing-prototypes]
> >>  void __used foo(void)
> >>              ^
> >>   CC      arch/x86/kernel/asm-offsets.s
> >
> >
> >
> > Sorry, I forgot to add 'static'.
> >
> > Adding both static and __used worked for me,
> > and I like the idea.
> >
>
> Aha - I'd also tried converting to static in my earlier attempts, but
> didn't realise we had __used!
>
> updating as "static __used" causes the following diff:
>
> diff -Nurp bounds.s.foo bounds.s.static-used
> --- bounds.s.foo        2018-10-05 10:20:53.269941404 +0100
> +++ bounds.s.static-used        2018-10-08 10:51:18.079309049 +0100
> @@ -110,7 +110,6 @@
>  #NO_APP
>         .align  2
>         .p2align 3,,7
> -       .global foo
>         .type   foo, %function
>  foo:
>         stp     x29, x30, [sp, -16]!    //,,,
>
>
> I'd say this is a pretty good alternative fix - however I see Arnd's
> version is already on it's way though akpm's tree...
>
> https://ozlabs.org/~akpm/mmots/broken-out/kbuild-fix-kernel-boundsc-w%3D1-warning.patch
>
> Anyway, as long as one of the variants gets there I'll be happy :)


I will leave it to Arnd.


When we fix arch/{mips,sparc}/kernel/asm-offsets.c,
'static __used' is just additions.

The 'main(void)' solution would require a little bit restructuring.



FWIW, with my quick analysis, the following should be fixed as well:

arch/alpha/kernel/asm-offsets.c
arch/c6x/kernel/asm-offsets.c
arch/ia64/kernel/asm-offsets.c
arch/ia64/kernel/nr-irqs.c
arch/mips/kernel/asm-offsets.c
arch/riscv/kernel/asm-offsets.c
arch/sparc/kernel/asm-offsets.c
arch/x86/kernel/asm-offsets.c
arch/x86/um/shared/sysdep/kernel-offsets.h
samples/bpf/syscall_nrs.c






-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning
  2018-10-08 14:32           ` Masahiro Yamada
@ 2018-10-08 14:41             ` Geert Uytterhoeven
  0 siblings, 0 replies; 17+ messages in thread
From: Geert Uytterhoeven @ 2018-10-08 14:41 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Kieran Bingham, Miguel Ojeda Sandonis, Arnd Bergmann,
	Andrew Morton, Linux-Renesas, stable, Linux Kernel Mailing List,
	linux-kbuild

On Mon, Oct 8, 2018 at 4:33 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> On Mon, Oct 8, 2018 at 7:01 PM Kieran Bingham
> <kieran.bingham+renesas@ideasonboard.com> wrote:
> > On 06/10/18 23:06, Masahiro Yamada wrote:
> > > On Sun, Oct 7, 2018 at 6:58 AM Masahiro Yamada
> > > <yamada.masahiro@socionext.com> wrote:
> > >>
> > >> Hi Miguel,
> > >>
> > >>
> > >> On Sun, Oct 7, 2018 at 6:18 AM Miguel Ojeda
> > >> <miguel.ojeda.sandonis@gmail.com> wrote:
> > >>>
> > >>> On Fri, Oct 5, 2018 at 10:35 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > >>>>
> > >>>> Building any configuration with 'make W=1' produces a warning:
> > >>>>
> > >>>> kernel/bounds.c:16:6: warnign: no previous prototype for 'foo' [-Wmissing-prototypes]
> > >>>>
> > >>>> When also passing -Werror, this prevents us from building any
> > >>>> other files. Nobody ever calls the function, but we can't make
> > >>>> it 'static' either since we want the compiler output.
> > >>>>
> > >>>> Calling it 'main' instead however avoids the warning, because gcc
> > >>>> does not insist on having a declaration for main.
> > >>>
> > >>> I think marking the function as static __used should do the trick and
> > >>> would be less confusing.
> > >>
> > >>
> > >>
> > >>
> > >> I tried __used, but I still see the warning.
> > >>
> > >>
> > >> masahiro@grover:~/ref/linux$ git diff
> > >> diff --git a/kernel/bounds.c b/kernel/bounds.c
> > >> index c373e88..aee0101 100644
> > >> --- a/kernel/bounds.c
> > >> +++ b/kernel/bounds.c
> > >> @@ -13,7 +13,7 @@
> > >>  #include <linux/log2.h>
> > >>  #include <linux/spinlock_types.h>
> > >>
> > >> -void foo(void)
> > >> +void __used foo(void)
> > >>  {
> > >>         /* The enum constants to put into include/generated/bounds.h */
> > >>         DEFINE(NR_PAGEFLAGS, __NR_PAGEFLAGS);
> > >> masahiro@grover:~/ref/linux$ make W=1  prepare
> > >>   CC      kernel/bounds.s
> > >> kernel/bounds.c:16:13: warning: no previous prototype for ‘foo’
> > >> [-Wmissing-prototypes]
> > >>  void __used foo(void)
> > >>              ^
> > >>   CC      arch/x86/kernel/asm-offsets.s
> > >
> > >
> > >
> > > Sorry, I forgot to add 'static'.
> > >
> > > Adding both static and __used worked for me,
> > > and I like the idea.
> > >
> >
> > Aha - I'd also tried converting to static in my earlier attempts, but
> > didn't realise we had __used!
> >
> > updating as "static __used" causes the following diff:
> >
> > diff -Nurp bounds.s.foo bounds.s.static-used
> > --- bounds.s.foo        2018-10-05 10:20:53.269941404 +0100
> > +++ bounds.s.static-used        2018-10-08 10:51:18.079309049 +0100
> > @@ -110,7 +110,6 @@
> >  #NO_APP
> >         .align  2
> >         .p2align 3,,7
> > -       .global foo
> >         .type   foo, %function
> >  foo:
> >         stp     x29, x30, [sp, -16]!    //,,,
> >
> >
> > I'd say this is a pretty good alternative fix - however I see Arnd's
> > version is already on it's way though akpm's tree...
> >
> > https://ozlabs.org/~akpm/mmots/broken-out/kbuild-fix-kernel-boundsc-w%3D1-warning.patch
> >
> > Anyway, as long as one of the variants gets there I'll be happy :)
>
>
> I will leave it to Arnd.
>
>
> When we fix arch/{mips,sparc}/kernel/asm-offsets.c,
> 'static __used' is just additions.
>
> The 'main(void)' solution would require a little bit restructuring.
>
>
>
> FWIW, with my quick analysis, the following should be fixed as well:
>
> arch/alpha/kernel/asm-offsets.c
> arch/c6x/kernel/asm-offsets.c
> arch/ia64/kernel/asm-offsets.c
> arch/ia64/kernel/nr-irqs.c
> arch/mips/kernel/asm-offsets.c
> arch/riscv/kernel/asm-offsets.c
> arch/sparc/kernel/asm-offsets.c
> arch/x86/kernel/asm-offsets.c
> arch/x86/um/shared/sysdep/kernel-offsets.h
> samples/bpf/syscall_nrs.c

All the other asm-offsets.c files already use "int main(void)" ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-10-08 21:53 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-21 14:22 [PATCH] kernel/bounds: Provide prototype for foo Kieran Bingham
2018-09-21 14:45 ` Greg Kroah-Hartman
2018-09-21 15:58   ` Kieran Bingham
2018-09-21 16:03     ` Kieran Bingham
2018-10-05  8:33 ` [PATCH] kbuild: fix kernel/bounds.c 'W=1' warning Arnd Bergmann
2018-10-05  8:47   ` Kieran Bingham
2018-10-05  8:52   ` David Laight
2018-10-05  9:07     ` Arnd Bergmann
2018-10-05  9:27       ` Kieran Bingham
2018-10-06 20:31   ` Masahiro Yamada
2018-10-06 21:18   ` Miguel Ojeda
2018-10-06 21:58     ` Masahiro Yamada
2018-10-06 22:06       ` Masahiro Yamada
2018-10-08 10:00         ` Kieran Bingham
2018-10-08 14:32           ` Masahiro Yamada
2018-10-08 14:41             ` Geert Uytterhoeven
2018-10-06 22:07       ` Miguel Ojeda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).