Why this union has char array at the end?

Question

While I was reading a Linux kernel implementation for RCU lock primitives, I ended up reaching the below code.

#define WRITE_ONCE(x, val) \
({                                                      \
        union { typeof(x) __val; char __c[1]; } __u =   \
                { .__val = (__force typeof(x)) (val) }; \
        __write_once_size(&(x), __u.__c, sizeof(x));    \
        __u.__val;                                      \
})


static __always_inline void __write_once_size(volatile void *p, void *res, int size)
{
        switch (size) {
        case 1: *(volatile __u8 *)p = *(__u8 *)res; break;
        case 2: *(volatile __u16 *)p = *(__u16 *)res; break;
        case 4: *(volatile __u32 *)p = *(__u32 *)res; break;
        case 8: *(volatile __u64 *)p = *(__u64 *)res; break;
        default:
                barrier();
                __builtin_memcpy((void *)p, (const void *)res, size);
                barrier();
        }
}

As far as I can tell, union ending with the char array having the size of the previous part of the union allows you to access memory assigned to the union in byte granularity.

For example,

#define UNION_PARTIAL \
    int a;            \
    int b;            \
    char *c;          \

union union_partial {
    UNION_PARTIAL
};

union union_full {
    UNION_PARTIAL
    char bytes[sizeof(union union_partial)];
};

However, it seems that the union used in the WRITE_ONCE macro is not declared to provide fine granularity accesses to the union memory. The below code may be used instead, but I don't know why we need char __c[1].

#define WRITE_ONCE(x, val) \
({                                                      \
        union { typeof(x) __val; char __c[1]; } __u =   \
                { .__val = (__force typeof(x)) (val) }; \
        __write_once_size(&(x), &(__u.val), sizeof(x)); \
        __u.__val;                                      \
})

is it only because to reduce burden for a programmer to type & in front of the __u.val ?

