5

In C++, it's well-defined to read from an union member which was most recently written, aka the active union member.

My question is whether std::memcpying a whole union object, as opposed to copying a particular union member, to an uninitialized memory area will preserve the active union member.

union A {
    int x;
    char y[4];
};

A a;
a.y[0] = 'U';
a.y[1] = 'B';
a.y[2] = '?';
a.y[3] = '\0';

std::byte buf[sizeof(A)];
std::memcpy(buf, &a, sizeof(A));

A& a2 = *reinterpret_cast<A*>(buf);

std::cout << a2.y << '\n'; // is `A::y` the active member of `a2`?
  • 2
    Modern C++ code will use a type-safe `std::variant` instead of a union, rendering this whole question moot. – Sam Varshavchik Jun 08 '20 at 00:39
  • 5
    @SamVarshavchik Even assuming that's true, I still think the question is valid. – cigien Jun 08 '20 at 00:42
  • 1
    Common sense says it should work for unions of only trivially-copyable types, but I'm not completely sure. – HolyBlackCat Jun 08 '20 at 00:43
  • I'm working on a piece of implementation code where the index-safety overhead of `std::variant` (checking for index match and throwing `std::bad_variant_access`), although cheap, is not required. –  Jun 08 '20 at 00:43
  • 1
    @SepiaColor It would not be terribly hard to write something similar without the type indexes. Of course it couldn't be directly copyable/moveable but it could still be assigned if you know what type is inside. – cdhowie Jun 08 '20 at 01:07
  • 3
    Since you tagged with `undefined-behavior`, I'll point out that you should use placement new instead of `malloc` since `malloc` doesn't actually create objects as far as the language is concerned. You might also have to throw in a `std::launder` on access to be fully in the clear. I don't remember how much of this changed in C++20, but I know there wasn't enough time to get Richard's full paper in. (Also, you should at least be able to replace `*reinterpret_cast` with `reinterpret_cast`.) – chris Jun 08 '20 at 01:12
  • @cdhowie I need the copy and move semantics, though. I'm thinking right now. If I must use an `std::variant` and avoid the index-safety overhead, I could store both an `std::variant` and a `void` pointer to the union member stored in the variant, since I know, at all times, which union member I'll be operating on. –  Jun 08 '20 at 01:17
  • 1
    @chris I see, but I'm not using `malloc`. Since the example union, `A`, has only trivially-copyable types, I don't think I have the need of placement new or `std::construct_at()`. And since neither union member is const-qualified, neither should I use `std::launder()`. Or am I wrong? –  Jun 08 '20 at 01:26
  • 1
    @SepiaColor, I think I horribly misread the memcpy in your example as malloc, yikes. Granted, it is a similar case where no `A` object exists there. It's not just for calling constructors, but telling the compiler what it can and can't assume. As for `launder`, I always need to re-research it for finer points, which is why I said might. Theoretically, you have an array of `std::byte` at that address and now you want an `A` instead, which is similar to the `const` usage. The logic there is telling the compiler not to assume there's only `std::byte[]`, but again, hazy finer points. – chris Jun 08 '20 at 01:41
  • 1
    All that said, I haven't seen compilers try too hard to optimize around object lifetime of trivial types and hopefully it will be simplified in the future to be mostly what people expect without having to be so explicit about it. – chris Jun 08 '20 at 01:44

4 Answers4

0

The assignments you have are okay because the assignment to non-class member a.y "begins its lifetime". However, your std::memcpy does not do that, so any accesses to members of a2 are invalid. So, you are relying on the consequences of undefined behaviour. Technically. In practice most toolchains are rather lax about aliasing and lifetime of primitive-type union members.

Unfortunately, there's more UB here, in that you're violating aliasing for the union itself: you can pretend that a T is a bunch of bytes, but you can't pretend that a bunch of bytes is a T, no matter how much reinterpret_casting you do. You could instantiate an A a2 normally and std::copy/std::memcpy over it from a, and then you're just back to the union member lifetime problem, if you care about that. But, I guess, if this option were open to you, you'd just be writing A a2 = a in the first place…

Asteroids With Wings
  • 17,071
  • 2
  • 21
  • 35
  • Well, if I'm not able to pretend a bunch of bytes is a `T`, then what is `reinterpret_cast` for? I guess what you meant is the fact you're `reinterpret_cast`ing a byte buffer will not lead the compiler into initiating the lifetime of the reinterpreted object. That's why you have to use placement new. But since the union I presented is only made of trivially-copyable types, I wondered if it is an exception. –  Jun 08 '20 at 03:07
  • 1
    @SepiaColor It's for providing access according to [these rules](http://eel.is/c++draft/basic.lval#11). I'm not aware of any such exemption for your case. Unions aren't actually very useful; they were more useful decades ago when compilers were glorified wrappers around assembly, and the notion of C being an abstraction was a bit of a joke... but, particularly for C++, that really isn't the case any more. – Asteroids With Wings Jun 08 '20 at 11:11
  • Since you think unions aren't very useful, you'd use `std::variant` instead for such cases, I take it? I wouldn't complain about `std::variant` if it supported type-unsafe access. And to me this is more of a design problem rather than a performance one. Standard containers such as `std::vector` provide an array subscript operator and a function `at` for, respectively, when you can and cannot assume things about the container. `std::variant` doesn't give you such option. As a result, your code will always reflect the idea of uncertainty, even when it shouldn't. –  Jun 08 '20 at 15:56
  • 1
    @SepiaColor I would. But I concede that there is no way to use it without index checks. I think the argument for that is usually not as strong as it would be for a vector (where fast access to possibly loads of elements is desired, and when you know the range won't change between those accesses). But I suppose there could be some variant usage in a tight loop, in theory... – Asteroids With Wings Jun 08 '20 at 15:57
  • @SepiaColor Oh, also, another usage of `reinterpret_cast` is that you can go back and forth between some pointer types, e.g. `void*`, for storage. So access is not the only consideration. – Asteroids With Wings Jun 08 '20 at 16:00
  • Just to illustrate my point above. Say you see this piece of code: `const int i = 10; if (i == 10) /* do something */`. What would you think of it? Since `i` is not declared `volatile` and is always 10, it doesn't make sense to have an if-branch. Same goes with `std::variant`. –  Jun 08 '20 at 16:01
  • @SepiaColor Yep, I'm following. But there are pros and cons to any design. How often is your variant usage as contrived as that? If you know the type statically right there, why are you even using a variant? `const int i = 10` is useful to give `10` a name rather than being a magic number; there's no equivalent concept for using a variant. – Asteroids With Wings Jun 08 '20 at 16:03
  • It should be up to the variant user/programmer as whether there are such contrived usages or not. Bounds-unchecked usage of `std::vector` can give you UB if not used properly. Why not `std::variant`? But maybe it's like you said, some implementation limitation. –  Jun 08 '20 at 16:08
  • 1
    @SepiaColor I can't quite answer why it wasn't given as an option: either some complexity, or they just didn't see the need to provide a dangerous access mechanism in this case. The original proposal may answer this, if you can find it. – Asteroids With Wings Jun 08 '20 at 16:10
  • 1
    @SepiaColor http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0088r3.html Happy reading ;) – Asteroids With Wings Jun 08 '20 at 16:12
  • Looks like they just wanted it to be type-safe in all cases. – Asteroids With Wings Jun 08 '20 at 16:18
  • I'll give it a look, thanks. Wrt wanting it to be type-safe in all cases, I mean, this is C++, not Java lmao –  Jun 08 '20 at 16:25
  • If it's not an implementation limitation, user could provide an `always_safe_variant` on top of a possibly type-unsafe `std::variant` if it's what they want... –  Jun 08 '20 at 16:27
  • 1
    @SepiaColor I agree the committee seems to oscillate between wanting everything to be Fisher Price baby-safe ("we must call it `std::move`, even though it does no such thing, because that's how most people will use it and they shouldn't care about details!") and wanting everything to be esoteric and super-dangerous (literally everything to do with templates). But then that's what you get from committees: camels. – Asteroids With Wings Jun 08 '20 at 17:05
  • @AsteroidsWithWings: I don't think the Committee members ever had a consensus understanding of which corner cases should or should not be regarded as working predictably. If some members who think a case should be defined interpret the Standard as defining it, while those who think it should be UB interpret the Standard that way, any attempt to make the Standard unambiguous will be opposed by at least one of those groups, making consensus impossible. – supercat Jun 13 '20 at 20:33
