llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.24k stars 11.66k forks source link

clang-tidy UndefinedBinaryOperatorResult possible false positive for shift of 0 #56253

Open batneil opened 2 years ago

batneil commented 2 years ago

With clang-tidy 14.0.5, compiling this test program produces a warning for shift of 0:

#include <cstdint>

static constexpr uint32_t get_shifted(uint32_t v)
{
    return v << 6;
}

extern struct _bitf
{
    uint64_t mode : 1;
} bitf;

static void func()
{
    const bool bitf_mode = bitf.mode;

    bool other_mode = true;

    const bool not_used = bitf_mode && !other_mode;

    uint32_t val = get_shifted((uint32_t)bitf_mode);
}

Compiling with clang-tidy --checks=-*,clang-analyzer-core.UndefinedBinaryOperatorResult bug.cpp -- -std=c++17 results in:

/tmp/bug.cpp:5:14: warning: The result of the left shift is undefined due to shifting '0' by '6', which is unrepresentable in the unsigned version of the return type 'uint32_t' [clang-analyzer-core.UndefinedBinaryOperatorResult]
    return v << 6;
             ^
/tmp/bug.cpp:15:5: note: 'bitf_mode' initialized here
    const bool bitf_mode = bitf.mode;
    ^~~~~~~~~~~~~~~~~~~~
/tmp/bug.cpp:19:27: note: Assuming 'bitf_mode' is false
    const bool not_used = bitf_mode && !other_mode;
                          ^~~~~~~~~
/tmp/bug.cpp:19:37: note: Left side of '&&' is false
    const bool not_used = bitf_mode && !other_mode;
                                    ^
/tmp/bug.cpp:21:32: note: Passing value via 1st parameter 'v'
    uint32_t val = get_shifted((uint32_t)bitf_mode);
                               ^~~~~~~~~~~~~~~~~~~
/tmp/bug.cpp:21:20: note: Calling 'get_shifted'
    uint32_t val = get_shifted((uint32_t)bitf_mode);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/bug.cpp:5:14: note: The result of the left shift is undefined due to shifting '0' by '6', which is unrepresentable in the unsigned version of the return type 'uint32_t'
    return v << 6;
           ~ ^

Perhaps I'm missing something, but the shift seems valid. The same code doesn't produce any warnings in version 13.0. Also, for what it's worth if I change the code so that bitf_mode comes from an extern bool rather than the struct member, I don't see the warning anymore.

llvmbot commented 2 years ago

@llvm/issue-subscribers-clang-static-analyzer

martong commented 2 years ago

I believe, this false report is because we do not handle integer casts properly. We are in the middle of implementing them. However, that seems to be a nontrivial problem that will take some time to tackle. We have high hopes for https://reviews.llvm.org/D103096 (@ASDenysPetrov).

This is the same issue that we have with (@vabridgers):

  1
  2 #include <assert.h>
  3 void bar(short k) {
  4   ++k; // k1 = k0 + 1
  5   assert(k == 1); // k1 == 1 --> k0 == 0
  6   (long)k << 16; // k0 + 1 << 16
  7 }

$ clang --analyze -Xclang -analyzer-checker=debug.ExprInspection   go.c

go.c:6:11: warning: The result of the left shift is undefined due to shifting '1' by '16', which is unrepresentable in the unsigned version of the return type 'long' [core.UndefinedBinaryOperatorResult]

  (long)k << 16; // k0 + 1 << 16

  ~~~~~~~ ^

1 warning generated.
blitz commented 1 year ago

We are also seeing this:

 error: The result of the left shift is undefined due to shifting '0' by '4', which is unrepresentable in the unsigned version of the return type 'uint64' [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors]