diff mbox series

x86/tss: Fix clang build following c/s 7888440625

Message ID 20190813120117.22528-1-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86/tss: Fix clang build following c/s 7888440625 | expand

Commit Message

Andrew Cooper Aug. 13, 2019, 12:01 p.m. UTC
Clang-3.5 from Debian Jessie fails with:

  smpboot.c:829:29: error: statement expression not allowed at file scope
          BUILD_BUG_ON(sizeof(this_cpu(tss_page)) != PAGE_SIZE);
                              ^
  /local/xen.git/xen/include/asm/percpu.h:14:7: note: expanded from macro
          'this_cpu'
      (*RELOC_HIDE(&per_cpu__##var, get_cpu_info()->per_cpu_offset))
        ^
  /local/xen.git/xen/include/xen/compiler.h:98:3: note: expanded from macro
          'RELOC_HIDE'
    ({ unsigned long __ptr;                       \
    ^
  /local/xen.git/xen/include/xen/lib.h:26:53: note: expanded from macro
          'BUILD_BUG_ON'
  #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
                                                      ^
  /local/xen.git/xen/include/xen/lib.h:25:57: note: expanded from macro
          'BUILD_BUG_ON_ZERO'
  #define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
                                                          ^
  1 error generated.
  /local/xen.git/xen/Rules.mk:202: recipe for target 'smpboot.o' failed

This is obviously a compiler bug because the BUILD_BUG_ON() is not at file
scope.  However, it can be worked around by using a local variable.

Spotted by Gitlab CI.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/smpboot.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Wei Liu Aug. 14, 2019, 10:37 a.m. UTC | #1
On Tue, Aug 13, 2019 at 01:01:17PM +0100, Andrew Cooper wrote:
> Clang-3.5 from Debian Jessie fails with:
> 
>   smpboot.c:829:29: error: statement expression not allowed at file scope
>           BUILD_BUG_ON(sizeof(this_cpu(tss_page)) != PAGE_SIZE);
>                               ^
>   /local/xen.git/xen/include/asm/percpu.h:14:7: note: expanded from macro
>           'this_cpu'
>       (*RELOC_HIDE(&per_cpu__##var, get_cpu_info()->per_cpu_offset))
>         ^
>   /local/xen.git/xen/include/xen/compiler.h:98:3: note: expanded from macro
>           'RELOC_HIDE'
>     ({ unsigned long __ptr;                       \
>     ^
>   /local/xen.git/xen/include/xen/lib.h:26:53: note: expanded from macro
>           'BUILD_BUG_ON'
>   #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
>                                                       ^
>   /local/xen.git/xen/include/xen/lib.h:25:57: note: expanded from macro
>           'BUILD_BUG_ON_ZERO'
>   #define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
>                                                           ^
>   1 error generated.
>   /local/xen.git/xen/Rules.mk:202: recipe for target 'smpboot.o' failed
> 
> This is obviously a compiler bug because the BUILD_BUG_ON() is not at file
> scope.  However, it can be worked around by using a local variable.
> 
> Spotted by Gitlab CI.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Wei Liu <wl@xen.org>
Jan Beulich Aug. 27, 2019, 2:29 p.m. UTC | #2
On 13.08.2019 14:01, Andrew Cooper wrote:
> Clang-3.5 from Debian Jessie fails with:
> 
>    smpboot.c:829:29: error: statement expression not allowed at file scope
>            BUILD_BUG_ON(sizeof(this_cpu(tss_page)) != PAGE_SIZE);
>                                ^
>    /local/xen.git/xen/include/asm/percpu.h:14:7: note: expanded from macro
>            'this_cpu'
>        (*RELOC_HIDE(&per_cpu__##var, get_cpu_info()->per_cpu_offset))
>          ^
>    /local/xen.git/xen/include/xen/compiler.h:98:3: note: expanded from macro
>            'RELOC_HIDE'
>      ({ unsigned long __ptr;                       \
>      ^
>    /local/xen.git/xen/include/xen/lib.h:26:53: note: expanded from macro
>            'BUILD_BUG_ON'
>    #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
>                                                        ^
>    /local/xen.git/xen/include/xen/lib.h:25:57: note: expanded from macro
>            'BUILD_BUG_ON_ZERO'
>    #define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
>                                                            ^
>    1 error generated.
>    /local/xen.git/xen/Rules.mk:202: recipe for target 'smpboot.o' failed
> 
> This is obviously a compiler bug because the BUILD_BUG_ON() is not at file
> scope.  However, it can be worked around by using a local variable.

It can be worked around, yes, but the result is not identical:

> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -826,9 +826,11 @@ static int setup_cpu_root_pgt(unsigned int cpu)
>           rc = clone_mapping(idt_tables[cpu], rpt);
>       if ( !rc )
>       {
> -        BUILD_BUG_ON(sizeof(this_cpu(tss_page)) != PAGE_SIZE);
> +        struct tss_page *this_tss = &per_cpu(tss_page, cpu);

Whatever type is compatible with struct tss_page * will now be
permitted on the rhs. Hence ...

> -        rc = clone_mapping(&per_cpu(tss_page, cpu).tss, rpt);
> +        BUILD_BUG_ON(sizeof(*this_tss) != PAGE_SIZE);

... this sizeof() is not necessarily checking the size of the
per-CPU variable. But anyway - the change has been committed
already, and the chances of this going wrong are low enough, so
please don't treat this as an objection or revert request.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 5057109a77..85c81c247e 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -826,9 +826,11 @@  static int setup_cpu_root_pgt(unsigned int cpu)
         rc = clone_mapping(idt_tables[cpu], rpt);
     if ( !rc )
     {
-        BUILD_BUG_ON(sizeof(this_cpu(tss_page)) != PAGE_SIZE);
+        struct tss_page *this_tss = &per_cpu(tss_page, cpu);
 
-        rc = clone_mapping(&per_cpu(tss_page, cpu).tss, rpt);
+        BUILD_BUG_ON(sizeof(*this_tss) != PAGE_SIZE);
+
+        rc = clone_mapping(&this_tss->tss, rpt);
     }
     if ( !rc )
         rc = clone_mapping((void *)per_cpu(stubs.addr, cpu), rpt);