On 16/10/18 02:34, Stefano Stabellini wrote: > 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 <stefa...@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)) >
I don't see why the unsigned long cast is needed. What is wrong by having "char *ptr + 0" which is legal pointer arithmetic, and has well defined behaviour? If it is necessary, then you need a (typeof(x)) cast again on the outside, because the unsigned long cast causes RELOC_HIDE's typeof to be wrong overall. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
