Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1702541 > unrolled thread
| Started by | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| First post | 2017-08-03 01:00 +0200 |
| Last post | 2017-08-03 19:10 +0200 |
| Articles | 6 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH 1/2] bitops: Avoid integer overflow warning in GENMASK_ULL Matthias Kaehlcke <mka@chromium.org> - 2017-08-03 01:00 +0200
[PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL Matthias Kaehlcke <mka@chromium.org> - 2017-08-03 01:00 +0200
Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL Nick Desaulniers <ndesaulniers@google.com> - 2017-08-03 01:20 +0200
Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL Matthias Kaehlcke <mka@chromium.org> - 2017-08-03 01:50 +0200
Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL Nick Desaulniers <ndesaulniers@google.com> - 2017-08-03 01:20 +0200
Re: [PATCH 1/2] bitops: Avoid integer overflow warning in GENMASK_ULL Matthias Kaehlcke <mka@chromium.org> - 2017-08-03 19:10 +0200
| From | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| Date | 2017-08-03 01:00 +0200 |
| Subject | [PATCH 1/2] bitops: Avoid integer overflow warning in GENMASK_ULL |
| Message-ID | <uaaMq-7QI-5@gated-at.bofh.it> |
GENMASK_ULL performs a left-shift of (~0ULL), which technically results in an integer overflow. clang raises a warning about this if the overflow occurs in a preprocessor expression. To avoid the overflow first perform a right-shift to clear the bits that are shifted out. Signed-off-by: Matthias Kaehlcke <mka@chromium.org> --- include/linux/bitops.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/bitops.h b/include/linux/bitops.h index a83c822c35c2..21dfe63001e3 100644 --- a/include/linux/bitops.h +++ b/include/linux/bitops.h @@ -22,7 +22,7 @@ (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h)))) #define GENMASK_ULL(h, l) \ - (((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h)))) + ((((~0ULL) >> (l)) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h)))) extern unsigned int __sw_hweight8(unsigned int w); extern unsigned int __sw_hweight16(unsigned int w); -- 2.14.0.rc1.383.gd1ce394fe2-goog
[toc] | [next] | [standalone]
| From | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| Date | 2017-08-03 01:00 +0200 |
| Subject | [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL |
| Message-ID | <uaaMq-7QI-17@gated-at.bofh.it> |
| In reply to | #1702541 |
As is the definition causes an integer overflow, which is expected,
however clang raises the following warning:
arch/arm64/kernel/head.S:47:8: warning:
integer overflow in preprocessor expression
#elif (PAGE_OFFSET & 0x1fffff) != 0
^~~~~~~~~~~
arch/arm64/include/asm/memory.h:52:46: note:
expanded from macro 'PAGE_OFFSET'
#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1))
~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
Use GENMASK_ULL() instead of shifting explicitly, the macro takes care
of avoiding the overflow.
Reported-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
arch/arm64/include/asm/memory.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 32f82723338a..732d4eed8edd 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -65,7 +65,7 @@
*/
#define VA_BITS (CONFIG_ARM64_VA_BITS)
#define VA_START (UL(0xffffffffffffffff) << VA_BITS)
-#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1))
+#define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
#define KIMAGE_VADDR (MODULES_END)
#define MODULES_END (MODULES_VADDR + MODULES_VSIZE)
#define MODULES_VADDR (VA_START + KASAN_SHADOW_SIZE)
--
2.14.0.rc1.383.gd1ce394fe2-goog
[toc] | [prev] | [next] | [standalone]
| From | Nick Desaulniers <ndesaulniers@google.com> |
|---|---|
| Date | 2017-08-03 01:20 +0200 |
| Subject | Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL |
| Message-ID | <uab5M-8f2-7@gated-at.bofh.it> |
| In reply to | #1702544 |
hmm, seems including the definition of GENMASK_ULL causes tons of issues see definition of UL() macro defines _AC token pastes UL on literal when not assembly so looks like GENMASK_ULL is not ready to be used from assembly On Wed, Aug 2, 2017 at 4:13 PM, Nick Desaulniers <ndesaulniers@google.com> wrote: > don't forget to include linux/bitops.h now in memory.h > > /usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/kernel/head.S:47:8: > error: > function-like macro 'GENMASK_ULL' is not defined > #elif (PAGE_OFFSET & 0x1fffff) != 0 > ^ > /usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/include/asm/memory.h:52:22: > note: > > expanded from macro 'PAGE_OFFSET' > #define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1) > ^ > 1 error generated. > > On Wed, Aug 2, 2017 at 3:51 PM, Matthias Kaehlcke <mka@chromium.org> wrote: >> As is the definition causes an integer overflow, which is expected, >> however clang raises the following warning: >> >> arch/arm64/kernel/head.S:47:8: warning: >> integer overflow in preprocessor expression >> #elif (PAGE_OFFSET & 0x1fffff) != 0 >> ^~~~~~~~~~~ >> arch/arm64/include/asm/memory.h:52:46: note: >> expanded from macro 'PAGE_OFFSET' >> #define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1)) >> ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~ >> >> Use GENMASK_ULL() instead of shifting explicitly, the macro takes care >> of avoiding the overflow. >> >> Reported-by: Nick Desaulniers <ndesaulniers@google.com> >> Signed-off-by: Matthias Kaehlcke <mka@chromium.org> >> --- >> arch/arm64/include/asm/memory.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h >> index 32f82723338a..732d4eed8edd 100644 >> --- a/arch/arm64/include/asm/memory.h >> +++ b/arch/arm64/include/asm/memory.h >> @@ -65,7 +65,7 @@ >> */ >> #define VA_BITS (CONFIG_ARM64_VA_BITS) >> #define VA_START (UL(0xffffffffffffffff) << VA_BITS) >> -#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1)) >> +#define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1) >> #define KIMAGE_VADDR (MODULES_END) >> #define MODULES_END (MODULES_VADDR + MODULES_VSIZE) >> #define MODULES_VADDR (VA_START + KASAN_SHADOW_SIZE) >> -- >> 2.14.0.rc1.383.gd1ce394fe2-goog >> > > > > -- > Thanks, > ~Nick Desaulniers -- Thanks, ~Nick Desaulniers
[toc] | [prev] | [next] | [standalone]
| From | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| Date | 2017-08-03 01:50 +0200 |
| Subject | Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL |
| Message-ID | <uabyN-8qn-7@gated-at.bofh.it> |
| In reply to | #1702551 |
El Wed, Aug 02, 2017 at 04:19:11PM -0700 Nick Desaulniers ha dit: > hmm, seems including the definition of GENMASK_ULL causes tons of issues > see definition of UL() macro > defines _AC > token pastes UL on literal when not assembly > so looks like GENMASK_ULL is not ready to be used from assembly sorry, it seems I only did a partial build when testing this :/ > On Wed, Aug 2, 2017 at 4:13 PM, Nick Desaulniers > <ndesaulniers@google.com> wrote: > > don't forget to include linux/bitops.h now in memory.h > > > > /usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/kernel/head.S:47:8: > > error: > > function-like macro 'GENMASK_ULL' is not defined > > #elif (PAGE_OFFSET & 0x1fffff) != 0 > > ^ > > /usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/include/asm/memory.h:52:22: > > note: > > > > expanded from macro 'PAGE_OFFSET' > > #define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1) > > ^ > > 1 error generated. > > > > On Wed, Aug 2, 2017 at 3:51 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > >> As is the definition causes an integer overflow, which is expected, > >> however clang raises the following warning: > >> > >> arch/arm64/kernel/head.S:47:8: warning: > >> integer overflow in preprocessor expression > >> #elif (PAGE_OFFSET & 0x1fffff) != 0 > >> ^~~~~~~~~~~ > >> arch/arm64/include/asm/memory.h:52:46: note: > >> expanded from macro 'PAGE_OFFSET' > >> #define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1)) > >> ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~ > >> > >> Use GENMASK_ULL() instead of shifting explicitly, the macro takes care > >> of avoiding the overflow. > >> > >> Reported-by: Nick Desaulniers <ndesaulniers@google.com> > >> Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > >> --- > >> arch/arm64/include/asm/memory.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h > >> index 32f82723338a..732d4eed8edd 100644 > >> --- a/arch/arm64/include/asm/memory.h > >> +++ b/arch/arm64/include/asm/memory.h > >> @@ -65,7 +65,7 @@ > >> */ > >> #define VA_BITS (CONFIG_ARM64_VA_BITS) > >> #define VA_START (UL(0xffffffffffffffff) << VA_BITS) > >> -#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1)) > >> +#define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1) > >> #define KIMAGE_VADDR (MODULES_END) > >> #define MODULES_END (MODULES_VADDR + MODULES_VSIZE) > >> #define MODULES_VADDR (VA_START + KASAN_SHADOW_SIZE) > >> > > > > > > > > >
[toc] | [prev] | [next] | [standalone]
| From | Nick Desaulniers <ndesaulniers@google.com> |
|---|---|
| Date | 2017-08-03 01:20 +0200 |
| Subject | Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL |
| Message-ID | <uab5M-8f2-9@gated-at.bofh.it> |
| In reply to | #1702544 |
don't forget to include linux/bitops.h now in memory.h
/usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/kernel/head.S:47:8:
error:
function-like macro 'GENMASK_ULL' is not defined
#elif (PAGE_OFFSET & 0x1fffff) != 0
^
/usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/include/asm/memory.h:52:22:
note:
expanded from macro 'PAGE_OFFSET'
#define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
^
1 error generated.
On Wed, Aug 2, 2017 at 3:51 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> As is the definition causes an integer overflow, which is expected,
> however clang raises the following warning:
>
> arch/arm64/kernel/head.S:47:8: warning:
> integer overflow in preprocessor expression
> #elif (PAGE_OFFSET & 0x1fffff) != 0
> ^~~~~~~~~~~
> arch/arm64/include/asm/memory.h:52:46: note:
> expanded from macro 'PAGE_OFFSET'
> #define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1))
> ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
>
> Use GENMASK_ULL() instead of shifting explicitly, the macro takes care
> of avoiding the overflow.
>
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> arch/arm64/include/asm/memory.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 32f82723338a..732d4eed8edd 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -65,7 +65,7 @@
> */
> #define VA_BITS (CONFIG_ARM64_VA_BITS)
> #define VA_START (UL(0xffffffffffffffff) << VA_BITS)
> -#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1))
> +#define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
> #define KIMAGE_VADDR (MODULES_END)
> #define MODULES_END (MODULES_VADDR + MODULES_VSIZE)
> #define MODULES_VADDR (VA_START + KASAN_SHADOW_SIZE)
> --
> 2.14.0.rc1.383.gd1ce394fe2-goog
>
--
Thanks,
~Nick Desaulniers
[toc] | [prev] | [next] | [standalone]
| From | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| Date | 2017-08-03 19:10 +0200 |
| Message-ID | <uarNg-32O-21@gated-at.bofh.it> |
| In reply to | #1702541 |
El Thu, Aug 03, 2017 at 04:24:56PM +0300 Yury Norov ha dit: > On Wed, Aug 02, 2017 at 03:51:58PM -0700, Matthias Kaehlcke wrote: > > GENMASK_ULL performs a left-shift of (~0ULL), which technically > > results in an integer overflow. clang raises a warning about > > this if the overflow occurs in a preprocessor expression. To > > avoid the overflow first perform a right-shift to clear the > > bits that are shifted out. > > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > > --- > > include/linux/bitops.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > > index a83c822c35c2..21dfe63001e3 100644 > > --- a/include/linux/bitops.h > > +++ b/include/linux/bitops.h > > @@ -22,7 +22,7 @@ > > (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h)))) > > Once you touche GENMASK_ULL, why not to fix also GENMASK. Will do, thanks. Even though GENMASK_ULL can't be used (as is) to define the arm64 PAGE_OFFSET as initially intended it seems the change is still worthwhile since there are a few other preprocessor expressions using GENMASK_ULL.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web