C++ Logo

liaison

Advanced search

Re: [wg14/wg21 liaison] (SC22WG14.19256) C memory object model study group - uninitialised reads and padding

From: Uecker, Martin <Martin.Uecker_at_[hidden]>
Date: Thu, 15 Apr 2021 09:51:13 +0000


Am Donnerstag, den 15.04.2021, 08:44 +0100 schrieb Anthony Williams:
> On 14/04/2021 20:09, Uecker, Martin via Liaison wrote:
> > Am Mittwoch, den 14.04.2021, 21:51 +0300 schrieb Ville Voutilainen:
> > > On Wed, 14 Apr 2021 at 21:47, Jens Gustedt via Liaison
> > > <liaison_at_lists.isocpp.org> wrote:
> > > > Am 14. April 2021 20:07:18 MESZ schrieb JF Bastien <
> > > > cxx_at_[hidden]>:
> > > > > On Wed, Apr 14, 2021 at 11:00 AM Uecker, Martin <
> > > > > Martin.Uecker_at_[hidden]> wrote:
> > > > > > Am Mittwoch, den 14.04.2021, 08:54 -0700 schrieb JF Bastien
> > > > > > via Liaison:
> > > > > > > On Tue, Apr 13, 2021 at 11:40 AM Peter Sewell <
> > > > > > > Peter.Sewell_at_cl.cam.ac.uk>
> > > > > > > wrote:
> > > > > > > > - reading uninitialised representation bytes and
> > > > > > > > padding bytes is also
> > > > > > > > necessary for other bytewise polymorphic operations:
> > > > > > > > memcmp, marshalling,
> > > > > > > > encryption, and hashing (deferring what one knows
> > > > > > > > about the results of
> > > > > > > > such reads for a moment). It's not clear how generally
> > > > > > > > these operations
> > > > > > > > have to be supported, and we would like more
> > > > > > > > data. Atomic cmpxchg on large
> > > > > > > > structs, implemented with locks, would do a
> > > > > > > > memcmp/memcpy combination (in
> > > > > > > > fact is described as such in the standard).
> > > > > > > >
> > > > > > >
> > > > > > > For atomics with padding, C++20 adopted the following
> > > > > > > change (and I expect
> > > > > > > that compilers will implement it in previous versions as
> > > > > > > well):
> > > > > > > http://wg21.link/P0528
> > > > > >
> > > > > > I am not terribly excited about this solution.
> > > > > >
> > > > > > I think C should stick to the memcmp/memcpy semantics of
> > > > > > cmpxchg
> > > > > > which operate on the representation including padding. This
> > > > > > fits
> > > > > > to the hardware instructions, simplifies compiler design
> > > > > > (no
> > > > > > need to look into each type), is easy to explain, handles
> > > > > > all
> > > > > > cases consistently including unions, and is what most
> > > > > > C programmers would expect.
> > > > >
> > > > > OK, but it doesn't work, as explained in the paper.
> > > >
> > > > Well, there is actually not much of an explanation in the
> > > > paper.
> > > >
> > > > And doesn't work isn't much of a description
> > >
> > > You need to look at the R0 revision
> > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0528r0.html
> >
> > The closest thing which could apply to C is the following:
> >
> > Padded infloop_maybe(Atomic* atomic) {
> > Padded desired; // Padding unknown.
> > Padded expected; // Could be different.
> > peek("desired before", &desired);
> > peek("expected before", &expected);
> > peek("atomic before", atomic);
> > while (
> > !atomic->compare_exchange_strong(
> > expected,
> > desired // Padding bits added and removed here ˙ ͜ʟ˙
> > ));
> > peek("expected after", &expected);
> > peek("atomic after", atomic);
> > return expected; // Maybe changed here as well.
> > }
> >
> > But the claim that this can loop indefinitely seems wrong.
> >
> > The padding of desired is irrelevant.
> >
> > If the padding of 'expected' is different from
> > the padding of 'atomic', then there is one additional
> > executation of the loop where 'expected' is
> > updated to a version with the right padding. Then
> > in the next round the compare exchange succeeds.
>
> I agree with this.

Good. So at least we can agree that the motivating
example of P0528 does not show why "the memcpy/memcmp
semantics do not work".

> However, that is not where the problem lies, as I see it.
>
> Atomic atomic;
> Padded some_value=init();
>
> Thread 1:
> atomic=some_value;
>
> Thread 2:
> Padded local=some_value;
> Padded new_value=whatever();
>
> if(atomic->compare_exchange_strong(local,new_value))
> do_stuff();
>
>
> Note this is an "if", not a "while" --- I am checking to see if
> thread 1 did its write before thread 2 or after.
>
> If padding bits are counted in the comparison, then the compare may
> not succeed, even if the value is the expected one, because the
> padding bits in "local" might not match those in "some_value", or the
> padding bits written to "atomic".

This is true.

> For padding bits to count in the comparison in
> compare_exchange_strong, we must ensure that they are preserved
> everywhere.

Agreed, this is what we now assume a programmer needs to be
aware of and why the standard specifically mentions
memcpy/memcmp as the definition of the semantics of these
functions for exactly this reason.

There might be an opportunity to improve the situation, e.g.
by specifying that all atomic stores set padding bytes of structs
to zero (this will not work for structs), or by having additional
tools that help programmers to clear the padding (which would
also be useful for other things).

> C++ explicitly does not preserve them, so they cannot be part of the
> comparison. C can make a different choice.

This is what this discussion about. But first, it was important
to establish that the current text in C does not have a problem.

C++ could also change to treating padding bytes similar
to C which would also solve the problems with
atomic_exchange_compare (including unions), align it
with C, and make low-level programming manipulating
padding bytes possible.


Best,
Martin








Received on 2021-04-15 04:51:23