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

[Stewart Hildebrand <Stewart.Hildebrand@xxxxxxxxxxxxxxx>]

On Friday, January 11, 2019 1:04 PM, Stefano Stabellini wrote: 
> On Fri, 11 Jan 2019, Jan Beulich wrote:
> > >>> On 11.01.19 at 03:14, <sstabellini@xxxxxxxxxx> wrote:
> > > Hi Juergen, Jan,
> > >
> > > I spoke with Julien: we are both convinced that the unsigned long
> > > solution is best. But Julien also did some research and he thinks that
> > > Jan's version (returning pointer type) not only does not help with
> > > MISRA-C, but also doesn't solve the potential GCC problem either. A
> > > description of the GCC issue is available here:
> > >
> > > https://kristerw.blogspot.com/2016/12/pointer-comparison-invalid-optimization.html?m=1
> >
> > I've read through it, and besides not agreeing with some of the
> > author's arguments I wasn't able to spot where it tells me why/how
> > the suggested approach doesn't solve the problem.
> >
> > > (Also keep in mind that Linux uses the unsigned long solution to solve
> > > the GCC issue, deviating from it doesn't seem wise.)
> >
> > Which specific gcc issue (that is not solved by retaining type)?
> 
> I am hoping Julien and his team will be able to provide the more
> decisive information next week for us to make a decision, but it looks
> like the issue is not clear-cut and people on the GCC list disagree on
> how it should be handled.
> 
> 
> The C standard says that "Two pointers compare equal if and only if both
> are null pointers, both are pointers to the same object (including a
> pointer to an object and a subobject at its beginning) or function, both
> are pointers to one past the last element of the same array object, or
> one is a pointer to one past the end of one array object and the other
> is a pointer to the start of a different array object that happens to
> immediately follow the first array object in the address space."
> 
> In short, the compiler is free to return false in a pointer comparison
> if it believes that the pointers point to different non-consecutive
> object.
> 
> 
> See this LKML message for the concrete issue:
> 
> https://lkml.org/lkml/2016/6/25/77
> 
> 
> See this comment from this thread
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61502 because it is
> enlightening:
> 
>   Just because two pointers print the same and have the same bit-pattern
>   doesn't mean they need to compare equal
> 
> Also this:
> 
>   > --- Comment #14 from Keith Thompson <Keith.S.Thompson at gmail dot com> 
> ---
>   > The C standard requires that, if y "happens to immediately follow"
>   > x in the address space, then a pointer just past the end of x shall
>   > compare equal to a pointer to the beginning of y (C99 and C11 6.5.9p6).
>   >
>   > How could I distinguish the current behavior of gcc from the behavior
>   > of a hypothetical C compiler that violates that requirement? In
>   > other words, in what sense does gcc actually obey that requirement?
> 
>   They are not distinguishable [...]
> 
> Finally continuing down the thread there is an example from the Linux
> kernel itself:
> 
>   Apparently some folks use linker scripts to get a specific arrangement of 
> objects.
> 
>   A fresh example is a problem in Linux -- https://lkml.org/lkml/2016/6/25/77 
> . A simplified example from
> http://pastebin.com/4Qc6pUAA :
> 
>   extern int __start[];
>   extern int __end[];
> 
>   extern void bar(int *);
> 
>   void foo()
>   {
>       for (int *x = __start; x != __end; ++x)
>           bar(x);
>   }
> 
>   This is optimized into an infinite loop by gcc 7 at -O.
> 
> 
> There is also a suggested workaround on the thread that uses assembly
> inline like we do and casts to int*. Overall, reading the blog post and
> the thread on the GCC bugzilla, I get the idea that comparing pointers
> like we do can be unreliable.
> 
> The limit of Jan's solution is that even if we go through an assembly
> indirection, we are still comparing pointers. We are opening ourselves
> up to trouble. The unsigned long solution looks safer, moreover, it
> puts us in the same bandwagon as the Linux kernel, which is as good as
> it gets as a guarantee that compilers won't break this behavior.
> 
> With the issue so unclear, do we feel confident enough to choose the
> more risky solution of the two (returning pointer type)?
> 

NO! Definitely not.

The issue seems to be that we are interpreting the linker-generated values
as pointer types, and then going on to do comparisons, subtractions, etc. on
those values. It seems that GCC's idea of what comprises a "pointer to an
object" does not take linker-generated values into account, then goes on to
make the assumption that two linker-provided values are "pointers to
different objects". Given the ambiguity of the C standard, one could probably
successfully argue that GCC did nothing wrong. I would argue that we are
relying on undefined behavior in a strict interpretation of the C standard.

The important message to take away from MISRA is that use of or reliance on
implementation-defined behavior should be understood and documented. In
fact that's the very first MISRA rule: Directive 1.1.

However, it would be even better to avoid having to rely on
implementation-defined behavior in the first place...
So here's a radical idea:

Why don't we change the type of _start so it's not a pointer type?
The MISRA rules in question (18.1 - 18.4) only pertain to pointer types,
so if _start isn't a pointer type, it should silence PRQA. Also, it seems like 
the
majority of references to _start/_end/etc. don't actually dereference the
value (i.e. not actually using the value as a pointer). And for cases where we
do need to dereference it (i.e. actually use it as a pointer) then introduce an
explicit cast, which would also serve as a hint for "yes, I really do know what
I'm doing in regards to the whole pointers to different objects issue".

Or as an alternative to the cast, introduce a new union type, with one member
as a sufficient-width integer type (for doing arithmetic, comparisons, etc.) and
another member as a pointer type. This would explicitly force us to consider
how exactly the value is being used each time it's referenced and choose the
appropriate interpretation.

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