Message ID | 20180914040649.1794-5-joel@jms.id.au (mailing list archive) |
---|---|
State | Accepted |
Commit | e00d93ac9a189673028ac125a74b9bc8ae73eebc |
Headers | show |
Series | powerpc: Clang build fixes | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
snowpatch_ozlabs/checkpatch | fail | Test checkpatch on branch next |
On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote: > > From: Anton Blanchard <anton@samba.org> > > This re-applies b91c1e3e7a6f which was reverted in f2ca80905929 > d466f6c5cac1 f84ed59a612d (powerpc/sparse: Constify the address pointer > ..."). > > We see a large number of duplicate const errors in the user access > code when building with llvm/clang: > > include/linux/pagemap.h:576:8: warning: duplicate 'const' declaration specifier > [-Wduplicate-decl-specifier] > ret = __get_user(c, uaddr); > > The problem is we are doing const __typeof__(*(ptr)), which will hit the > warning if ptr is marked const. > > Removing const does not seem to have any effect on GCC code generation. I wouldn't expect it to for a local variable with such localized usage. I myself am quite liberal in applying `const` to everything, so I will try to fix this in Clang as well, but this should silence the warning for users of older versions of Clang and results in no functional change. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > Signed-off-by: Anton Blanchard <anton@samba.org> > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > If we don't want to apply this, other options are suppressing the > warning, or wait for a fix to land in clang > (https://github.com/ClangBuiltLinux/linux/issues/52). > --- > arch/powerpc/include/asm/uaccess.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h > index bac225bb7f64..15bea9a0f260 100644 > --- a/arch/powerpc/include/asm/uaccess.h > +++ b/arch/powerpc/include/asm/uaccess.h > @@ -260,7 +260,7 @@ do { \ > ({ \ > long __gu_err; \ > __long_type(*(ptr)) __gu_val; \ > - const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ > + __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ > __chk_user_ptr(ptr); \ > if (!is_kernel_addr((unsigned long)__gu_addr)) \ > might_fault(); \ > @@ -274,7 +274,7 @@ do { \ > ({ \ > long __gu_err = -EFAULT; \ > __long_type(*(ptr)) __gu_val = 0; \ > - const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ > + __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ > might_fault(); \ > if (access_ok(VERIFY_READ, __gu_addr, (size))) { \ > barrier_nospec(); \ > @@ -288,7 +288,7 @@ do { \ > ({ \ > long __gu_err; \ > __long_type(*(ptr)) __gu_val; \ > - const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ > + __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ > __chk_user_ptr(ptr); \ > barrier_nospec(); \ > __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \ > -- > 2.17.1 >
On Sat, 15 Sep 2018 at 03:27, Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote: > > > > From: Anton Blanchard <anton@samba.org> > > > > This re-applies b91c1e3e7a6f which was reverted in f2ca80905929 > > d466f6c5cac1 f84ed59a612d (powerpc/sparse: Constify the address pointer > > ..."). > > > > We see a large number of duplicate const errors in the user access > > code when building with llvm/clang: > > > > include/linux/pagemap.h:576:8: warning: duplicate 'const' declaration specifier > > [-Wduplicate-decl-specifier] > > ret = __get_user(c, uaddr); > > > > The problem is we are doing const __typeof__(*(ptr)), which will hit the > > warning if ptr is marked const. > > > > Removing const does not seem to have any effect on GCC code generation. > > I wouldn't expect it to for a local variable with such localized > usage. I myself am quite liberal in applying `const` to everything, > so I will try to fix this in Clang as well, but this should silence > the warning for users of older versions of Clang and results in no > functional change. > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Nick has written a clang patch that suppresses the warning in the same way as GCC. Assuming it gets merged, as we depend on clang-8 we could chose to not merge the kernel patch. https://reviews.llvm.org/D52248 Cheers, Joel
On Fri, 2018-09-14 at 04:06:48 UTC, Joel Stanley wrote: > From: Anton Blanchard <anton@samba.org> > > This re-applies b91c1e3e7a6f which was reverted in f2ca80905929 > d466f6c5cac1 f84ed59a612d (powerpc/sparse: Constify the address pointer > ..."). > > We see a large number of duplicate const errors in the user access > code when building with llvm/clang: > > include/linux/pagemap.h:576:8: warning: duplicate 'const' declaration specifier > [-Wduplicate-decl-specifier] > ret = __get_user(c, uaddr); > > The problem is we are doing const __typeof__(*(ptr)), which will hit the > warning if ptr is marked const. > > Removing const does not seem to have any effect on GCC code generation. > > Signed-off-by: Anton Blanchard <anton@samba.org> > Signed-off-by: Joel Stanley <joel@jms.id.au> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/e00d93ac9a189673028ac125a74b9b cheers
Le 19/09/2018 à 09:45, Joel Stanley a écrit : > On Sat, 15 Sep 2018 at 03:27, Nick Desaulniers <ndesaulniers@google.com> wrote: >> >> On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote: >>> >>> From: Anton Blanchard <anton@samba.org> >>> >>> This re-applies b91c1e3e7a6f which was reverted in f2ca80905929 >>> d466f6c5cac1 f84ed59a612d (powerpc/sparse: Constify the address pointer >>> ..."). >>> >>> We see a large number of duplicate const errors in the user access >>> code when building with llvm/clang: >>> >>> include/linux/pagemap.h:576:8: warning: duplicate 'const' declaration specifier >>> [-Wduplicate-decl-specifier] >>> ret = __get_user(c, uaddr); >>> >>> The problem is we are doing const __typeof__(*(ptr)), which will hit the >>> warning if ptr is marked const. >>> >>> Removing const does not seem to have any effect on GCC code generation. >> >> I wouldn't expect it to for a local variable with such localized >> usage. I myself am quite liberal in applying `const` to everything, >> so I will try to fix this in Clang as well, but this should silence >> the warning for users of older versions of Clang and results in no >> functional change. >> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > Nick has written a clang patch that suppresses the warning in the same > way as GCC. Assuming it gets merged, as we depend on clang-8 we could > chose to not merge the kernel patch. > > https://reviews.llvm.org/D52248 > Seems like Michael has merged this patch anyway. Christophe
Christophe LEROY <christophe.leroy@c-s.fr> writes: > Le 19/09/2018 à 09:45, Joel Stanley a écrit : >> On Sat, 15 Sep 2018 at 03:27, Nick Desaulniers <ndesaulniers@google.com> wrote: >>> >>> On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote: >>>> >>>> From: Anton Blanchard <anton@samba.org> >>>> >>>> This re-applies b91c1e3e7a6f which was reverted in f2ca80905929 >>>> d466f6c5cac1 f84ed59a612d (powerpc/sparse: Constify the address pointer >>>> ..."). >>>> >>>> We see a large number of duplicate const errors in the user access >>>> code when building with llvm/clang: >>>> >>>> include/linux/pagemap.h:576:8: warning: duplicate 'const' declaration specifier >>>> [-Wduplicate-decl-specifier] >>>> ret = __get_user(c, uaddr); >>>> >>>> The problem is we are doing const __typeof__(*(ptr)), which will hit the >>>> warning if ptr is marked const. >>>> >>>> Removing const does not seem to have any effect on GCC code generation. >>> >>> I wouldn't expect it to for a local variable with such localized >>> usage. I myself am quite liberal in applying `const` to everything, >>> so I will try to fix this in Clang as well, but this should silence >>> the warning for users of older versions of Clang and results in no >>> functional change. >>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> >> >> Nick has written a clang patch that suppresses the warning in the same >> way as GCC. Assuming it gets merged, as we depend on clang-8 we could >> chose to not merge the kernel patch. >> >> https://reviews.llvm.org/D52248 > > Seems like Michael has merged this patch anyway. I'll happily revert it though if someone tells me to. cheers
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h index bac225bb7f64..15bea9a0f260 100644 --- a/arch/powerpc/include/asm/uaccess.h +++ b/arch/powerpc/include/asm/uaccess.h @@ -260,7 +260,7 @@ do { \ ({ \ long __gu_err; \ __long_type(*(ptr)) __gu_val; \ - const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ + __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ __chk_user_ptr(ptr); \ if (!is_kernel_addr((unsigned long)__gu_addr)) \ might_fault(); \ @@ -274,7 +274,7 @@ do { \ ({ \ long __gu_err = -EFAULT; \ __long_type(*(ptr)) __gu_val = 0; \ - const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ + __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ might_fault(); \ if (access_ok(VERIFY_READ, __gu_addr, (size))) { \ barrier_nospec(); \ @@ -288,7 +288,7 @@ do { \ ({ \ long __gu_err; \ __long_type(*(ptr)) __gu_val; \ - const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ + __typeof__(*(ptr)) __user *__gu_addr = (ptr); \ __chk_user_ptr(ptr); \ barrier_nospec(); \ __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \