[PATCH 0/3] gcc 7 build fixes (hypervisor side)

[PATCH 0/3] gcc 7 build fixes (hypervisor side)

[Jan Beulich]

I think it would be good for 4.9 to build out of the box with this recently
released compiler version.

1: xmalloc: correct _xmalloc_array() indentation
2: x86: fix build with gcc 7
3: arm: fix build with gcc 7

Signed-off-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 0/3] gcc 7 build fixes (hypervisor side)

[Julien Grall]

Hi Jan,

On 18/05/17 09:01, Jan Beulich wrote:
> I think it would be good for 4.9 to build out of the box with this recently
> released compiler version.

I don't see the 3 patches on the ML. Is there any SMTP problem?
>
> 1: xmalloc: correct _xmalloc_array() indentation
> 2: x86: fix build with gcc 7
> 3: arm: fix build with gcc 7
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 1/3] xmalloc: correct _xmalloc_array() indentation

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

It's been wrongly indented using tabs till now, and the stray blank
ahead of the final return statement gets in the way of using .i files
for detailed analysis of other compiler issues
(-Wmisleading-indentation kickin in due to the tab->space
transformation done in the course of pre-processing).

Also add missing spaces inside the if() at once, including the similar
case in _xzalloc_array().

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/xen/xmalloc.h
+++ b/xen/include/xen/xmalloc.h
@@ -33,17 +33,17 @@ extern void *_xzalloc(unsigned long size
 static inline void *_xmalloc_array(
     unsigned long size, unsigned long align, unsigned long num)
 {
-	/* Check for overflow. */
-	if (size && num > UINT_MAX / size)
-		return NULL;
- 	return _xmalloc(size * num, align);
+    /* Check for overflow. */
+    if ( size && num > UINT_MAX / size )
+        return NULL;
+    return _xmalloc(size * num, align);
 }
 
 static inline void *_xzalloc_array(
     unsigned long size, unsigned long align, unsigned long num)
 {
     /* Check for overflow. */
-    if (size && num > UINT_MAX / size)
+    if ( size && num > UINT_MAX / size )
         return NULL;
     return _xzalloc(size * num, align);
 }




[-- Attachment #2: xmalloc_array-indent.patch --]
[-- Type: text/plain, Size: 1304 bytes --]

xmalloc: correct _xmalloc_array() indentation

It's been wrongly indented using tabs till now, and the stray blank
ahead of the final return statement gets in the way of using .i files
for detailed analysis of other compiler issues
(-Wmisleading-indentation kickin in due to the tab->space
transformation done in the course of pre-processing).

Also add missing spaces inside the if() at once, including the similar
case in _xzalloc_array().

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/xen/xmalloc.h
+++ b/xen/include/xen/xmalloc.h
@@ -33,17 +33,17 @@ extern void *_xzalloc(unsigned long size
 static inline void *_xmalloc_array(
     unsigned long size, unsigned long align, unsigned long num)
 {
-	/* Check for overflow. */
-	if (size && num > UINT_MAX / size)
-		return NULL;
- 	return _xmalloc(size * num, align);
+    /* Check for overflow. */
+    if ( size && num > UINT_MAX / size )
+        return NULL;
+    return _xmalloc(size * num, align);
 }
 
 static inline void *_xzalloc_array(
     unsigned long size, unsigned long align, unsigned long num)
 {
     /* Check for overflow. */
-    if (size && num > UINT_MAX / size)
+    if ( size && num > UINT_MAX / size )
         return NULL;
     return _xzalloc(size * num, align);
 }

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 2/3] x86: fix build with gcc 7

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]

-Wint-in-bool-context, enabled by default in gcc 7, doesn't like
multiplication in conditional operators. "Hide" them, at the risk of
the next compiler version becoming smarter and recognizing even those.
(The hope is that added smartness then would also better deal with
legitimate cases like the ones here.)

The change could have been done in access_ok(), but I think we better
keep it at the places the compiler is actually unhappy about.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3318,7 +3318,7 @@ static void emulate_gate_op(struct cpu_u
                 return;
             }
             stkp = (unsigned int *)(unsigned long)((unsigned int)base + esp);
-            if ( !compat_access_ok(stkp - 4 - nparm, (4 + nparm) * 4) )
+            if ( !compat_access_ok(stkp - 4 - nparm, 16 + nparm * 4) )
             {
                 do_guest_trap(TRAP_gp_fault, regs);
                 return;
@@ -3338,7 +3338,7 @@ static void emulate_gate_op(struct cpu_u
                     return do_guest_trap(TRAP_gp_fault, regs);
                 ustkp = (unsigned int *)(unsigned long)
                         ((unsigned int)base + regs->esp + nparm * 4);
-                if ( !compat_access_ok(ustkp - nparm, nparm * 4) )
+                if ( !compat_access_ok(ustkp - nparm, 0 + nparm * 4) )
                 {
                     do_guest_trap(TRAP_gp_fault, regs);
                     return;
--- a/xen/include/asm-x86/x86_64/uaccess.h
+++ b/xen/include/asm-x86/x86_64/uaccess.h
@@ -42,7 +42,7 @@ extern void *xlat_malloc(unsigned long *
 
 #define array_access_ok(addr, count, size) \
     (likely(((count) ?: 0UL) < (~0UL / (size))) && \
-     access_ok(addr, (count) * (size)))
+     access_ok(addr, 0 + (count) * (size)))
 
 #define __compat_addr_ok(d, addr) \
     ((unsigned long)(addr) < HYPERVISOR_COMPAT_VIRT_START(d))
@@ -55,7 +55,7 @@ extern void *xlat_malloc(unsigned long *
 
 #define compat_array_access_ok(addr,count,size) \
     (likely((count) < (~0U / (size))) && \
-     compat_access_ok(addr, (count) * (size)))
+     compat_access_ok(addr, 0 + (count) * (size)))
 
 #define __put_user_size(x,ptr,size,retval,errret)			\
 do {									\




[-- Attachment #2: gcc7-x86.patch --]
[-- Type: text/plain, Size: 2310 bytes --]

x86: fix build with gcc 7

-Wint-in-bool-context, enabled by default in gcc 7, doesn't like
multiplication in conditional operators. "Hide" them, at the risk of
the next compiler version becoming smarter and recognizing even those.
(The hope is that added smartness then would also better deal with
legitimate cases like the ones here.)

The change could have been done in access_ok(), but I think we better
keep it at the places the compiler is actually unhappy about.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3318,7 +3318,7 @@ static void emulate_gate_op(struct cpu_u
                 return;
             }
             stkp = (unsigned int *)(unsigned long)((unsigned int)base + esp);
-            if ( !compat_access_ok(stkp - 4 - nparm, (4 + nparm) * 4) )
+            if ( !compat_access_ok(stkp - 4 - nparm, 16 + nparm * 4) )
             {
                 do_guest_trap(TRAP_gp_fault, regs);
                 return;
@@ -3338,7 +3338,7 @@ static void emulate_gate_op(struct cpu_u
                     return do_guest_trap(TRAP_gp_fault, regs);
                 ustkp = (unsigned int *)(unsigned long)
                         ((unsigned int)base + regs->esp + nparm * 4);
-                if ( !compat_access_ok(ustkp - nparm, nparm * 4) )
+                if ( !compat_access_ok(ustkp - nparm, 0 + nparm * 4) )
                 {
                     do_guest_trap(TRAP_gp_fault, regs);
                     return;
--- a/xen/include/asm-x86/x86_64/uaccess.h
+++ b/xen/include/asm-x86/x86_64/uaccess.h
@@ -42,7 +42,7 @@ extern void *xlat_malloc(unsigned long *
 
 #define array_access_ok(addr, count, size) \
     (likely(((count) ?: 0UL) < (~0UL / (size))) && \
-     access_ok(addr, (count) * (size)))
+     access_ok(addr, 0 + (count) * (size)))
 
 #define __compat_addr_ok(d, addr) \
     ((unsigned long)(addr) < HYPERVISOR_COMPAT_VIRT_START(d))
@@ -55,7 +55,7 @@ extern void *xlat_malloc(unsigned long *
 
 #define compat_array_access_ok(addr,count,size) \
     (likely((count) < (~0U / (size))) && \
-     compat_access_ok(addr, (count) * (size)))
+     compat_access_ok(addr, 0 + (count) * (size)))
 
 #define __put_user_size(x,ptr,size,retval,errret)			\
 do {									\

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 3/3] arm: fix build with gcc 7

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1900 bytes --]

The compiler dislikes duplicat "const", and the ones it complains about
look like they we in fact meant to be placed differently.

Also fix array_access_okay() (just like on x86), despite the construct
being unused on ARM: -Wint-in-bool-context, enabled by default in
gcc 7, doesn't like multiplication in conditional operators. "Hide" it,
at the risk of the next compiler version becoming smarter and
recognizing even that. (The hope is that added smartness then would
also better deal with legitimate cases like the one here.) The change
could have been done in access_ok(), but I think we better keep it at
the place the compiler is actually unhappy about.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Note: Build tested only.

--- a/xen/arch/arm/platforms/brcm.c
+++ b/xen/arch/arm/platforms/brcm.c
@@ -271,7 +271,7 @@ static __init int brcm_init(void)
     return brcm_populate_plat_regs();
 }
 
-static const char const *brcm_dt_compat[] __initconst =
+static const char *const brcm_dt_compat[] __initconst =
 {
     "brcm,bcm7445d0",
     NULL
--- a/xen/arch/arm/platforms/rcar2.c
+++ b/xen/arch/arm/platforms/rcar2.c
@@ -46,7 +46,7 @@ static int __init rcar2_smp_init(void)
     return 0;
 }
 
-static const char const *rcar2_dt_compat[] __initdata =
+static const char *const rcar2_dt_compat[] __initconst =
 {
     "renesas,lager",
     NULL
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -8,7 +8,8 @@
 #define access_ok(addr,size) (1)
 
 #define array_access_ok(addr,count,size) \
-    (likely(count < (~0UL/size)) && access_ok(addr,count*size))
+    (likely((count) < (~0UL / (size))) && \
+     access_ok(addr, 0 + (count) * (size)))
 
 unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len);
 unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,




[-- Attachment #2: gcc7-arm.patch --]
[-- Type: text/plain, Size: 1923 bytes --]

arm: fix build with gcc 7

The compiler dislikes duplicat "const", and the ones it complains about
look like they we in fact meant to be placed differently.

Also fix array_access_okay() (just like on x86), despite the construct
being unused on ARM: -Wint-in-bool-context, enabled by default in
gcc 7, doesn't like multiplication in conditional operators. "Hide" it,
at the risk of the next compiler version becoming smarter and
recognizing even that. (The hope is that added smartness then would
also better deal with legitimate cases like the one here.) The change
could have been done in access_ok(), but I think we better keep it at
the place the compiler is actually unhappy about.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Note: Build tested only.

--- a/xen/arch/arm/platforms/brcm.c
+++ b/xen/arch/arm/platforms/brcm.c
@@ -271,7 +271,7 @@ static __init int brcm_init(void)
     return brcm_populate_plat_regs();
 }
 
-static const char const *brcm_dt_compat[] __initconst =
+static const char *const brcm_dt_compat[] __initconst =
 {
     "brcm,bcm7445d0",
     NULL
--- a/xen/arch/arm/platforms/rcar2.c
+++ b/xen/arch/arm/platforms/rcar2.c
@@ -46,7 +46,7 @@ static int __init rcar2_smp_init(void)
     return 0;
 }
 
-static const char const *rcar2_dt_compat[] __initdata =
+static const char *const rcar2_dt_compat[] __initconst =
 {
     "renesas,lager",
     NULL
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -8,7 +8,8 @@
 #define access_ok(addr,size) (1)
 
 #define array_access_ok(addr,count,size) \
-    (likely(count < (~0UL/size)) && access_ok(addr,count*size))
+    (likely((count) < (~0UL / (size))) && \
+     access_ok(addr, 0 + (count) * (size)))
 
 unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len);
 unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 0/3] gcc 7 build fixes (hypervisor side)

[Jan Beulich]

>>> On 18.05.17 at 10:34, <julien.grall@arm.com> wrote:
> On 18/05/17 09:01, Jan Beulich wrote:
>> I think it would be good for 4.9 to build out of the box with this recently
>> released compiler version.
> 
> I don't see the 3 patches on the ML. Is there any SMTP problem?

They should be there now (or really soon).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] xmalloc: correct _xmalloc_array() indentation

[Wei Liu]

On Thu, May 18, 2017 at 02:39:45AM -0600, Jan Beulich wrote:
> It's been wrongly indented using tabs till now, and the stray blank
> ahead of the final return statement gets in the way of using .i files
> for detailed analysis of other compiler issues
> (-Wmisleading-indentation kickin in due to the tab->space

kickin -> kicks

> transformation done in the course of pre-processing).
> 
> Also add missing spaces inside the if() at once, including the similar
> case in _xzalloc_array().
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] xmalloc: correct _xmalloc_array() indentation

[Andrew Cooper]

On 18/05/17 10:40, Wei Liu wrote:
> On Thu, May 18, 2017 at 02:39:45AM -0600, Jan Beulich wrote:
>> It's been wrongly indented using tabs till now, and the stray blank
>> ahead of the final return statement gets in the way of using .i files
>> for detailed analysis of other compiler issues
>> (-Wmisleading-indentation kickin in due to the tab->space
> kickin -> kicks
>
>> transformation done in the course of pre-processing).
>>
>> Also add missing spaces inside the if() at once, including the similar
>> case in _xzalloc_array().
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/3] x86: fix build with gcc 7

[Andrew Cooper]

On 18/05/17 09:40, Jan Beulich wrote:
> -Wint-in-bool-context, enabled by default in gcc 7, doesn't like
> multiplication in conditional operators. "Hide" them, at the risk of
> the next compiler version becoming smarter and recognizing even those.
> (The hope is that added smartness then would also better deal with
> legitimate cases like the ones here.)
>
> The change could have been done in access_ok(), but I think we better
> keep it at the places the compiler is actually unhappy about.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 3/3] arm: fix build with gcc 7

[Julien Grall]

Hi,

On 18/05/17 09:41, Jan Beulich wrote:
> The compiler dislikes duplicat "const", and the ones it complains about

s/duplicat/duplicate/

> look like they we in fact meant to be placed differently.
>
> Also fix array_access_okay() (just like on x86), despite the construct
> being unused on ARM: -Wint-in-bool-context, enabled by default in
> gcc 7, doesn't like multiplication in conditional operators. "Hide" it,
> at the risk of the next compiler version becoming smarter and
> recognizing even that. (The hope is that added smartness then would
> also better deal with legitimate cases like the one here.) The change
> could have been done in access_ok(), but I think we better keep it at
> the place the compiler is actually unhappy about.

I am wondering if we should drop array_access_ok and access_ok as they 
are not used.

Anyway, I am happy with both way:

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 0/3] gcc 7 build fixes (hypervisor side)

[Julien Grall]

Hi Jan,

On 18/05/17 09:01, Jan Beulich wrote:
> I think it would be good for 4.9 to build out of the box with this recently
> released compiler version.
>
> 1: xmalloc: correct _xmalloc_array() indentation
> 2: x86: fix build with gcc 7
> 3: arm: fix build with gcc 7
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

For the 3 fixes:

Release-acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 3/3] arm: fix build with gcc 7

[Stefano Stabellini]

On Thu, 18 May 2017, Julien Grall wrote:
> Hi,
> 
> On 18/05/17 09:41, Jan Beulich wrote:
> > The compiler dislikes duplicat "const", and the ones it complains about
> 
> s/duplicat/duplicate/
> 
> > look like they we in fact meant to be placed differently.
> > 
> > Also fix array_access_okay() (just like on x86), despite the construct
> > being unused on ARM: -Wint-in-bool-context, enabled by default in
> > gcc 7, doesn't like multiplication in conditional operators. "Hide" it,
> > at the risk of the next compiler version becoming smarter and
> > recognizing even that. (The hope is that added smartness then would
> > also better deal with legitimate cases like the one here.) The change
> > could have been done in access_ok(), but I think we better keep it at
> > the place the compiler is actually unhappy about.
> 
> I am wondering if we should drop array_access_ok and access_ok as they are not
> used.
> 
> Anyway, I am happy with both way:
> 
> Reviewed-by: Julien Grall <julien.grall@arm.com>

Is this series for 4.9?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 3/3] arm: fix build with gcc 7

[Jan Beulich]

>>> On 18.05.17 at 17:12, <julien.grall@arm.com> wrote:
> On 18/05/17 09:41, Jan Beulich wrote:
>> The compiler dislikes duplicat "const", and the ones it complains about
> 
> s/duplicat/duplicate/
> 
>> look like they we in fact meant to be placed differently.
>>
>> Also fix array_access_okay() (just like on x86), despite the construct
>> being unused on ARM: -Wint-in-bool-context, enabled by default in
>> gcc 7, doesn't like multiplication in conditional operators. "Hide" it,
>> at the risk of the next compiler version becoming smarter and
>> recognizing even that. (The hope is that added smartness then would
>> also better deal with legitimate cases like the one here.) The change
>> could have been done in access_ok(), but I think we better keep it at
>> the place the compiler is actually unhappy about.
> 
> I am wondering if we should drop array_access_ok and access_ok as they 
> are not used.

I did consider this too, but thought that such a decision (and patch)
would better come from someone closer to ARM.

> Anyway, I am happy with both way:
> 
> Reviewed-by: Julien Grall <julien.grall@arm.com>

Thanks, Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 3/3] arm: fix build with gcc 7

[Jan Beulich]

>>> On 18.05.17 at 20:35, <sstabellini@kernel.org> wrote:
> On Thu, 18 May 2017, Julien Grall wrote:
>> Hi,
>> 
>> On 18/05/17 09:41, Jan Beulich wrote:
>> > The compiler dislikes duplicat "const", and the ones it complains about
>> 
>> s/duplicat/duplicate/
>> 
>> > look like they we in fact meant to be placed differently.
>> > 
>> > Also fix array_access_okay() (just like on x86), despite the construct
>> > being unused on ARM: -Wint-in-bool-context, enabled by default in
>> > gcc 7, doesn't like multiplication in conditional operators. "Hide" it,
>> > at the risk of the next compiler version becoming smarter and
>> > recognizing even that. (The hope is that added smartness then would
>> > also better deal with legitimate cases like the one here.) The change
>> > could have been done in access_ok(), but I think we better keep it at
>> > the place the compiler is actually unhappy about.
>> 
>> I am wondering if we should drop array_access_ok and access_ok as they are not
>> used.
>> 
>> Anyway, I am happy with both way:
>> 
>> Reviewed-by: Julien Grall <julien.grall@arm.com>
> 
> Is this series for 4.9?

