Re: [PATCH 3/4] xen: introduce __symbol

[Stefano Stabellini <sstabellini@kernel.org>]

On Mon, 15 Oct 2018, Julien Grall wrote:
> Hi,
> 
> Please use scripts/get_maintainers.pl to CC relevant maintainers.
> 
> On 15/10/2018 10:56, Stefano Stabellini wrote:
> > Introduce a macro, __symbol, which is a simple wrapper around RELOC_HIDE
> > to be used everywhere symbols such as _stext and _etext are used in the
> > code.
> > 
> > RELOC_HIDE is needed when accessing symbols such as _stext and _etext
> > because the C standard forbids comparisons between pointers pointing to
> > different objects. _stext, _etext, etc. are all pointers to different
> > objects from ANCI C point of view.
> > 
> > To work around potential C compiler issues (which have actually
> > been found, see the comment on top of RELOC_HIDE in Linux), and to help
> > with certifications, let's introduce some syntactic sugar to be used in
> > following patches.
> > 
> > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> > ---
> >   xen/include/asm-arm/mm.h   | 6 ++++++
> >   xen/include/asm-x86/page.h | 6 ++++++
> >   2 files changed, 12 insertions(+)
> > 
> > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> > index 940b74b..02ce05a 100644
> > --- a/xen/include/asm-arm/mm.h
> > +++ b/xen/include/asm-arm/mm.h
> > @@ -284,6 +284,12 @@ static inline uint64_t gvirt_to_maddr(vaddr_t va,
> > paddr_t *pa,
> >   #define __mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT))
> >     /*
> > + * Use RELOC_HIDE with symbols such as _stext and _etext to avoid errors
> > + * on comparing pointers to different objects
> > + */
> > +#define __symbol(x)         ((char *)RELOC_HIDE((unsigned long)(x), 0))
> 
> There are no different between arm and x86. A better place would be
> xen/compiler.h (or something common).

OK


> But, after this patch, there are even more chance the compiler will consider
> that 2 _symbol(...) will come from different objects. So how is this meant to
> help here?

I think the mistake was to cast the return to char*. If I remove the
cast, as Andrew suggested, then any comparison would be a comparison
between unsigned long, that should be accepted and safe.

However, the parameter to RELOC_HIDE has to be casted to unsigned long,
because most often we pass char*:

/local/repos/xen-upstream/xen/include/xen/compiler.h:100:5: error: cast specifies array type
     (typeof(ptr)) (__ptr + (off)); })

So I think the __symbol macro should be:

+#define __symbol(x)         (RELOC_HIDE((unsigned long)(x), 0))


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