Re: [Xen-devel] [PATCH] x86/vmx: fix build with clang 3.8.0

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.02.17 at 14:45, <roger.pau@xxxxxxxxxx> wrote:
> On Thu, Feb 09, 2017 at 06:14:54AM -0700, Jan Beulich wrote:
>> >>> On 09.02.17 at 14:05, <andrew.cooper3@xxxxxxxxxx> wrote:
>> > On 09/02/17 13:01, Jan Beulich wrote:
>> >>>>> On 09.02.17 at 13:49, <andrew.cooper3@xxxxxxxxxx> wrote:
>> >>> On 09/02/17 11:33, Roger Pau Monne wrote:
>> >>>> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
>> >>>> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
>> >>>> @@ -602,15 +602,16 @@ void vmx_pi_hooks_assign(struct domain *d);
>> >>>>  void vmx_pi_hooks_deassign(struct domain *d);
>> >>>>  
>> >>>>  /* EPT violation qualifications definitions */
>> >>>> -typedef union __transparent__ ept_qual {
>> >>>> +typedef union ept_qual {
>> >>> Please can we use
>> >>>
>> >>> typedef __transparent__ union ept_qual {
>> >>>
>> >>> which clang is happy with, and will help avoid problems such as the
>> >>> cper_mce_record issue in c/s f8be76e2fe
>> >> Would clang also be happy with it moved near the end of that
>> >> line
>> >>
>> >> typedef union ept_qual __transparent__ {
>> >>
>> >> Having the attribute ahead of "union" is, I think, strictly speaking
>> >> undefined behavior, as it then may as well apply to "typedef".
>> > 
>> > No.  The result is
>> > 
>> > /local/xen.git/xen/include/asm/hvm/vmx/vmx.h:605:40: error: expected
>> > identifier or '('
>> > typedef union ept_qual __transparent__ {
>> >                                        ^
>> > /local/xen.git/xen/include/asm/hvm/vmx/vmx.h:614:3: error: type
>> > specifier missing, defaults to 'int' [-Werror,-Wimplicit-int]
>> > } ept_qual_t;
>> >   ^~~~~~~~~~
>> > 2 errors generated.
>> > 
>> > In which case the original patch as proposed will probably do.  It turns
>> > out the presence of ept_qual_t does cause a compiler error if
>> > __transparent__ is missing from scope.
>> 
>> But then the question is what the attribute applies to in the original
>> version - the union, or just the typedef? The placement would
>> suggest the latter, so I'd again be afraid of undefined behavior. Can
>> it be moved ahead on that line?
> 
> This is what the clang folks seem to test:
> 
> https://github.com/llvm-mirror/clang/blob/master/test/Sema/transparent-union.c
>  
> 
> So I would keep it with the current semantics, to stay in line with what 
> they do.

At the risk of tripping over a future gcc change in behavior? Better
not ...

Jan


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