| Message ID | 20170823140538.1590432-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
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[];
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.
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
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
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;
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(-) -- 2.9.0