Which architecture causes this problem?

The build fails with GCC on all architectures.

If char is really 32-bits long, then the uint8_t type must necessarily also be at least 32-bits long. char is the smallest unit you can have in C, absent non-standard extensions (and ignoring the special case of bit fields which aren't a first-class type).

*sigh* Now this causes a -Wcast-align error with clang even though it's the same exact code as used in udp_input() in the kernel. I guess the kernel doesn't build with -Wcast-align perhaps?

All the solutions do not address the underlying problem: Do we know, that the payload is correctly assigned or not?
If we know, that we should use

alignas(int32_t) void * pstart = &up[p->ip_hl];
struct udphdr *u = pstart;

If not: every attempt to trick the compiler will cause a bus error on the problematic architecture anyway.

In D31941#771955, @donner wrote:

All the solutions do not address the underlying problem: Do we know, that the payload is correctly assigned or not?
If we know, that we should use

alignas(int32_t) void * pstart = &up[p->ip_hl];
struct udphdr *u = pstart;

If not: every attempt to trick the compiler will cause a bus error on the problematic architecture anyway.

udphdr doesn't need 4 byte alignment, just 2 byte. struct ip has an alignment of 2. What makes the compiler unhappy is the abuse of using a uint32_t pointer as a lazy way to multiply hlen by 4. The compiler (correctly) thinks this means that the intermediate up pointer requires 4 byte alignment. One could instead do something rather gross like:

uint16_t *us = (void *)p;
struct udphdr *u = (void *)&(us[p->ip_hl << 1]);

However, this seems quite obscure (using uint16_t to multiply by 2, then << 1 to multiply by 2 again). Rather than abusing pointer math in this way, the kernel's approach of 'header + len' seems more straightforward.

In D31941#772286, @jhb wrote:

udphdr doesn't need 4 byte alignment, just 2 byte. struct ip has an alignment of 2. What makes the compiler unhappy is the abuse of using a uint32_t pointer as a lazy way to multiply hlen by 4. The compiler (correctly) thinks this means that the intermediate up pointer requires 4 byte alignment. One could instead do something rather gross like:

How about

typedef header_word_t uint8_t[4];
header_word_t *us = (void *)p;
struct udphdr *u = (void *)&(us[p->ip_hl]);

Or plain and simple:

struct udphdr *u = (void *)((char *)p + 4*(p->ip_hl));

In D31941#772518, @donner wrote:

In D31941#772286, @jhb wrote:

udphdr doesn't need 4 byte alignment, just 2 byte. struct ip has an alignment of 2. What makes the compiler unhappy is the abuse of using a uint32_t pointer as a lazy way to multiply hlen by 4. The compiler (correctly) thinks this means that the intermediate up pointer requires 4 byte alignment. One could instead do something rather gross like:

How about

typedef header_word_t uint8_t[4];
header_word_t *us = (void *)p;
struct udphdr *u = (void *)&(us[p->ip_hl]);

Or plain and simple:

struct udphdr *u = (void *)((char *)p + 4*(p->ip_hl));

I tried this second one earlier (it was the previous version of this review) but it fails because -Wcast-align on clang doesn't like demoting the the alignment of p (2) to a char * (1) hence why the current patch uses uintptr_t.