[PATCH] [RFC] ARM: vdso: work around gcc-8 warning

[PATCH] [RFC] ARM: vdso: work around gcc-8 warning

[Arnd Bergmann]

gcc-8 correctly points out that reading four bytes from a pointer to a
'char' variable is wrong

arch/arm/kernel/vdso.c: In function 'vdso_init':
arch/arm/kernel/vdso.c:200:6: error: '__builtin_memcmp_eq' reading 4 bytes from a region of size 1 [-Werror=stringop-overflow=]

However, in this case the variable just stands for the beginning of the
vdso and is not actually a 'char', so the code is doing what it is meant
to do.

Not sure what the best solution for this is, changing the hack to
declare the variable as 'int' instead makes the warning go away.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/include/asm/vdso.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/vdso.h b/arch/arm/include/asm/vdso.h
index d0295f1dd1a3..eff7d3a1d1ce 100644
--- a/arch/arm/include/asm/vdso.h
+++ b/arch/arm/include/asm/vdso.h
@@ -11,7 +11,7 @@ struct mm_struct;
 
 void arm_install_vdso(struct mm_struct *mm, unsigned long addr);
 
-extern char vdso_start, vdso_end;
+extern int vdso_start, vdso_end;
 
 extern unsigned int vdso_total_pages;
 
-- 
2.9.0

[PATCH] [RFC] ARM: vdso: work around gcc-8 warning

[Ard Biesheuvel]

On 23 August 2017 at 15:05, Arnd Bergmann <arnd@arndb.de> wrote:
> gcc-8 correctly points out that reading four bytes from a pointer to a
> 'char' variable is wrong
>
> arch/arm/kernel/vdso.c: In function 'vdso_init':
> arch/arm/kernel/vdso.c:200:6: error: '__builtin_memcmp_eq' reading 4 bytes from a region of size 1 [-Werror=stringop-overflow=]
>
> However, in this case the variable just stands for the beginning of the
> vdso and is not actually a 'char', so the code is doing what it is meant
> to do.
>
> Not sure what the best solution for this is, changing the hack to
> declare the variable as 'int' instead makes the warning go away.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/include/asm/vdso.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/vdso.h b/arch/arm/include/asm/vdso.h
> index d0295f1dd1a3..eff7d3a1d1ce 100644
> --- a/arch/arm/include/asm/vdso.h
> +++ b/arch/arm/include/asm/vdso.h
> @@ -11,7 +11,7 @@ struct mm_struct;
>
>  void arm_install_vdso(struct mm_struct *mm, unsigned long addr);
>
> -extern char vdso_start, vdso_end;
> +extern int vdso_start, vdso_end;
>
>  extern unsigned int vdso_total_pages;
>

May I suggest

extern char vdso_start[], vdso_end[];

[PATCH] [RFC] ARM: vdso: work around gcc-8 warning

[Mark Rutland]

On Wed, Aug 23, 2017 at 03:07:04PM +0100, Ard Biesheuvel wrote:
> On 23 August 2017 at 15:05, Arnd Bergmann <arnd@arndb.de> wrote:
> > gcc-8 correctly points out that reading four bytes from a pointer to a
> > 'char' variable is wrong
> >
> > arch/arm/kernel/vdso.c: In function 'vdso_init':
> > arch/arm/kernel/vdso.c:200:6: error: '__builtin_memcmp_eq' reading 4 bytes from a region of size 1 [-Werror=stringop-overflow=]
> >
> > However, in this case the variable just stands for the beginning of the
> > vdso and is not actually a 'char', so the code is doing what it is meant
> > to do.
> >
> > Not sure what the best solution for this is, changing the hack to
> > declare the variable as 'int' instead makes the warning go away.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  arch/arm/include/asm/vdso.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/include/asm/vdso.h b/arch/arm/include/asm/vdso.h
> > index d0295f1dd1a3..eff7d3a1d1ce 100644
> > --- a/arch/arm/include/asm/vdso.h
> > +++ b/arch/arm/include/asm/vdso.h
> > @@ -11,7 +11,7 @@ struct mm_struct;
> >
> >  void arm_install_vdso(struct mm_struct *mm, unsigned long addr);
> >
> > -extern char vdso_start, vdso_end;
> > +extern int vdso_start, vdso_end;
> >
> >  extern unsigned int vdso_total_pages;
> 
> May I suggest
> 
> extern char vdso_start[], vdso_end[];

FWIW, arm64 does this to solve this problem, since commit:

dbbb08f500d61463 ("arm64, vdso: Define vdso_{start,end} as array")

... and x86 has done likewise for a while.

Thanks,
Mark.

[PATCH] [RFC] ARM: vdso: work around gcc-8 warning

[Arnd Bergmann]

On Wed, Aug 23, 2017 at 4:11 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Aug 23, 2017 at 03:07:04PM +0100, Ard Biesheuvel wrote:
>> On 23 August 2017 at 15:05, Arnd Bergmann <arnd@arndb.de> wrote:

>> > diff --git a/arch/arm/include/asm/vdso.h b/arch/arm/include/asm/vdso.h
>> > index d0295f1dd1a3..eff7d3a1d1ce 100644
>> > --- a/arch/arm/include/asm/vdso.h
>> > +++ b/arch/arm/include/asm/vdso.h
>> > @@ -11,7 +11,7 @@ struct mm_struct;
>> >
>> >  void arm_install_vdso(struct mm_struct *mm, unsigned long addr);
>> >
>> > -extern char vdso_start, vdso_end;
>> > +extern int vdso_start, vdso_end;
>> >
>> >  extern unsigned int vdso_total_pages;
>>
>> May I suggest
>>
>> extern char vdso_start[], vdso_end[];

This was my first attempt, but when I ran into a build error I did not
even look further

> FWIW, arm64 does this to solve this problem, since commit:
>
> dbbb08f500d61463 ("arm64, vdso: Define vdso_{start,end} as array")
>
> ... and x86 has done likewise for a while.

Yes, this looks good, I'll send a patch to do the same.

    Arnd

[PATCH] [RFC] ARM: vdso: work around gcc-8 warning

[Russell King - ARM Linux]

On Wed, Aug 23, 2017 at 04:05:26PM +0200, Arnd Bergmann wrote:
> gcc-8 correctly points out that reading four bytes from a pointer to a
> 'char' variable is wrong
> 
> arch/arm/kernel/vdso.c: In function 'vdso_init':
> arch/arm/kernel/vdso.c:200:6: error: '__builtin_memcmp_eq' reading 4 bytes from a region of size 1 [-Werror=stringop-overflow=]
> 
> However, in this case the variable just stands for the beginning of the
> vdso and is not actually a 'char', so the code is doing what it is meant
> to do.
> 
> Not sure what the best solution for this is, changing the hack to
> declare the variable as 'int' instead makes the warning go away.

Well, one way to look at the code is an array of characters, so
we could say that it should be:

extern char vdso_start[], vdso_end[];

which is exactly the same construct that we use for _stext and
similar.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up