Aliasing array of unions-CodePudding

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.

CodePudding user response：

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().