[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL

Jan Beulich writes ("Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL"):
> On 06.02.19 at 16:41, <ian.jackson@xxxxxxxxxx> wrote:
> > (i) define indirection variables eg end_ in an assembly language file.
> > (ii) convert to uintptr_t before comparing
> > 
> > (i) is IMO wholly safe but it is a bit ugly and slightly less
> > performant.
> > 
> > I think (ii) is fairly safe.  I doubt that we will find (ii) broken.
> > In particular, because there is little motivation for compiler authors
> > to try to `optimise it'.
> If you want to be "prepared" for them taking apart asm()-s, how
> would you expect them to never "look through" casts?

I'm not sure what you mean by `look through casts'.  I expect the
compiler to `look through casts' for both value and provenance

But comparing uintptr_t's is never UB, no matter their value or
provenance.  So there is simply unarguably no UB if the comparison is
done with uintptr_t's.  The conversions themselves are ID, not UB.

Whether comparing pointer values is UB depends on their value and
provenance, and in general the compiler is able to look through most
transformations (including casts) to determine the ultimate provenance
- ie the provenance rules are not defeated by casts.

So that's de jure.

De facto, there is little incentive for a compiler to misoptimise
uintptr_t comparisons, and much incentive for it to get `better' at
disentangling pointer provenance for the purpose of (mis)optimising
pointer comparisons.

> >  extern const struct wombat _wombats_start[];
> >  extern const struct abstract_symbol _wombats_end[];
> Hmm, that's certainly an interesting approach, and requires
> care only when introducing a new pair of symbols of this kind.
> But of course the macro you suggest to carry out the
> comparison will have the same weakness as any open coded
> cast to a suitable integer type. But there are benefits:

libxl already relies on casting to uintptr_t as a way of avoiding the
rules restricting pointer comparisons.  See these comments:
  libxl_event.c     l.476  libxl__watch_slot_contents
  libxl_internal.h  l.322  typedef struct libxl__ev_watch_slot

> - it marks problem sites clearly (one of Stefano's goals),
> - it isolates future changes to how exactly the comparisons
>   are to be done to the comparison macro(s)
> - it doesn't undermine type safety of the main (start-of-
>   whatever) symbols (one of my goals),
> - it allows the end-of-whatever symbols to also be handed to
>   functions in a type-safe manner

Yes.  However...

>   (we could even have per-instance structure flavors, such that
>   unrelated "end" symbols can't be mixed up; for example the type
>   could simply be a structure wrapping a field of the original base
>   type).

I think this would be difficult to achieve without writing a forbidden
pointer comparison in the macro.  It could perhaps be achieved with
typeof() if that is available in hypervisor code.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.