• Jose E. Marchesi's avatar
    libbpf: Avoid casts from pointers to enums in bpf_tracing.h · a9e7715c
    Jose E. Marchesi authored
    [Differences from V1:
      - Do not introduce a global typedef, as this is a public header.
      - Keep the void* casts in BPF_KPROBE_READ_RET_IP and
        BPF_KRETPROBE_READ_RET_IP, as these are necessary
        for converting to a const void* argument of
        bpf_probe_read_kernel.]
    
    The BPF_PROG, BPF_KPROBE and BPF_KSYSCALL macros defined in
    tools/lib/bpf/bpf_tracing.h use a clever hack in order to provide a
    convenient way to define entry points for BPF programs as if they were
    normal C functions that get typed actual arguments, instead of as
    elements in a single "context" array argument.
    
    For example, PPF_PROGS allows writing:
    
      SEC("struct_ops/cwnd_event")
      void BPF_PROG(cwnd_event, struct sock *sk, enum tcp_ca_event event)
      {
            bbr_cwnd_event(sk, event);
            dctcp_cwnd_event(sk, event);
            cubictcp_cwnd_event(sk, event);
      }
    
    That expands into a pair of functions:
    
      void ____cwnd_event (unsigned long long *ctx, struct sock *sk, enum tcp_ca_event event)
      {
            bbr_cwnd_event(sk, event);
            dctcp_cwnd_event(sk, event);
            cubictcp_cwnd_event(sk, event);
      }
    
      void cwnd_event (unsigned long long *ctx)
      {
            _Pragma("GCC diagnostic push")
            _Pragma("GCC diagnostic ignored \"-Wint-conversion\"")
            return ____cwnd_event(ctx, (void*)ctx[0], (void*)ctx[1]);
            _Pragma("GCC diagnostic pop")
      }
    
    Note how the 64-bit unsigned integers in the incoming CTX get casted
    to a void pointer, and then implicitly converted to whatever type of
    the actual argument in the wrapped function.  In this case:
    
      Arg1: unsigned long long -> void * -> struct sock *
      Arg2: unsigned long long -> void * -> enum tcp_ca_event
    
    The behavior of GCC and clang when facing such conversions differ:
    
      pointer -> pointer
    
        Allowed by the C standard.
        GCC: no warning nor error.
        clang: no warning nor error.
    
      pointer -> integer type
    
        [C standard says the result of this conversion is implementation
         defined, and it may lead to unaligned pointer etc.]
    
        GCC: error: integer from pointer without a cast [-Wint-conversion]
        clang: error: incompatible pointer to integer conversion [-Wint-conversion]
    
      pointer -> enumerated type
    
        GCC: error: incompatible types in assigment (*)
        clang: error: incompatible pointer to integer conversion [-Wint-conversion]
    
    These macros work because converting pointers to pointers is allowed,
    and converting pointers to integers also works provided a suitable
    integer type even if it is implementation defined, much like casting a
    pointer to uintptr_t is guaranteed to work by the C standard.  The
    conversion errors emitted by both compilers by default are silenced by
    the pragmas.
    
    However, the GCC error marked with (*) above when assigning a pointer
    to an enumerated value is not associated with the -Wint-conversion
    warning, and it is not possible to turn it off.
    
    This is preventing building the BPF kernel selftests with GCC.
    
    This patch fixes this by avoiding intermediate casts to void*,
    replaced with casts to `unsigned long long', which is an integer type
    capable of safely store a BPF pointer, much like the standard
    uintptr_t.
    
    Testing performed in bpf-next master:
      - vmtest.sh -- ./test_verifier
      - vmtest.sh -- ./test_progs
      - make M=samples/bpf
    No regressions.
    Signed-off-by: default avatarJose E. Marchesi <jose.marchesi@oracle.com>
    Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
    Link: https://lore.kernel.org/bpf/20240502170925.3194-1-jose.marchesi@oracle.com
    a9e7715c
usdt.bpf.h 8.31 KB