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

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

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

See this LKML message for the concrete issue:


See this comment from this thread
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61502 because it is

  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 
  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)
  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)?

Xen-devel mailing list



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