0

My question is whether std::memcpying a whole union object, as opposed to copying a particular union member, to an uninitialized memory area will preserve the active union member.

It'll be copied as expected.

It's how you're reading the result that may, or may not, make your program have undefined behavior.


Using std::memcpy copies char's from one source to a destination. Raw memory copying is ok. Reading from memory as something it wasn't initialized to be is not ok.

Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
0

So far as I can tell, the C++ Standard makes no distinction between the following two functions on platforms where the sizes of int and foo would happen to be identical [as would typically be the case]

struct s1 { int x; };
struct s2 { int x; };
union foo { s1 a; s2 b; } u1, u2;
void test1(void)
{
  u1.a.x = 1;
  u2.b.x = 2;
  std::memcpy(&u1, &u2, sizeof u1);
}
void test2(void)
{
  u1.a = 1;
  u2.b = 2;
  std::memcpy(&u1.a.x, &u2.b.x, sizeof u1.a.x);
}

If a union of trivially-copyable types is a trivially-copyable type, that would suggest that the active member of u1 after the memcpy in test1 should be b. In the equivalent function test2, however, copying all the bytes from an int object into an one that's part of active union member s1.a should leave the active union member as a.

IMHO, this issue could be easily resolved by recognizing that a union may have multiple "potentially active" members, and allowing certain actions to be performed on any member that is at least potentially active (rather than limiting them to one particular active member). That would among other things allow the Common Initial Sequence rule to be made much clearer more useful, without unduly inhibiting optimizations, by specifying that the act of taking the address of a union member makes it "at least potentially" active, until the next time the union is written via non-character-based access, and providing that common-initial-sequence inspection or bytewise writing of a potentially active union member is allowed, but does not change the active member.

Unfortunately, when the Standard was first written, there was no effort to explore all of the relevant corner cases, much less reach a consensus about how they should be handled. At the time, I don't think there would have been opposition to the idea of officially accommodating multiple potentially-active members, since most compiler designs would naturally accommodate that without difficulty. Unfortunately, some compilers have evolved in a way that would make support for such constructs more difficult than it would have been if accommodated from the start, and their maintainers would block any changes that would contradict their design decisions, even if the Standard was never intended to allow such decisions in the first place.

supercat
  • 77,689
  • 9
  • 166
  • 211
0

Before I answer your question, I think your code should add this:

static_assert(std::is_trivial<A>());

Because in order to keep compatibility with C, trivial types get extra guarantees. For example, the requirement of running an object's constructor before using it (See https://eel.is/c++draft/class.cdtor) applies only to an object whose constructor is not trivial.


Because your union is trivial, your code is fine up to and including the memcpy. Where you run into trouble is *reinterpret_cast<A*>(buf);

Specifically, you are using an A object before its lifetime has begin.

As stated in https://eel.is/c++draft/basic.life , lifetime begins when storage with proper alignment and size for the type has been obtained, and its initialization is complete. A trivial type has "vacuous" initialization, so no problem there, however the storage is a problem.

When your example gets storage for buf,

std::byte buf[sizeof(A)];

It does not obtain proper alignment for the type. You'd need to change that line to:

alignas(A) std::byte buf[sizeof(A)];
jorgbrown
  • 1,993
  • 16
  • 23