Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1633439 > unrolled thread
| Started by | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| First post | 2017-04-30 16:10 +0200 |
| Last post | 2017-05-09 01:20 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 1/2] kbuild: clang: Disable 'address-of-packed-member' warning Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-04-30 16:10 +0200
Re: [PATCH 1/2] kbuild: clang: Disable 'address-of-packed-member' warning Matthias Kaehlcke <mka@chromium.org> - 2017-05-02 03:30 +0200
Re: [PATCH 1/2] kbuild: clang: Disable 'address-of-packed-member' warning Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-05-06 19:00 +0200
Re: [PATCH 1/2] kbuild: clang: Disable 'address-of-packed-member' warning Matthias Kaehlcke <mka@chromium.org> - 2017-05-09 01:20 +0200
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-04-30 16:10 +0200 |
| Subject | Re: [PATCH 1/2] kbuild: clang: Disable 'address-of-packed-member' warning |
| Message-ID | <tBXHX-4br-1@gated-at.bofh.it> |
Hi Matthias,
2017-04-22 6:39 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> clang generates plenty of these warnings in different parts of the code,
> to an extent that the warnings are little more than noise. Disable the
> 'address-of-packed-member' warning.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
As far as I compiled arch/x86/configs/x86_64_defconfig,
all address-of-packed-member warnings came from the single point:
./arch/x86/include/asm/processor.h:534:30: warning: taking address of
packed member 'sp0' of class or structure 'x86_hw_tss' may result in
an unaligned pointer value [-Waddress-of-packed-member]
return this_cpu_read_stable(cpu_tss.x86_tss.sp0);
^~~~~~~~~~~~~~~~~~~
./arch/x86/include/asm/percpu.h:391:59: note: expanded from macro
'this_cpu_read_stable'
#define this_cpu_read_stable(var) percpu_stable_op("mov", var)
^~~
./arch/x86/include/asm/percpu.h:228:16: note: expanded from macro
'percpu_stable_op'
: "p" (&(var))); \
^~~
For this case, I was able to fix it with the following patch:
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 9fa0360..de25d1c 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -211,26 +211,27 @@ do {
\
#define percpu_stable_op(op, var) \
({ \
typeof(var) pfo_ret__; \
+ void *__p = &(var); \
switch (sizeof(var)) { \
case 1: \
asm(op "b "__percpu_arg(P1)",%0" \
: "=q" (pfo_ret__) \
- : "p" (&(var))); \
+ : "p" (__p)); \
break; \
case 2: \
asm(op "w "__percpu_arg(P1)",%0" \
: "=r" (pfo_ret__) \
- : "p" (&(var))); \
+ : "p" (__p)); \
break; \
case 4: \
asm(op "l "__percpu_arg(P1)",%0" \
: "=r" (pfo_ret__) \
- : "p" (&(var))); \
+ : "p" (__p)); \
break; \
case 8: \
asm(op "q "__percpu_arg(P1)",%0" \
: "=r" (pfo_ret__) \
- : "p" (&(var))); \
+ : "p" (__p)); \
break; \
default: __bad_percpu_size(); \
} \
I'd like to see as much effort as possible
before we decide to hide this kind of warning.
Is it OK with you ?
--
Best Regards
Masahiro Yamada
[toc] | [next] | [standalone]
| From | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| Date | 2017-05-02 03:30 +0200 |
| Subject | Re: [PATCH 1/2] kbuild: clang: Disable 'address-of-packed-member' warning |
| Message-ID | <tCuNz-87d-1@gated-at.bofh.it> |
| In reply to | #1633439 |
Hi Masahiro,
El Sun, Apr 30, 2017 at 10:59:52PM +0900 Masahiro Yamada ha dit:
> 2017-04-22 6:39 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> > clang generates plenty of these warnings in different parts of the code,
> > to an extent that the warnings are little more than noise. Disable the
> > 'address-of-packed-member' warning.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>
>
> As far as I compiled arch/x86/configs/x86_64_defconfig,
> all address-of-packed-member warnings came from the single point:
>
> ./arch/x86/include/asm/processor.h:534:30: warning: taking address of
> packed member 'sp0' of class or structure 'x86_hw_tss' may result in
> an unaligned pointer value [-Waddress-of-packed-member]
> return this_cpu_read_stable(cpu_tss.x86_tss.sp0);
> ^~~~~~~~~~~~~~~~~~~
> ./arch/x86/include/asm/percpu.h:391:59: note: expanded from macro
> 'this_cpu_read_stable'
> #define this_cpu_read_stable(var) percpu_stable_op("mov", var)
> ^~~
> ./arch/x86/include/asm/percpu.h:228:16: note: expanded from macro
> 'percpu_stable_op'
> : "p" (&(var))); \
> ^~~
>
>
>
> For this case, I was able to fix it with the following patch:
>
>
> diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
> index 9fa0360..de25d1c 100644
> --- a/arch/x86/include/asm/percpu.h
> +++ b/arch/x86/include/asm/percpu.h
> @@ -211,26 +211,27 @@ do {
> \
> #define percpu_stable_op(op, var) \
> ({ \
> typeof(var) pfo_ret__; \
> + void *__p = &(var); \
> switch (sizeof(var)) { \
> case 1: \
> asm(op "b "__percpu_arg(P1)",%0" \
> : "=q" (pfo_ret__) \
> - : "p" (&(var))); \
> + : "p" (__p)); \
> break; \
> case 2: \
> asm(op "w "__percpu_arg(P1)",%0" \
> : "=r" (pfo_ret__) \
> - : "p" (&(var))); \
> + : "p" (__p)); \
> break; \
> case 4: \
> asm(op "l "__percpu_arg(P1)",%0" \
> : "=r" (pfo_ret__) \
> - : "p" (&(var))); \
> + : "p" (__p)); \
> break; \
> case 8: \
> asm(op "q "__percpu_arg(P1)",%0" \
> : "=r" (pfo_ret__) \
> - : "p" (&(var))); \
> + : "p" (__p)); \
> break; \
> default: __bad_percpu_size(); \
> } \
Thanks for having a look!
It is odd though that you only see warnings from that origin, I
encounter plenty of others with x86_64_defconfig, mostly stemming
from uaccess macros:
kernel/power/user.c:439:35: warning: taking address of packed member
'dev' of class or structure 'compat_resume_swap_area' may result in an
unaligned pointer value [-Waddress-of-packed-member]
err |= get_user(swap_area.dev, &u_swap_area->dev);
^~~~~~~~~~~~~~~~
./arch/x86/include/asm/uaccess.h:168:23: note: expanded from macro 'get_user'
register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX); \
^~~
./arch/x86/include/asm/uaccess.h:132:41: note: expanded from macro '__inttype'
__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
^
I looked into fixing different cases, but didn't see a clear path
forward since we can't just cast the type away as in your patch above.
> I'd like to see as much effort as possible
> before we decide to hide this kind of warning.
>
> Is it OK with you ?
Sounds good, though I will probably leave this one for someone else :)
Cheers
Matthias
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-05-06 19:00 +0200 |
| Message-ID | <tEbdL-24O-3@gated-at.bofh.it> |
| In reply to | #1634071 |
Hi Matthias,
2017-05-02 10:23 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> Hi Masahiro,
>
> El Sun, Apr 30, 2017 at 10:59:52PM +0900 Masahiro Yamada ha dit:
>
>> 2017-04-22 6:39 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
>> > clang generates plenty of these warnings in different parts of the code,
>> > to an extent that the warnings are little more than noise. Disable the
>> > 'address-of-packed-member' warning.
>> >
>> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>>
>>
>> As far as I compiled arch/x86/configs/x86_64_defconfig,
>> all address-of-packed-member warnings came from the single point:
>>
>> ./arch/x86/include/asm/processor.h:534:30: warning: taking address of
>> packed member 'sp0' of class or structure 'x86_hw_tss' may result in
>> an unaligned pointer value [-Waddress-of-packed-member]
>> return this_cpu_read_stable(cpu_tss.x86_tss.sp0);
>> ^~~~~~~~~~~~~~~~~~~
>> ./arch/x86/include/asm/percpu.h:391:59: note: expanded from macro
>> 'this_cpu_read_stable'
>> #define this_cpu_read_stable(var) percpu_stable_op("mov", var)
>> ^~~
>> ./arch/x86/include/asm/percpu.h:228:16: note: expanded from macro
>> 'percpu_stable_op'
>> : "p" (&(var))); \
>> ^~~
>>
>>
>>
>> For this case, I was able to fix it with the following patch:
>>
>>
>> diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
>> index 9fa0360..de25d1c 100644
>> --- a/arch/x86/include/asm/percpu.h
>> +++ b/arch/x86/include/asm/percpu.h
>> @@ -211,26 +211,27 @@ do {
>> \
>> #define percpu_stable_op(op, var) \
>> ({ \
>> typeof(var) pfo_ret__; \
>> + void *__p = &(var); \
>> switch (sizeof(var)) { \
>> case 1: \
>> asm(op "b "__percpu_arg(P1)",%0" \
>> : "=q" (pfo_ret__) \
>> - : "p" (&(var))); \
>> + : "p" (__p)); \
>> break; \
>> case 2: \
>> asm(op "w "__percpu_arg(P1)",%0" \
>> : "=r" (pfo_ret__) \
>> - : "p" (&(var))); \
>> + : "p" (__p)); \
>> break; \
>> case 4: \
>> asm(op "l "__percpu_arg(P1)",%0" \
>> : "=r" (pfo_ret__) \
>> - : "p" (&(var))); \
>> + : "p" (__p)); \
>> break; \
>> case 8: \
>> asm(op "q "__percpu_arg(P1)",%0" \
>> : "=r" (pfo_ret__) \
>> - : "p" (&(var))); \
>> + : "p" (__p)); \
>> break; \
>> default: __bad_percpu_size(); \
>> } \
>
> Thanks for having a look!
>
> It is odd though that you only see warnings from that origin, I
> encounter plenty of others with x86_64_defconfig, mostly stemming
> from uaccess macros:
>
> kernel/power/user.c:439:35: warning: taking address of packed member
> 'dev' of class or structure 'compat_resume_swap_area' may result in an
> unaligned pointer value [-Waddress-of-packed-member]
> err |= get_user(swap_area.dev, &u_swap_area->dev);
> ^~~~~~~~~~~~~~~~
> ./arch/x86/include/asm/uaccess.h:168:23: note: expanded from macro 'get_user'
> register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX); \
> ^~~
> ./arch/x86/include/asm/uaccess.h:132:41: note: expanded from macro '__inttype'
> __typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
> ^
>
> I looked into fixing different cases, but didn't see a clear path
> forward since we can't just cast the type away as in your patch above.
Curious.
I tested clang 3.0 thru 4.0, but I could not reproduce this.
This part just calculates sizeof(*(ptr)).
I think it is a false positive warning bug if clang reports this.
--
Best Regards
Masahiro Yamada
[toc] | [prev] | [next] | [standalone]
| From | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| Date | 2017-05-09 01:20 +0200 |
| Subject | Re: [PATCH 1/2] kbuild: clang: Disable 'address-of-packed-member' warning |
| Message-ID | <tF06B-1YT-1@gated-at.bofh.it> |
| In reply to | #1636952 |
Hi Masahiro,
El Sun, May 07, 2017 at 01:52:25AM +0900 Masahiro Yamada ha dit:
> 2017-05-02 10:23 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> > Hi Masahiro,
> >
> > El Sun, Apr 30, 2017 at 10:59:52PM +0900 Masahiro Yamada ha dit:
> >
> >> 2017-04-22 6:39 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> >> > clang generates plenty of these warnings in different parts of the code,
> >> > to an extent that the warnings are little more than noise. Disable the
> >> > 'address-of-packed-member' warning.
> >> >
> >> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >>
> >>
> >> As far as I compiled arch/x86/configs/x86_64_defconfig,
> >> all address-of-packed-member warnings came from the single point:
> >>
> >> ./arch/x86/include/asm/processor.h:534:30: warning: taking address of
> >> packed member 'sp0' of class or structure 'x86_hw_tss' may result in
> >> an unaligned pointer value [-Waddress-of-packed-member]
> >> return this_cpu_read_stable(cpu_tss.x86_tss.sp0);
> >> ^~~~~~~~~~~~~~~~~~~
> >> ./arch/x86/include/asm/percpu.h:391:59: note: expanded from macro
> >> 'this_cpu_read_stable'
> >> #define this_cpu_read_stable(var) percpu_stable_op("mov", var)
> >> ^~~
> >> ./arch/x86/include/asm/percpu.h:228:16: note: expanded from macro
> >> 'percpu_stable_op'
> >> : "p" (&(var))); \
> >> ^~~
> >>
> >>
> >>
> >> For this case, I was able to fix it with the following patch:
> >>
> >>
> >> diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
> >> index 9fa0360..de25d1c 100644
> >> --- a/arch/x86/include/asm/percpu.h
> >> +++ b/arch/x86/include/asm/percpu.h
> >> @@ -211,26 +211,27 @@ do {
> >> \
> >> #define percpu_stable_op(op, var) \
> >> ({ \
> >> typeof(var) pfo_ret__; \
> >> + void *__p = &(var); \
> >> switch (sizeof(var)) { \
> >> case 1: \
> >> asm(op "b "__percpu_arg(P1)",%0" \
> >> : "=q" (pfo_ret__) \
> >> - : "p" (&(var))); \
> >> + : "p" (__p)); \
> >> break; \
> >> case 2: \
> >> asm(op "w "__percpu_arg(P1)",%0" \
> >> : "=r" (pfo_ret__) \
> >> - : "p" (&(var))); \
> >> + : "p" (__p)); \
> >> break; \
> >> case 4: \
> >> asm(op "l "__percpu_arg(P1)",%0" \
> >> : "=r" (pfo_ret__) \
> >> - : "p" (&(var))); \
> >> + : "p" (__p)); \
> >> break; \
> >> case 8: \
> >> asm(op "q "__percpu_arg(P1)",%0" \
> >> : "=r" (pfo_ret__) \
> >> - : "p" (&(var))); \
> >> + : "p" (__p)); \
> >> break; \
> >> default: __bad_percpu_size(); \
> >> } \
> >
> > Thanks for having a look!
> >
> > It is odd though that you only see warnings from that origin, I
> > encounter plenty of others with x86_64_defconfig, mostly stemming
> > from uaccess macros:
> >
> > kernel/power/user.c:439:35: warning: taking address of packed member
> > 'dev' of class or structure 'compat_resume_swap_area' may result in an
> > unaligned pointer value [-Waddress-of-packed-member]
> > err |= get_user(swap_area.dev, &u_swap_area->dev);
> > ^~~~~~~~~~~~~~~~
> > ./arch/x86/include/asm/uaccess.h:168:23: note: expanded from macro 'get_user'
> > register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX); \
> > ^~~
> > ./arch/x86/include/asm/uaccess.h:132:41: note: expanded from macro '__inttype'
> > __typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
> > ^
> >
> > I looked into fixing different cases, but didn't see a clear path
> > forward since we can't just cast the type away as in your patch above.
>
>
> Curious.
> I tested clang 3.0 thru 4.0, but I could not reproduce this.
>
> This part just calculates sizeof(*(ptr)).
> I think it is a false positive warning bug if clang reports this.
The instance above is indeed somewhat doubtful, in any case there are
plenty of others, most of them from fs/compat.c using __get/put_user_xyz():
fs/compat.c:366:33: warning: taking address of packed member 'l_whence' of class or structure 'compat_flock64' may result in an unaligned pointer value [-Waddress-of-packed-member]
__get_user(kfl->l_whence, &ufl->l_whence) ||
^~~~~~~~~~~~~
arch/x86/include/asm/uaccess.h:505:27: note: expanded from macro '__get_user'
__get_user_nocheck((x), (ptr), sizeof(*(ptr)))
^~~
arch/x86/include/asm/uaccess.h:436:29: note: expanded from macro '__get_user_nocheck'
__get_user_size(__gu_val, (ptr), (size), __gu_err, -EFAULT); \
^~~
arch/x86/include/asm/uaccess.h:361:21: note: expanded from macro '__get_user_size'
__get_user_asm(x, ptr, retval, "w", "w", "=r", errret); \
^~~
arch/x86/include/asm/uaccess.h:385:19: note: expanded from macro '__get_user_asm'
: "m" (__m(addr)), "i" (errret), "0" (err))
^~~~
arch/x86/include/asm/uaccess.h:444:51: note: expanded from macro '__m'
#define __m(x) (*(struct __large_struct __user *)(x))
^
The clang version I use is fairly recent since it includes some
fixes needed to build a working kernel (mostly for ARM64).
clang --version
Chromium OS 5.0_pre300080-r1 clang version 5.0.0
Cheers
Matthias
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web