llvm / llvm-project

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

[libc] -Winteger-overflow in libc/test/src/math/smoke/LdExpTest.h #115205

Open nickdesaulniers opened 1 week ago

nickdesaulniers commented 1 week ago

building for i386:

[285/319] Building CXX object libc/test/src/math/smoke/CMa...smoke.ldexpl_test.__unit__.__build__.dir/ldexpl_test.cpp.o
In file included from /android0/llvm-project/libc/test/src/math/smoke/ldexpl_test.cpp:9:
/android0/llvm-project/libc/test/src/math/smoke/LdExpTest.h:53:63: warning: overflow in expression; result is -2'147'483'648 with type 'long' [-Winteger-overflow]
   53 |     long long_exp_array[4] = {LONG_MIN, INT_MIN - 1L, INT_MAX + 1L, LONG_MAX};
      |                                                       ~~~~~~~~^~~~
/android0/llvm-project/libc/test/src/math/smoke/LdExpTest.h:53:49: warning: overflow in expression; result is 2'147'483'647 with type 'long' [-Winteger-overflow]
   53 |     long long_exp_array[4] = {LONG_MIN, INT_MIN - 1L, INT_MAX + 1L, LONG_MAX};
      |                                         ~~~~~~~~^~~~
/android0/llvm-project/libc/test/src/math/smoke/LdExpTest.h:53:63: warning: overflow in expression; result is -2'147'483'648 with type 'long' [-Winteger-overflow]
   53 |     long long_exp_array[4] = {LONG_MIN, INT_MIN - 1L, INT_MAX + 1L, LONG_MAX};
      |                                                       ~~~~~~~~^~~~
/android0/llvm-project/libc/test/src/math/smoke/LdExpTest.h:53:49: warning: overflow in expression; result is 2'147'483'647 with type 'long' [-Winteger-overflow]
   53 |     long long_exp_array[4] = {LONG_MIN, INT_MIN - 1L, INT_MAX + 1L, LONG_MAX};
      |                                         ~~~~~~~~^~~~
$ cmake ../runtimes -G Ninja -DLLVM_ENABLE_LLD=ON -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_RUNTIMES="libc" -DLIBC_TARGET_TRIPLE=i386-linux-gnu -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
$ libc/test/src/math/smoke/libc.test.src.math.smoke.ldexpf_test.__unit__.__build__
llvmbot commented 1 week ago

@llvm/issue-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

building for i386: ``` [285/319] Building CXX object libc/test/src/math/smoke/CMa...smoke.ldexpl_test.__unit__.__build__.dir/ldexpl_test.cpp.o In file included from /android0/llvm-project/libc/test/src/math/smoke/ldexpl_test.cpp:9: /android0/llvm-project/libc/test/src/math/smoke/LdExpTest.h:53:63: warning: overflow in expression; result is -2'147'483'648 with type 'long' [-Winteger-overflow] 53 | long long_exp_array[4] = {LONG_MIN, INT_MIN - 1L, INT_MAX + 1L, LONG_MAX}; | ~~~~~~~~^~~~ /android0/llvm-project/libc/test/src/math/smoke/LdExpTest.h:53:49: warning: overflow in expression; result is 2'147'483'647 with type 'long' [-Winteger-overflow] 53 | long long_exp_array[4] = {LONG_MIN, INT_MIN - 1L, INT_MAX + 1L, LONG_MAX}; | ~~~~~~~~^~~~ /android0/llvm-project/libc/test/src/math/smoke/LdExpTest.h:53:63: warning: overflow in expression; result is -2'147'483'648 with type 'long' [-Winteger-overflow] 53 | long long_exp_array[4] = {LONG_MIN, INT_MIN - 1L, INT_MAX + 1L, LONG_MAX}; | ~~~~~~~~^~~~ /android0/llvm-project/libc/test/src/math/smoke/LdExpTest.h:53:49: warning: overflow in expression; result is 2'147'483'647 with type 'long' [-Winteger-overflow] 53 | long long_exp_array[4] = {LONG_MIN, INT_MIN - 1L, INT_MAX + 1L, LONG_MAX}; | ~~~~~~~~^~~~ ```
nickdesaulniers commented 1 week ago

cc @overmighty this was introduced by b5efd214297a5. Should we perhaps be using uint64_t (or long long) here? i386 is ILP32, so sizeof(long) == sizeof(int).

overmighty commented 1 week ago

cc @overmighty this was introduced by b5efd214297a5. Should we perhaps be using uint64_t (or long long) here? i386 is ILP32, so sizeof(long) == sizeof(int).

Sure. I remember getting those warnings, I guess it was while testing on 32-bit Arm. I didn't immediately fix it because it shouldn't be a problem in practice due to the if (/* ... */) return; before that line, but obviously it would be nice to compile without warnings.

nickdesaulniers commented 1 week ago

Right, that return won't prevent semantic analysis from flagging type related issues. Perhaps rather than an early return, we should just use preprocessor guards to include the relevant tests on LP64 only targets?

overmighty commented 1 week ago

Sure, but we should keep the if constexpr (sizeof(U) < sizeof(long)) part of the early return to avoid testing func with a long when it takes an int.