Re: [Xen-devel] [PATCH v4 2/2] xen: use SYMBOL when required

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.11.18 at 23:02, <sstabellini@xxxxxxxxxx> wrote:
> On Tue, 13 Nov 2018, Jan Beulich wrote:
>> >>> On 13.11.18 at 14:17, <Julien.Grall@xxxxxxx> wrote:
>> > On 13/11/2018 12:56, Jan Beulich wrote:
>> >>>>> On 13.11.18 at 00:06, <sstabellini@xxxxxxxxxx> wrote:
>> >>> @@ -33,8 +33,8 @@ static int init_percpu_area(unsigned int cpu)
>> >>>       if ( (p = alloc_xenheap_pages(PERCPU_ORDER, 0)) == NULL )
>> >>>           return -ENOMEM;
>> >>>   
>> >>> -    memset(p, 0, __per_cpu_data_end - __per_cpu_start);
>> >>> -    __per_cpu_offset[cpu] = p - __per_cpu_start;
>> >>> +    memset(p, 0, SYMBOL(__per_cpu_data_end) - SYMBOL(__per_cpu_start));
>> >>> +    __per_cpu_offset[cpu] = (unsigned long)p - SYMBOL(__per_cpu_start);
>> >> 
>> >> Can't you make SYMBOL() retain the original type, such that casts
>> >> like this one aren't needed? As soon as the compiler doesn't know
>> >> anymore that particular globals (or statics) are used, it can't infer
>> >> anymore that two pointers can't possibly point into the same array.
>> > 
>> > If SYMBOL() keeps the original type, then you will still substract 2 
>> > pointers. If the compiler can't infer the cannot possibly point into the 
>> > same array, it also cannot infer they point to the same. So that would 
>> > be undefined, right?
>> 
>> Undefined behavior results if you _actually_ subtract pointers pointing
>> into different objects. Subtracting of pointers is not generally undefined.
>> The compiler can use undefined-ness only if it can prove that both
>> pointers do point into different objects.
> 
> Let's remember that we are not trying to work-around the compiler, we
> are trying to make our code C standard compliant :-)  The compiler might
> not be able to infer anymore that two pointers can't possibly point into
> the same array, but we would still be not-compliant. It doesn't solve
> our problem, especially in regards to certifications.

But then this entire patch is pointless: SYMBOL() is exclusively about
deluding the compiler. To make the code standard compliant, you'd
have to e.g. do away with all combined (start and end) uses (in C
files) of symbols bounding sections. I at least cannot think of a
standard compliant way of expressing these. Oddly enough I had
once tried to deal with this issue (for other reasons), but the patch
wasn't liked:
https://lists.xenproject.org/archives/html/xen-devel/2016-08/msg02718.html
All the remaining end symbols then could obviously go away in favor
of using the size expressions, but as you see further C limitations
make it necessary to use asm() for the ones which get converted.

Talking of asm()-s: C standard compliance, in a strict sense, would
require dropping all of them as well. I'm afraid that when writing
special purpose code like OS kernels or hypervisors are, if you
want to avoid to resort extensively to assembly code, you'll have
to accept to bend some of the language rules, just making sure
that the compiler won't have means to mis-interpret the constructs
used.

> I is safer to use unsigned long as return type for SYMBOL and avoid
> pointers comparisons completely. The code impact is very limited and
> then we don't have to prove same or different "objectness" at all.

Well, that's one perspective to take. The other is that hidden or
explicit casts are always a risk (and hence when reviewing code
I'm quite picky about any ones introduced anew or even just
retained without reason). Making constructs needing to cast
things at least finally cast back to the original type often at least
lowers this risk.

Jan



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