UB when type punning through unions

Consider the following code:

static_assert(sizeof(float) == sizeof(int));
union U {
  float f;
  int i;
} u = { 1.0f };
int i = u.i;

This is performing a type pun through a union. In C, this code has well-defined behavior, whereas in C++, this code has undefined behavior. However, despite this being UB in C++, it is an incredibly common idiom to see in practice. As a result, even with strict aliasing enabled, neither Clang nor GCC takes advantage of the UB so long as the accesses are done through the union object itself. Our sanitizers do not diagnose the undefined behavior either: Compiler Explorer Essentially, accesses through unions all end up using an omnipotent char type for the accesses.

GCC documents this guarantee: Optimize Options (Using the GNU Compiler Collection (GCC))

I propose that Clang documents the same (or very similar) guarantees by defining the behavior explicitly as being the same in C and C++. I don’t think we can realistically start to aggressively optimize based on this UB due to how much existing code would break, so we might as well tell our users it’s safe to rely on.

6 Likes

I agree; as long as the memory access is done directly through the union, there’s no reason we can’t define this as valid.

I’m sympathetic to the effort of admitting status quo, but if we’re getting an extension out of gray area, I’d like to see something resembling C++ Standard wording describing what the status quo is from the C++ perspective. For instance, when I’m looking at a C++ conformance test, it would be useful to distinguish between “missing extension warning” and genuine “accepts invalid” case.

Here’s my poor attempt at wording if I correctly assume that this idiom requires class member access syntax where object expression is of union-like ([class.union.anon]) type:

If a class member access expression ([expr.ref]), that is not the left operand of an assignment operator or a subexpression thereof, designates an object O1 that is a variant member ([class.union.anon]) of type T, then an object O2 of the type T whose address is the same as address of O1 is implicitly created in the storage of O1; object representation of O2 is the contents of the storage of O1.

Notably, I’m not sure how good it handles members of array type and nested unions.

Note that we actually broke this back in Clang 4 and fixed it in Clang 5 (I remember because it caused us a bunch of downstream pain); see the discussions in RFC: Representing unions in TBAA, 32056 – Invalid code generation (aliasing between scalar and vector type), ⚙ D31885 Remove TBAA information from LValues representing union members, and ⚙ D33328 [CodeGen] Pessimize aliasing for union members (and may-alias) objects. I’d be in favor of adding the explicit guarantee here, since lots of places rely on it and we’ve made efforts to preserve it when it got broken in the past.

2 Likes

I don’t think we can formalize this as implicitly creating an object there, which would end the lifetime of the previous object. We also can’t make it as broad as any member access, because we can’t reasonably make u.classMember.foo() well-defined.

Perhaps something more like:

If a member access expression to a union member (or a subobject thereof) is (if of non-class, non-array type) converted to an r-value or (if of class type) used as the argument of a call to a trivial copy or move constructor or assignment operator, and the union object has a different active member than the member named, and the storage of the named member (or subobject) lies within the storage of the active union member, then the access is well-defined as if an object of the accessed type did exist at that address, and the memory is reinterpreted as if it were copied into a value of the resulting type by memcpy.

I’m not sure if we need to do anything for the left operand of assignment operators, because the existing logic for changing the active member always seems to apply.

I think documenting this make sense, I will point out that I believe the gcc documentation is could be improved on in many ways. It does not explicitly mention this is undefined behavior in C++, it follows from it being UB that it is not standard conformant but it is probably worth it to explicitly say so.

I think it would be more helpful to not say there are standards conforming alternatives even if this not elaborated on.

This does not have to be a deep research project to document but we can improve on the level of details provided in the gcc docs.

I agree we cannot break this guarantee, since it’s widely used. BUT, this language extension is an extremely poor fit for C++ code, because in C++, reference binding is silent, which causes users to break the “directly through the union” policy in ways they were not expecting.

So (inasmuch as we think TBAA should even exist in the first place…), I think we really ought to discourage anyone from using this extension. Folks should just use memcpy already (or std::bit_cast where applicable).

Why not? start_lifetime_as’ing union members as they are accessed should bring us back into the set of defined behaviors, as they require objects to be within their lifetime. Additional benefit is that this approach follows the existing wording for how assignment makes union members active.

I don’t think u.classMember.foo() can designate a variant member, so this is not allowed under my previous, surely flawed wording. u.classMember can, but the updated wording (below) require variant members to be of standard-layout type, so this should be fixed. Updated version is below.

This wording implies that arguments of calls to trivial special member functions do not undergo lvalue-to-rvalue conversion, which is surprising to me. Mind leaving some references?


Updated wording, with the help of @katzdm, incorporating @AaronBallman findings.

Should be considered after [class.union.general]/5, which describes how assignment operator makes a union member active:

Otherwise, for a class member access expression E1.E2 ([expr.ref]) where E2 designates a variant member V of a union-like class C ([class.union.anon]), all variant members of which are of standard-layout type, the evaluation of an lvalue-to-rvalue conversion applied to E1.E2 implicitly begins the lifetime of an object O for which

  • O is the subobject of the object designated by E1 corresponding to V ([class.mem.general]), and
  • O has the same address and object representation as the active member of the object designated by E1.

[ Note: id-expression that denotes a non-static non-type member of a class can be transformed into a class member access expression ([expr.prim.id.general]). — end note ]

Formally ending the lifetime of the existing member could make programs invalid that are valid under the standard. Like, you’re allowed to take the address of the active union member and just pass it around, and the exception we’re making here doesn’t help with that because the subsequent accesses don’t syntactically involve accessing a union member.

The u.classMember portion of that designates a variant member, and then we implicitly take the address of that member and pass it off as the this parameter to a method, which no longer knows that it’s dealing with a union member. The program needs to follow the normal active-union-member rules here because we can’t reasonably special-case it.

The standard assumes that there’s a primitive concept of reading an object of non-class type. This is only ever requested by applying lvalue-to-rvalue conversion (L2R) to a gl-value expression, so we usually talk about L2R for these objects.

The standard does not have a primitive concept of reading an object of class type. When you copy an object of class type, you’re always formally invoking a special member, and that generally means binding a reference parameter to the original operand. From the caller’s perspective, there’s no direct read of the class object as a whole. Defaulted implementations just recursively do memberwise operations, so there’s also no primitive read of the class object as a whole from the callee’s perspective. Trivially-defaulted implementations are special only in that they are known to be implementable with memcpy.

L2R conversion is a little weird for class types. Expressions of class type can formally undergo L2R conversion, but this is extremely rare — IIRC, it only happens when you pass them as variadic arguments. Every other place in the standard that triggers L2R conversion actually has special-case rules for class types, generally to instead trigger overload resolution to select a conversion operator or constructor. And even the L2R conversion performed by variadic arguments is ultimately just defined as copy-initializing a temporary, i.e. calling a conversion operator or constructor.

So we don’t talk about L2R conversion as a shorthand for a primitive read of a C++ class value, the way we do for other values, because:

  • most copies of class values do not actually involve L2R conversion,
  • the operation performed by L2R conversion on a class value can invoke user-defined code, and
  • C++ has no formal concept of primitively reading an entire class object anyway.

So that’s why my suggested wording talks about using it as the argument of a trivially-defaulted special member.

This does not have to be a deep research project to document but we can do better that the gcc docs.

Thanks for reporting this. I’ve sent a patch to improve on that.

I think it also does disservice to not say there are standards conforming alternatives even if this not elaborated on.

I’ll look at handling that too.

1 Like

Thank you, that is a great improvement and it will definitely help folks in the future to better understand the feature.

Apologies, on second reading, I realize my wording comes off a lot stronger than I intended.

1 Like

No worries at all, it’s easily done, and thanks for clarifying. I have some bigger changes planned to that section as I don’t think it really reflects modern advice we definitely give people. I’ll do all of that when adding std::bit_cast.

2 Likes

When documenting this, please don’t forget that we’re not supporting this in compile-time executed code.

1 Like

Strongly agreed

I’ve put together some initial documentation for what I have in mind, but before I post it as a PR, I’m curious if there’s general agreement with the direction I’m taking:

Type Punning Through Unions

A common idiom in C is to pun a type through use of a union, as in:

static_assert(sizeof(float) == sizeof(int));
static_assert(alignof(float) == alignof(int));
union U {
  float f;
  int i;
};

union U u = { 1.0f }; // Assigns into u.f
int i = u.i;          // Reads from u.i

This is valid to do in C (see DR 283, Defect report #283)
but is undefined behavior in standard C++ because the active member of the union
is U::f due to the initialization of u but U::i is being accessed in the
initialization of i.

Due to how common this practice is in C++, Clang defines the runtime behavior
of accessing the inactive member of a union to be the same in C and C++ if the
following conditions are true:

  • memory accesses happen through an object of the union type directly,
  • the members being accessed are accessed directly, and
  • all members of the union are standard layout ([class]).

This applies regardless of whether -fstrict-aliasing is enabled or not.
However, it does not apply within a constant evaluation context.

Note, the following are examples that remain undefined behavior:

int undefined1() {
  float f = 1.0f;
  return ((U *)&f)->i; // Access is not directly through an object of the union type.
}

int undefined2() {
  U u = { 1.0f };
  int *ptr = &u.i;
  return *ptr; // Access is not directly through an object of the union type.
}

int undefined3(int U::* ptm) {
  U u = { 1.0f };
  return u.*ptm; // Member is accessed through a pointer-to-member, not directly.
}

union Undefined {
  int i;
  std::string s; // Member is not standard layout
};
int undefined4() {
  Undefined u;
  u.s = "testing";
  return u.i;
}

constexpr int erroneous() {
  union U {
    float f;
    int i;
  } u = { 0.0f };
  return u.i; // Possibly ill-formed, not valid in a constant evaluation context
}
static_assert(erroneous() == 0); // Constant evaluation context, ill-formed
assert(erroneous() == 0); // Not a constant evaluation context, not ill-formed
1 Like

The phrasing “memory accesses happen through an object of the union type directly” seems unclear, given that the memory access doesn’t occur as part of accessing the member of the union type, but rather later, in an lvalue-to-rvalue conversion.

Maye instead we should instead specify something like: The glvalue arising from a union member access is specially marked. Objects of any dynamic type are type-accessible via such a glvalue. Any operation on such a glvalue drops this special marking except struct member access and (primitive) array element accesses [and are there any other exceptions?].

Also…I just went to re-read the C standard, and I’m a bit confused: it seems that the allowance to use a union to access the wrong type is just in a footnote – as if it’s a non-authoritative clarification of rules specified elsewhere – but isn’t (AFAICT) actually specified elsewhere?

Regarding the examples: both undefined1 and undefined2 would also be undefined under C’s rules, I think, not just the C++ extension? Though I’m not super-sure about undefined1. The access is through a pointer to a union type, so I guess the invalid part there is that you’re accessing a union U, when there’s no such union U object constructed at that address. (I think Clang would compile this code as-the-user-intended today in both C and C++, since it emits omnipotent char TBAA metadata for loads from a union.)

Finally, while we’re documenting things…we might want to also describe the ability to access members of __attribute__((packed)) structures which are misaligned for their underlying types in a similar way – the behaviors are very similar there.

I could go with something like that, though I’d prefer if we could avoid terms like glvalue because they’re not super user-friendly. But I don’t insist, either.

Yeah, unfortunately, the way to read the C standard involves reading the entire list of DRs which explain how to understand the intent of the standard. DRs would often clarify things without changing words in the standard to… you know… actually clarify anything. This happens to be DR 283: Defect report #283.

Yeah, those situations are UB already today. I was thinking we’d want them to remain UB to leave the door open for optimizations there because they’re less idiomatic forms of punning and are documented as being UB by GCC.

I’d be fine with that, but as a separate exercise because packed structures are already an extension (so we should be documenting those more generally).

1 Like

The comment should be that there is no object of the union type.

Is there a reason for this case to be UB? The type of the member is known (which is about all you get from naming the member if U is actually a union type). I suppose a union-like class example would be more motivating for making a claim of UB.

Aside from https://wg21.link/class.union.general#5, the list in https://wg21.link/class.temporary#6 may be useful for inspiration here.

I (also) think that the first “directly” can be misread here. Additionally, Clang has an extension that allows for nested anonymous unions; as a result, any uncertainty around the application of the above to anonymous unions is amplified (e.g., it is unclear which union is being referenced in “all members of the union”).

What I worry about is that once this guarantee is explicitly given, it would be very hard to take it away. If ever an optimization involving “correct” union assumptions is found, or some new language feature relies on the standard behavior, it would put us in a hard place. Its much harder to walk back an explicit guarantee than it would be to change behavior that was never 100% supported.

Our sanitizers do not diagnose the undefined behavior either

Type Sanitizer should, in general, catch these union type puns. There are some known bugs currently with unions and struct initializers with {} though so your milage may vary. This doesn’t mean we can’t make the guarantee and add a footnote about TySan following the stricter standard though.

In theory, sure. But in practice it doesn’t matter, people have assumed and will continue to assume it works, so you’re not going to stop people from doing it by not officially documenting it. Documenting it just acknowledges the reality and tries to formalise it so people have an idea where the limits of it are when it comes to interactions with C++-specific behaviour.