Your assumption is correct. The only purpose of `__c` here is to hold the address of the new `union` object so you could use `__u.__c` instead of `&(__u.__val)`. — Alex Lop., Jan 14 '19 at 08:31
@AlexLop. I appreciate the confirmation, but it seems that __c is not a good naming convention for indicating that __c is the pointer to the union memory itself. What does it mean? is it just one of the bad naming? or is it custom of the Linux programmer that I don't know? — ruach, Jan 14 '19 at 08:44
This code is just... eww. The non-standard goo aside, casting from a `char*` to `(__u16 *)` etc appears to be a strict aliasing violation. gcc would probably love to go bananas here. The effective type of the original object is probably something like "qualified __u16". Seems like UB to me. — Lundin, Jan 14 '19 at 10:26
@Lundin Sorry for bothering, because I am not familiar with the standard type-casting; I think I cannot fully understand your points. Is it mean that a type-casting from the char * to (__u16 *) is not allowed in the standard? — ruach, Jan 14 '19 at 14:08
@JaehyukLee The cast and pointer conversion in itself is allowed. The problem arises when we access the data stored. If it is stored as one type but you access it through a pointer to another type, you invoke undefined behavior bugs. I don't see the whole picture here, so I can't say for sure if that's the case. (And the Linux kernel depends heavily on non-standard extensions anyhow.) But there is no doubt that this is some very ugly, hackish code. — Lundin, Jan 14 '19 at 14:22
@Lundin Thanks for the explanation. Then, if the p and res have the same type, is it okay to access them using a different pointer type that has the same size as the initial type? Or accessing the p and res with other types of a pointer itself could be problematic? — ruach, Jan 14 '19 at 15:35
@JaehyukLee It depends. For example a `float*` might have the same size as a `uint32_t*`, and the data pointed at could still be of the same size. But you can't convert between them because they are simply not compatible. — Lundin, Jan 14 '19 at 15:37
@Lundin it is still confusing little bit. Even though two types are different, if they have the same size, then accessing the memory will return the 32 bit value stored in the memory. Therefore even though two types have different interpretation for the content stored in the memory, it seems that it does not generate any problem when the different type of pointers are used for just reading and writing the value from one memory location to the other with any pointer type. Is it still wrong ? Sorry for bothering — ruach, Jan 14 '19 at 16:02
I am curious because in the above code example write_once_size will be invoked with two arguments that have been declared with the same type but pointed to by different pointer type. Therefore, even though different pointer type could result in wrong interpretation or operation but if it is just used for copying only specific amount of data from one memory address to the other, is there any case that generate wrong copy? — ruach, Jan 14 '19 at 16:12
Yes it's wrong. There are trap representation. And accessing a trap representation causes an exception or other implementation defined behavior. On a particular representation there may exist values that are in the uint32_t value space, but are not represented by the float type. So doing `*(float*)(uint32_t[]){ THAT_VALUE }` might be ub, if `THAT_VALUE` is a trap representation when using float. Or you might get what standard calls, unspecified value. — KamilCuk, Jan 14 '19 at 19:54
The current implementation of `__write_once_size` will result in undefined behavior on machines with alignment requirements and if those alignment requirements are not fit for the pointers passed into `__write_once_size`. This is kernel code, standard is standard, kernel works with the compiler, with which it should generate code that works. — KamilCuk, Jan 14 '19 at 20:00
@JaehyukLee https://stackoverflow.com/questions/98650/what-is-the-strict-aliasing-rule. — Lundin, Jan 14 '19 at 22:22
for the readers who will visit this question later, it was also useful to read [this](https://lkml.org/lkml/2003/2/26/158). — ruach, Jan 15 '19 at 05:18

score 1 · Accepted Answer · answered Jan 14 '19 at 08:44

Let's dwell into READ_ONCE(). The C compiler will complain if passing a const pointer into a function that takes a void * pointer. The __read_once_size is declared as static __always_inline void __read_once_size(volatile void *p, void *res, int size). The macro was declared as:

#define READ_ONCE(x) \
    ({ typeof(x) __val; __read_once_size(&x, &__val, sizeof(__val)); __val; })

will cause warnings when the typedef has const in it, I think like:

typedef const int type_t;
struct a_s {
   type_t m;
};
struct a_s a = { .m = 1; };
type_t b = READ_ONCE(a.m);

In such usage typeof(x) is const int, so &__val is const int*, which will cause warnings/errors when casting into void*. In such usage the const is not cast away on typeof, so we pass a const * pointer to the __write_once_size function. So the author decided to use a "union trick" to cast the constness away by passing a pointer to the array starting at the same place as the value. (One could also do some strange casts (const void*)(uintptr_t)(void*), but it wouldn't be so portable).

The author in this commit explained this and changed the READ_ONCE macro:

- ({ typeof(x) __val; __read_once_size(&x, &__val, sizeof(__val)); __val; })
+ ({ union { typeof(x) __val; char __c[1]; } __u; __read_once_size(&(x), __u.__c, sizeof(x)); __u.__val; })

Probably to be consistent and accommodate with READ_ONCE() changes, the WRITE_ONCE() macro was changed accordingly in this commit.

If my understanding is correct, because the typeof returns not only the type of the variable but also the const keyword, so the second parameter which is a void * doesn't match with the const XX * and generates the error. Therefore, to prevent the warning and errors, instead of the val that has const keyword in its type, union is used to reference the same memory but with the char * which can be cast to void * without any error. Is it correct? — ruach, Jan 14 '19 at 14:04
Yes. And I think the commit message explained it better then I did. It's basically doing like `__UNCONST()`. — KamilCuk, Jan 14 '19 at 19:51
That last [commit](https://github.com/torvalds/linux/commit/ab3f02fc237211f0583c1e7ba3bf504747be9b8d) should probably have been split, but I guess it's far too late to do so now! — Ian Abbott, Jan 18 '19 at 15:50

Why this union has char array at the end?

1 Answers1