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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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:
> >>> --- a/xen/arch/x86/alternative.c
> >>> +++ b/xen/arch/x86/alternative.c
> >>> @@ -194,7 +194,7 @@ void init_or_livepatch apply_alternatives(struct 
> >>> alt_instr *start,
> >>>        * So be careful if you want to change the scan order to any other
> >>>        * order.
> >>>        */
> >>> -    for ( a = base = start; a < end; a++ )
> >>> +    for ( a = base = start; SYMBOL(a) < SYMBOL(end); a++ )
> >> 
> >> At this point all is fine: end is allowed to point to the end of start[].
> > 
> > But it can also point to somewhere else. The compiler cannot know it, right?
> 
> Correct. My point is that at this point the compiler cannot use its
> knowledge of what is (il)legal to "optimize" generated code.
> 
> >>> @@ -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.

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.

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