Message ID | 20170410164610.7cty25z4y3wedcy4@dhcp-3-128.uk.xensource.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
>>> On 10.04.17 at 18:46, <roger.pau@citrix.com> wrote: > I have a (I think) less intrusive fix, which relies on using _Pragma, pasted > below. Let me know what you think, and I can formally submit it. Personally I like this much better as a workaround, provided the omitted warning isn't just cosmetic (i.e. the generated code then also matches gcc's rather than doing what the warning suggests clang does). I do think, however, that the comment would better go next to the #define. Jan
On Tue, Apr 11, 2017 at 12:21:23AM -0600, Jan Beulich wrote: > >>> On 10.04.17 at 18:46, <roger.pau@citrix.com> wrote: > > I have a (I think) less intrusive fix, which relies on using _Pragma, pasted > > below. Let me know what you think, and I can formally submit it. > > Personally I like this much better as a workaround, provided the > omitted warning isn't just cosmetic (i.e. the generated code then > also matches gcc's rather than doing what the warning suggests > clang does). I do think, however, that the comment would better > go next to the #define. Yes, the generated code matches gcc behavior. I've been told that what might behave differently is something like: switch ( foo ) { case 1: while ( ({ if ( bar ) { x = 1; break; } x; }) ) { ... But this is not the case clearly, and the clang warning is simply a bug. OK, so let me move the comment with the defines and submit the patch properly, thanks. Roger.
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h index 2fbe705518..d24e30c3df 100644 --- a/xen/include/asm-x86/atomic.h +++ b/xen/include/asm-x86/atomic.h @@ -43,8 +43,23 @@ build_add_sized(add_u64_sized, "q", uint64_t, "ri") void __bad_atomic_size(void); +/* + * NB: we need to disable the gcc-compat warnings for clang in read_atomic or + * else it will complain with: "'break' is bound to loop, GCC binds it to + * switch" when read_atomic is used inside of a while expression inside of a + * switch statement, ie: + * + * switch (...) + * { + * case ...: + * while ( read_atomic(&foo) ) { ... } + * + * This has already been reported upstream: + * http://bugs.llvm.org/show_bug.cgi?id=32595 + */ #define read_atomic(p) ({ \ unsigned long x_; \ + CLANG_DISABLE_WARN_GCC_COMPAT_START \ switch ( sizeof(*(p)) ) { \ case 1: x_ = read_u8_atomic((uint8_t *)(p)); break; \ case 2: x_ = read_u16_atomic((uint16_t *)(p)); break; \ @@ -52,6 +67,7 @@ void __bad_atomic_size(void); case 8: x_ = read_u64_atomic((uint64_t *)(p)); break; \ default: x_ = 0; __bad_atomic_size(); break; \ } \ + CLANG_DISABLE_WARN_GCC_COMPAT_END \ (typeof(*(p)))x_; \ }) diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h index 16aeeea7f1..569dcb70d6 100644 --- a/xen/include/xen/compiler.h +++ b/xen/include/xen/compiler.h @@ -100,4 +100,15 @@ # define ASM_FLAG_OUT(yes, no) no #endif +#ifdef __clang__ +# define CLANG_DISABLE_WARN_GCC_COMPAT_START \ + _Pragma("clang diagnostic push") \ + _Pragma("clang diagnostic ignored \"-Wgcc-compat\"") +# define CLANG_DISABLE_WARN_GCC_COMPAT_END \ + _Pragma("clang diagnostic pop") +#else +# define CLANG_DISABLE_WARN_GCC_COMPAT_START +# define CLANG_DISABLE_WARN_GCC_COMPAT_END +#endif + #endif /* __LINUX_COMPILER_H */