Re: [PATCH v3 bpf-next 1/3] bpf: switch BPF UAPI #define constants used from BPF program side to enums

[Michael Forney <mforney@mforney.org>]

Hi,

On 2020-03-04, Daniel Borkmann <daniel@iogearbox.net> wrote:
> I was about to push the series out, but agree that there may be a risk for
> #ifndefs
> in the BPF C code. If we want to be on safe side, #define FOO FOO would be
> needed.

I did indeed hit some breakage due to this change, but not for the
anticipated reason.

The C standard requires that enumeration constants be representable as
an int, and have type int. While it is a common extension to allow
constants that exceed the limits of int, and this is required
elsewhere in Linux UAPI headers, this is the first case I've
encountered where the constant is not representable as unsigned int
either:

	enum {
		BPF_F_CTXLEN_MASK		= (0xfffffULL << 32),
	};

To see why this can be problematic, consider the following program:

	#include <stdio.h>
	
	enum {
		A = 1,
		B = 0x80000000,
		C = 1ULL << 32,
	
		A1 = sizeof(A),
		B1 = sizeof(B),
	};
	
	enum {
		A2 = sizeof(A),
		B2 = sizeof(B),
	};
	
	int main(void) {
		printf("sizeof(A) = %d, %d\n", (int)A1, (int)A2);
		printf("sizeof(B) = %d, %d\n", (int)B1, (int)B2);
	}

You might be surprised by the output:

	sizeof(A) = 4, 4
	sizeof(B) = 4, 8

This is because the type of B is different inside and outside the
enum. In my C compiler, I have implemented the extension only for
constants that fit in unsigned int to avoid these confusing semantics.

Since BPF_F_CTXLEN_MASK is the only offending constant, is it possible
to restore its definition as a macro?

Also, I'm not sure if it was considered, but using enums also changes
the signedness of these constants. Many of the previous macro
expressions had type unsigned long long, and now they have type int
(the type of the expression specifying the constant value does not
matter). I could see this causing problems if these constants are used
in expressions involving shifts or implicit conversions.

Thanks for your time,

-Michael