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

[George Dunlap <dunlapg@xxxxxxxxx>]

On Tue, Jan 22, 2019 at 9:17 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
>
> >>> On 22.01.19 at 00:41, <sstabellini@xxxxxxxxxx> wrote:
> > We haven't managed to reach consensus on this topic. Your view might be
> > correct, but it is not necessarily supported by compilers' behavior,
> > which depends on the opinion of compilers engineers on the topic, and
> > MISRAC compliance, which depends on the opinion of MISRAC specialists on
> > the topic. If we take your suggested approach we end up with the code
> > most likely to break in case the compilers engineers or the MISRAC
> > experts disagree with you. In this case, being right doesn't necessarily
> > lead to the code less likely to break.
> >
> > Regardless, if that is the decision of the Xen community as a whole,
> > I'll follow it. My preference remains with approach 3. (var.S), followed
> > by approach 2. (SYMBOL_HIDE returns uintptr_t), but I am willing to
> > refresh my series to do approach 1. (SYMBOL_HIDE returns pointer type)
> > if that is the only way forward.
> >
> > Let us come to a conclusion so that we can move on.
>
> How can we come to a conclusion when things remain unclear? I see
> only two ways forward - either we settle the dispute (which I'm
> afraid would require involvement of someone accepted by all of us
> as a "C language lawyer", which would include judgment about the
> MISRA-C implications),

Well, no, I don't think a "C language lawyer" would help here.

You keep making the case for C spec compliance as though we're dealing
with zealous but ultimately rational people, who will a) almost never
violate the C spec, and b) actually fix their compiler if their
language does violate the spec.

But it's clear from reading those threads that this is not the case.
Over and over people presented  clear arguments, from the spec and the
spec committee, showing that what gcc was doing was both wrong and
impractical; and the compiler people kept coming up with more and more
tortuous interpretations to justify the compiler's behavior.  The
whole thing with supposing that the C standard anticipated a
compacting garbage collector was the cherry on the cake.

We're not living in a rational world where if we just follow the rules
we'll be safe.  We have a dictat from the high council in the form of
a C spec which is divorced from actual usage and utility, and we have
a load of insane fanatics trying to impose their interpretation
doctrinal purity on the world, and ready to burn any heretics writing
code that doesn't match the view they happen to hold that day.  "The
compiler can't optimize this because it can't prove they're different
objects" is a fine principle, but it's pretty clear that they're
willing to continue optimizing things even if you *can* prove they
fall inside the rules of pointer comparison.

In such a situation, *there is no perfectly safe option*.  No matter
what position you take, the fanatics may end up deciding that you're a
heretic and need to be burned at the stake.  Might they decide that
they know that extern pointers point to different objects, and
therefore can't be compared? Maybe!  Might they decide they can pierce
the veil of asm to determine the source of unsigned longs they're
comparing? Possibly!  Could they decide that a uintptr_t received from
the heathen lands of assembly is anathema, and therefore casting it to
a pointer is undefined behavior?  They certainly could!

And even if you do convince them their interpretation is wrong and
they fix their compiler, the damage is still done: there are still,
out in the wild, vulnerable binaries built with buggy compilers and
buggy compilers that produce vulnerable binaries, until they all die
of old age.

*Any* interpretation we choose may be declared at some point by the
compiler folks to be heresy.  But, there are less safe option and more
safe options.  Our goal with regard to the C Standard cannot,
unfortunately, be "follow the rules".  Our goal must be to *minimize
the risk* of being caught in the next wave of the compiler
optimization purges.

MISRA is quirky and often impractical, but ultimately their goal with
rules like this is to try to protect you from the fanatics who write
compilers (insofar as that is possible).  So if we do our best to be
as safe as possible from the compiler fanatics, we have a pretty good
chance of being considered MISRA compliant as well.

It seems to me that anything that involves directly comparing pointers
is simply more likely to be come the target of optimization (and thus
more dangerous) than comparing unsigned long and uintptr_t.  And
although I'm not terribly familiar with the "intptr" types, it seems
to me that casting from uintptr_t is less likely to ever be considered
deviant behavior than casting from unsigned long.

As such, I think casting the return value of asm to a pointer is far
too dangerous.  Using extern pointers seems quite dangerous to me as
well.  So it seems to me that using asm to generate an unsigned long
would be absolute minimum behavior.  Using uintprt_t values, and in
particular importing them from assembly, might give us yet another
level of safety (in case unsigned long -> pointer casts ever become a
target).

Are these guaranteed to avoid "UB hazard" issues in the future?  Of
course not; nothing can.  But they seem to me to be a lot less risky
than asm -> ptr or extern pointers.

> Only at that point can we then decide whether any of
> the proposed "solutions" (in quotes because I remain unconvinced
> there's a problem to solve here other than working around compiler
> bugs) is/are necessary _and_ fulfilling the purpose, and if multiple
> remain, which of them we like best / is the least bad one.

Improvements this series seeks to make, as I understand it, include
(in order of urgency):

1. Fixing one concrete instance of "UB hazard"
2. Minimize risk of further "UB hazard" in this bit of functionality
3. Retain the effort Stefano has put in identifying all the places
where such UB hazards need to be addressed.
4. Move towards MISRA-C compliance.

As far as I can tell, primary objections you've leveled at the options
which try to address 2-4 are:

a. "UB hazard" still not zero
b. MISRA compliancy no 100%
c. Ugly
d. Inefficient

(Obviously some proposals have had more technical discussion.)

Anything I missed?

 -George

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