On 10/16/19 11:18 AM, 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.
Maybe I missed something, but why are we using `<=` at all? Why not use
`<`?
Or is this some weird C pointer comparison UB thing?
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
