• David Faust's avatar
    bpf: avoid gcc overflow warning in test_xdp_vlan.c · 792a04be
    David Faust authored
    This patch fixes an integer overflow warning raised by GCC in
    xdp_prognum1 of progs/test_xdp_vlan.c:
    
      GCC-BPF  [test_maps] test_xdp_vlan.bpf.o
    progs/test_xdp_vlan.c: In function 'xdp_prognum1':
    progs/test_xdp_vlan.c:163:25: error: integer overflow in expression
     '(short int)(((__builtin_constant_p((int)vlan_hdr->h_vlan_TCI)) != 0
       ? (int)(short unsigned int)((short int)((int)vlan_hdr->h_vlan_TCI
       << 8 >> 8) << 8 | (short int)((int)vlan_hdr->h_vlan_TCI << 0 >> 8
       << 0)) & 61440 : (int)__builtin_bswap16(vlan_hdr->h_vlan_TCI)
       & 61440) << 8 >> 8) << 8' of type 'short int' results in '0' [-Werror=overflow]
      163 |                         bpf_htons((bpf_ntohs(vlan_hdr->h_vlan_TCI) & 0xf000)
          |                         ^~~~~~~~~
    
    The problem lies with the expansion of the bpf_htons macro and the
    expression passed into it.  The bpf_htons macro (and similarly the
    bpf_ntohs macro) expand to a ternary operation using either
    __builtin_bswap16 or ___bpf_swab16 to swap the bytes, depending on
    whether the expression is constant.
    
    For an expression, with 'value' as a u16, like:
    
      bpf_htons (value & 0xf000)
    
    The entire (value & 0xf000) is 'x' in the expansion of ___bpf_swab16
    and we get as one part of the expanded swab16:
    
      ((__u16)(value & 0xf000) << 8 >> 8 << 8
    
    This will always evaluate to 0, which is intentional since this
    subexpression deals with the byte guaranteed to be 0 by the mask.
    
    However, GCC warns because the precise reason this always evaluates to 0
    is an overflow.  Specifically, the plain 0xf000 in the expression is a
    signed 32-bit integer, which causes 'value' to also be promoted to a
    signed 32-bit integer, and the combination of the 8-bit left shift and
    down-cast back to __u16 results in a signed overflow (really a 'warning:
    overflow in conversion from int to __u16' which is propegated up through
    the rest of the expression leading to the ultimate overflow warning
    above), which is a valid warning despite being the intended result of
    this code.
    
    Clang does not warn on this case, likely because it performs constant
    folding later in the compilation process relative to GCC.  It seems that
    by the time clang does constant folding for this expression, the side of
    the ternary with this overflow has already been discarded.
    
    Fortunately, this warning is easily silenced by simply making the 0xf000
    mask explicitly unsigned.  This has no impact on the result.
    Signed-off-by: default avatarDavid Faust <david.faust@oracle.com>
    Cc: jose.marchesi@oracle.com
    Cc: cupertino.miranda@oracle.com
    Cc: Eduard Zingerman <eddyz87@gmail.com>
    Cc: Yonghong Song <yonghong.song@linux.dev>
    Link: https://lore.kernel.org/r/20240508193512.152759-1-david.faust@oracle.comSigned-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
    792a04be
test_xdp_vlan.c 7.09 KB