Yes, as mentioned in 0/3.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 3/3] arm: fix build with gcc 7

[Julien Grall]

Hi Jan,

On 19/05/2017 07:16, Jan Beulich wrote:
>>>> On 18.05.17 at 17:12, <julien.grall@arm.com> wrote:
>> On 18/05/17 09:41, Jan Beulich wrote:
>>> The compiler dislikes duplicat "const", and the ones it complains about
>>
>> s/duplicat/duplicate/
>>
>>> look like they we in fact meant to be placed differently.
>>>
>>> Also fix array_access_okay() (just like on x86), despite the construct
>>> being unused on ARM: -Wint-in-bool-context, enabled by default in
>>> gcc 7, doesn't like multiplication in conditional operators. "Hide" it,
>>> at the risk of the next compiler version becoming smarter and
>>> recognizing even that. (The hope is that added smartness then would
>>> also better deal with legitimate cases like the one here.) The change
>>> could have been done in access_ok(), but I think we better keep it at
>>> the place the compiler is actually unhappy about.
>>
>> I am wondering if we should drop array_access_ok and access_ok as they
>> are not used.
>
> I did consider this too, but thought that such a decision (and patch)
> would better come from someone closer to ARM.

I will send a patch for this patch after 4.9.

Cheers,

>
>> Anyway, I am happy with both way:
>>
>> Reviewed-by: Julien Grall <julien.grall@arm.com>
>
> Thanks, Jan
>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel