While looking at the code for Dear Imgui, I found the following code (edited for relevance):
struct ImVec2
{
float x, y;
float& operator[] (size_t idx) { return (&x)[idx]; }
};
It's pretty clear that this works in practice, but from the perspective of the C++ standard, is this code legal? And if not, do any of the major compilers (G++, MSVC, Clang) offer any explicit or implicit guarantees that this code will work as intended?
This is almost safe in ISO C++, and also ISO C, and compilers appear to define the behaviour even with float* To be fully safe, you should cast to char* for the pointer math before casting to float*; the ISO standards only allow pointer math on pointers to arrays, but you're supposed to be able to treat any object as an array of char or std::byte, which is what makes offsetof usable to make a pointer you can deref. But in practice on real implementations like GCC, it seems well-defined even with just float*.
Assuming no padding: you can static_assert that offsetof(ImVec2, y) == sizeof(float).
For a standard-layout type, a pointer to the first member is convertible to/from a pointer to the whole struct/class object.
Given a standard-layout type, it's well-defined to index into it using offsetof(T,y) as an offset. See Using offsetof to access struct member (C, but I assume the intent in C++ is for offsetof to be usable the same way.) There is some debate over whether the wording of the ISO standards truly supports this, but that's the intent, and compiler devs agree it should be well-defined.
Unlike pointers where it matters how you obtained it (not just its numeric value because pointers don't have to be flat integers), a size_t is fungible. Since a value of 4 from offsetof works, a run-time variable 4 from idx*sizeof(float) also works.
The char* math using offsetof might be safely done using float* instead of actually casting to char* and back. But this isn't well supported by the wording of the standard and relies on some assumptions about things being equivalent. For maximum safety, use char* so you're only relying on the same behaviour that using offsetof to access a member would, which the standard I think intends to be well-defined.
See Is adding to a "char *" pointer UB, when it doesn't actually point to a char array? where zwol's answer points out the conflicting goals of making it UB to access outside the bounds of a member array of a struct, but also to allow member access via an offset from offsetof.
The behaviour is undefined if idx isn't 0 or 1, of course, since you don't do bounds-checking. (Using idx & 1 would cost an AND instruction, but fairly cheaply give you unsigned mod 2. But an out-of-bounds index is very likely to be a bug, so silently working in that case is not great. If you want anything for bounds checking, probably a branch that's never taken in the non-buggy case, like an assert, or throwing an exception, or returning NaN.)
It might even be legal to access past the end of the struct starting with this pointer, if it was part of an array of such structs. We'd have to justify it as converting to an array member, and then accessing into another array member similar to offsetof. (Accessing one array member relative to another is guaranteed).
In C, Is pointer to struct a pointer to its first member? - yes, and vice versa, citing N1570 6.7.2.1p15.
In C++, the same guarantee is restricted to "standard layout" types, which rules out there being a vtable. Padding before the first member is disallowed, and pointer conversion between the first member and whole struct is allowed. See 11.4.1 Class members - General in the current draft:
- If a standard-layout class object has any non-static data members, its address is the same as the address of its first non-static data member if that member is not a bit-field. Its address is also the same as the address of each of its base class subobjects.
[Note 11: There can therefore be unnamed padding within a standard-layout struct object inserted by an implementation, but not at its beginning, as necessary to achieve appropriate alignment. — end note]
[Note 12: The object and its first subobject are pointer-interconvertible ([basic.compound], [expr.static.cast]). — end note]
Another way to write this is to start with the struct object yourself, without relying on &x implicitly working as this. And do the math using char*.
You could reinterpret_cast<const char*>(this) + 4*idx to get a pointer to the member, then cast that to float* and deref. (Or actually sizeof(float), and assuming offsetof(ImVec2, y) == sizeof(float).) Since you have a 2-member struct, idx * offsetof(ImVec2,y) using char* math would also work, and hopefully let the compiler still make x86 asm like lea rax, [rdi + rsi*4] to return a pointer aka C++ reference.
This is equivalent to casting this to float*, except the actual pointer math happens on a char*, which is intended to be allowed within any object.
#include <cstdlib>
#include <cstddef>
#include <type_traits>
struct ImVec2
{
float x, y;
float& operator[] (size_t idx) {
static_assert(std::is_standard_layout<ImVec2>::value, "can't index in a struct that isn't standard layout");
// offset(x) == 0 is guaranteed by ISO C++ for standard-layout types
static_assert(offsetof(ImVec2, x) == 0, "struct of float x,y isn't 2 contiguous members");
// A hypothetical compiler could put padding before y
static_assert(offsetof(ImVec2, y) == sizeof(float), "struct of float x,y isn't 2 contiguous members");
// assert(idx <= sizeof(*this) / sizeof(x) && "out of bounds access to xy vector");
char *obj = reinterpret_cast<char*>(this);
obj += sizeof(float) * idx; // or idx * offsetof(T,y) for a 2-member struct
return *reinterpret_cast<float*>(obj);
// memcpy into float tmp could avoid ever dereferencing a float* if you only want to return by value
// It's safe to derive a pointer to a member from a pointer to the whole object
}
float & index_from_member (size_t idx){
return (&x)[idx]; // Less safe; (ImVec2*)(&x) is allowed, but the pointer math is on float* not char*
}
};
This will of course compile to the same asm for mainstream CPUs where struct layout is normal and the simple version in the question works.
For the version in the question, or the one starting with this, GCC only warns for a compile-time-constant index of 2 or greater. That's a good sign that it knows there might be a problem, but doesn't think there is when the access is still within the whole struct that the member was part of.
Lack of a compiler warning or runtime detection by UBSAN does not prove it's safe in ISO C++ or C in general, or even that it's fully safe with that compiler.
But presence of a warning in one case and lack of it in another does confirm that the compiler cares about a difference, and that's where the threshold is. Unless there's some other UB it's not warning about. Or it's always possible that the warning and some other part of GCC's internals are out of sync, and some value-range proving part of GCC might infer __builtin_assume(idx==0) despite not not warning. That's probably not what GCC does, but the lack of warning at idx=1 doesn't definitively prove it's safe even though it warns at idx=2. We have other supporting evidence, though, such as code like this existing in real-world source code and apparently working.
So it appears that GCC does define the behaviour. With return iv.index_from_member(1), there's no warning even though we're accessing outside the bounds of x.
Godbolt - GCC and clang with -O3 -Wall -fsanitize=undefined - with a constant 1 arg, both versions just compile to a load, vs. with 2 they also make code that will print an error if executed.
In that link, I showed one of each: the new version with iv[1], the old version with iv.index_from_member(2);. Reversing those, the warning comes only from the new version.
## GCC12.2 -O3 -Wall
<source>: In function 'float test_orig()':
<source>:38:32: warning: array subscript 2 is outside array bounds of 'ImVec2 [1]' [-Warray-bounds]
38 | return iv.index_from_member(2);
| ~~~~~~~~~~~~~~~~~~~~^~~
<source>:37:12: note: at offset 8 into object 'iv' of size 8
37 | ImVec2 iv = {2.0, 2.0};
| ^~
ASM generation compiler returned: 0
Note that GCC's warning describes it as an object of size 8.
Clang doesn't warn even with -O3 -Wall -Wextra, but with -fsanitize=undefined it makes asm that will unconditionally call __ubsan_handle_type_mismatch_v1 with a compile-time constant idx of 2 (after inlining). (After checking for pointer overflow of the stack pointer first, i.e. that RSP on function entry wasn't 0.)
/app/example.cpp:22:16: runtime error: reference binding to address 0x7ffce4f8eba0 with insufficient space for an object of type 'float'
0x7ffce4f8eba0: note: pointer points here
00 00 00 40 b0 b5 34 d6 08 56 00 00 83 e0 48 0e 86 7f 00 00 00 00 00 00 00 00 00 00 98 ec f8 e4
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /app/example.cpp:22:16 in
/app/example.cpp:38:15: runtime error: load of address 0x7ffce4f8eba0 with insufficient space for an object of type 'float'
0x7ffce4f8eba0: note: pointer points here
00 00 00 40 b0 b5 34 d6 08 56 00 00 83 e0 48 0e 86 7f 00 00 00 00 00 00 00 00 00 00 98 ec f8 e4
In a different case where the standard didn't define the behaviour (e.g. in a non standard-layout class, or maybe if there was some other member before x), the most likely way for it to break would be that after inlining, the compiler would conclude that the only possible idx value is 0, and not actually do runtime-variable indexing at all. And optimize away earlier and later computations that led to or use idx. (But UB can cause arbitrary breakage if a compiler doesn't define the behaviour, at least de-facto for the current compiler version.)
It's not strict-aliasing UB. You're not accessing an int object through a float* or something like that. Both objects are float, the only potential problem is deriving a float* to y from a separate object x which merely happens to be located next to it. It's legal to create a one-past-the-end pointer to any object, including a scalar float, but it's not in general legal to deref it. We have to look to other rules to justify this. gcc -fno-strict-aliasing wouldn't make things legal if there had been a problem.