Aliasing array of unions

Question

This is a follow-up question to this question.

The following code is on Compiler Explorer.

#include <stddef.h>
#include <stdint.h>

union my_type
{
    uint8_t m8[8];
    uint16_t m16[4];
    uint32_t m32[2];
    uint64_t m64;
};

void my_copy(uint32_t volatile *restrict dst, uint32_t const *restrict src, size_t const count)
{
    for (size_t i = 0; i < count / 8; ++i)
    {
        *dst++ = *src++;
        *dst++ = *src++;
    }
}

int main(int argc, char *argv[])
{
    union my_type src[100];
    union my_type dst[200];

    my_copy(dst[42].m32, src[10].m32, sizeof(union my_type) * 60);
    
    return 0;
}

While my_copy looks contrived, the access pattern is forced by hardware (must have 2x 32-bit writes to consecutive aligned locations). The rest of the ugliness is due to the intersection of a couple of sections of code that were written by different developers a couple of years apart.

The question is, are the arguments passed to my_copy an issue? If this was just to copy a single union my_type, I believe that it would be ok. I don't know if that is valid to copy an array, though.

I don't see how how manually unrolling the loop is any better of having only one `*dst++ = *src++;` in it for the *"2x 32-bit writes to consecutive aligned locations"*. These postincrements are likely to add some other stuff between the actual memory accesses. — Eugene Sh., Mar 15 '22 at 21:04
@EugeneSh. There is additional code that has to be executed every second write. — Graznarak, Mar 15 '22 at 21:22
@user17732522 I added `volatile`, but it isn't particularly relevant to the question about using `m32` to alias the array of unions. — Graznarak, Mar 15 '22 at 21:30
Pretty sure it's technically not legal, as you access the array `src[10].m32` beyond its size which is 2, and likewise `dst[42].m32`. I haven't yet managed to make it fail on a real compiler, though. — Nate Eldredge, Mar 15 '22 at 21:38
What are you doing with `sizeof(union my_type) * 60`? `sizeof (my_type)` isn't guaranteed (though presumably it should be 64-bytes). The `count / 8`, presuming a 64-byte size, would copy 480 bytes. I may just be misunderstanding what you are attempting, but that looks odd. — David C. Rankin, Mar 15 '22 at 21:42
@NateEldredge That is my suspicion as well, but I would like confirmation since UB will bite me in the butt at some point in the future. — Graznarak, Mar 15 '22 at 21:42
@DavidC.Rankin `count` is in bytes. To copy 60x `union my_type` I have to multiply by the `sizeof` in order to get to bytes. — Graznarak, Mar 15 '22 at 21:44
That then does make sense. I'm with @NateEldredge on the array-bound issue. — David C. Rankin, Mar 15 '22 at 21:44
Nitpick: it seems unnecessary to pass `sizeof(union my_type) * 60` to the function and then divide by the magic number `8` in the function. The code can only work if `sizeof(union my_type)` is 8. So I'd put a `static_assert(sizeof(union my_type) == 8)` somewhere in the code, and then just pass the actual `count` of `60` to the function. Or at least do the division in the calling code, where the assumption is being made. — user3386109, Mar 15 '22 at 21:45
@user3386109 There are legacy-code related reasons that I can't do that. — Graznarak, Mar 15 '22 at 21:46
Yep, it's pretty clear that there's a bigger picture that you haven't shown us. You seem to be proposing to put some duct tape on the Titanic. — user3386109, Mar 15 '22 at 21:49
@user3386109 I work on embedded systems. The hardware puts some really odd constraints on the code. Some pieces are legacy (the function signature). Other pieces are hardware mandated (two 32-bit writes at a time). — Graznarak, Mar 15 '22 at 21:52
You have my sympathies, but your proposed solution is not the correct solution to [the problem that you're actually trying to solve](https://meta.stackexchange.com/questions/66377/what-is-the-xy-problem). As you've already been told, the function is given the address of an array of size 2, and it writes past the end of that array. That's technically undefined behavior. — user3386109, Mar 15 '22 at 22:20

score 2 · Accepted Answer · answered Mar 15 '22 at 22:13

The question is, are the arguments passed to my_copy an issue? If this was just to copy a single union my_type, I believe that it would be ok. I don't know if that is valid to copy an array, though.

I'm not sure whether to blame it on the arguments, exactly, but you pass pointers derived from two arrays, and then use them to overrun the bounds of those arrays. Undefined behavior results. That the storage thereby accessed can be considered to stay within the bounds main()'s src and dst arrays does not formally matter.

In practice, the program probably behaves as you expect, though I find the particular calling idiom extremely obscure. Why is sizeof(union mytype) involved? Why is the corresponding parameter named count, when it is actually eight times the number of loop iterations? I think I have figured it out, but I shouldn't have to be uncertain about that.

If you want to copy between two disjoint arrays of union my_type, using the 2x32-bit access pattern of the example, then I would write it like this:

void my_copy(volatile union mytype * restrict dst,
        volatile const union my_type * restrict src, size_t byte_count) {
    // TODO: validate (or assert) that byte_count is a multiple of 8
    for (size_t i = 0; i < byte_count / 8; ++i) {
        dst->m32[0] = src->m32[0];
        dst->m32[1] = src->m32[1];
        ++dst;
        ++src;
    }
}

int main(int argc, char *argv[]) {
    union my_type src[100];
    union my_type dst[200];

    my_copy(dst + 42, src + 10, 8 * 60);
    
    return 0;
}

That's not too different from what you present, and it avoids the array-overrun issue. And if you want to keep sizeof(union my_type) then even that would make a bit more sense in conjunction with the revised types of the first two parameters to my_copy().

Aliasing array of unions

1 Answers1