Re: [Xen-devel] [PATCH for-4.13] xen/arm: Don't use _end in is_xen_fixed_mfn()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 16 Oct 2019, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [PATCH for-4.13] xen/arm: Don't use _end in 
> is_xen_fixed_mfn()"):
> > My suggestion is going to work: "the compiler sees through casts"
> > referred to comparisons between pointers, where we temporarily casted
> > both pointers to integers and back to pointers via a MACRO. That case
> > was iffy because the MACRO was clearly a workaround the spec.
> > 
> > Here the situation is different. For one, we are doing arithmetic. Also
> > virt_to_maddr already takes a vaddr_t as argument. So instead of doing
> > pointers arithmetic, then converting to vaddr_t, we are converting to
> > vaddr_t first, then doing arithmetics, which is fine both from a C99
> > point of view and even a MISRA C point of view. I can't see a problem
> > with that. I am sure as I reasonable can be :-)
> 
> FTAOD I think you are suggesting this:
>  - +     (mfn_to_maddr(mfn) <= virt_to_maddr(_end - 1)))
>  + +     (mfn_to_maddr(mfn) <= virt_to_maddr(((vaddr_t)_end - 1)))
> 
> virt_to_maddr(va) is a macro which expands to
>    __virt_to_maddr((vaddr_t)(va))
> 
> So what is happening here is that the cast to an integer type is being
> done before the subtraction.
> 
> Without the cast, you are calculating the pointer value _end-1 from
> the value _end, which is UB.  With the cast you are calculating an
> integer value.  vaddr_t is unsigned, so all arithmetic operations are
> defined.  Nothing casts the result back to the "forbidden" (with this
> provenance) pointer value, so all is well.
> 
> (With the macro expansion the cast happens twice.  This is probably
> better than using __virt_to_maddr here.)
> 
> Ie, in this case I agree with Stefano.  The cast is both necessary and
> sufficient.

Thanks Ian for the second pair of eyes!

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel