Re: [Xen-devel] [PATCH for-4.9 v2 2/2] x86/dm: fix clang build

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Mon, Apr 10, 2017 at 04:02:37AM -0600, Jan Beulich wrote:
> >>> On 10.04.17 at 11:46, <roger.pau@xxxxxxxxxx> wrote:
> > On Mon, Apr 10, 2017 at 03:27:35AM -0600, Jan Beulich wrote:
> >> >>> On 10.04.17 at 11:01, <roger.pau@xxxxxxxxxx> wrote:
> >> > --- a/xen/arch/x86/hvm/dm.c
> >> > +++ b/xen/arch/x86/hvm/dm.c
> >> > @@ -408,9 +408,16 @@ static int dm_op(domid_t domid,
> >> >          {
> >> >              struct p2m_domain *p2m = p2m_get_hostp2m(d);
> >> >  
> >> > -            while ( read_atomic(&p2m->ioreq.entry_count) &&
> >> > -                    first_gfn <= p2m->max_mapped_pfn )
> >> > +            while ( first_gfn <= p2m->max_mapped_pfn )
> >> >              {
> >> > +                /*
> >> > +                 * NB: read_atomic cannot be used in the loop condition 
> >> > because
> >> > +                 * clang complains with: "'break' is bound to loop, GCC 
> >> > binds
> >> > +                 * it to switch", so move it inside of the loop instead.
> >> > +                 */
> >> > +                if ( !read_atomic(&p2m->ioreq.entry_count) )
> >> > +                    break;
> >> 
> >> How is this behavior of clang in line with the language spec, namely
> >> "A break statement terminates execution of the smallest enclosing
> >> switch or iteration statement"?
> > 
> > I don't think the condition itself is part of the iteration statement (AFAIK
> > the condition is the expression, not the statement).
> 
> I don't understand (and if I take your wording verbally, then you're
> giving even more reason for clang being wrong, but I think taking it
> verbally would be wrong - "while" itself in my understanding very
> much belongs to the iteration statement, as does the condition; the
> body of the while statement then is _another_ statement, but not
> necessarily an iteration one anymore). Anyway, in
> 
>     while ( ({ switch ( x ) { default: break; } }) );
> 
> I don't think there's any question what "the smallest enclosing switch
> or iteration statement" is.

I think the language might not be clear on this case, the definition of a while
loop statement is:

while ( expression ) statement

And then the definition of break:

A break statement terminates execution of the smallest enclosing switch or
iteration statement.

I agree completely with you that the "break" should _only_ apply to the inner
switch, regardless of where that switch is placed. In this case it's used
inside of an expression, so maybe that's an excuse for the clang guys to get
creative?

That's why I've added the comment, because I think this is non-obvious.

Roger.

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