Mesa (master): util: Use uint64_t for shifting left in sign_extend and strunc

[GitLab Mirror]

Module: Mesa
Branch: master
Commit: 69420c28bdc8b7100252bf167475ab2ff29f7693
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=69420c28bdc8b7100252bf167475ab2ff29f7693

Author: Michel Dänzer <mdaenzer at redhat.com>
Date:   Wed Sep 25 11:44:24 2019 +0200

util: Use uint64_t for shifting left in sign_extend and strunc

Shifting int64_t values left into the sign bit has undefined behaviour:

../src/util/fast_idiv_by_const.c:175:14: runtime error: left shift of 131 by 56 places cannot be represented in type 'long int'
    #0 0x561337ed10c1 in sign_extend ../src/util/fast_idiv_by_const.c:175
    #1 0x561337ed1335 in util_compute_fast_sdiv_info ../src/util/fast_idiv_by_const.c:239
    #2 0x561337e17519 in fast_idiv_by_const_int8_Test::TestBody() ../src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp:357
    #3 0x561337ea815d in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../src/gtest/src/gtest.cc:2402
    #4 0x561337e93a6b in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../src/gtest/src/gtest.cc:2438
    #5 0x561337e38e75 in testing::Test::Run() ../src/gtest/src/gtest.cc:2474
    #6 0x561337e3b54e in testing::TestInfo::Run() ../src/gtest/src/gtest.cc:2656
    #7 0x561337e3dcdc in testing::TestCase::Run() ../src/gtest/src/gtest.cc:2774
    #8 0x561337e596b6 in testing::internal::UnitTestImpl::RunAllTests() ../src/gtest/src/gtest.cc:4649
    #9 0x561337eabd76 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../src/gtest/src/gtest.cc:2402
    #10 0x561337e97443 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../src/gtest/src/gtest.cc:2438
    #11 0x561337e50cd8 in testing::UnitTest::Run() ../src/gtest/src/gtest.cc:4257
    #12 0x561337ed0b91 in RUN_ALL_TESTS() ../src/gtest/include/gtest/gtest.h:2233
    #13 0x561337ed0a51 in main ../src/gtest/src/gtest_main.cc:37
    #14 0x7f85ba483bba in __libc_start_main ../csu/libc-start.c:308
    #15 0x561337e14dc9 in _start (/home/daenzer/src/mesa-git/mesa/build-amd64-sanitize/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test+0x96dc9)

../src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp:51:14: runtime error: left shift of negative value -63
    #0 0x55fc3c0e67cc in strunc ../src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp:51
    #1 0x55fc3c0e6d93 in smul_high ../src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp:140
    #2 0x55fc3c0e7067 in fast_sdiv ../src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp:181
    #3 0x55fc3c0e858b in fast_idiv_by_const_int8_Test::TestBody() ../src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp:358
    #4 0x55fc3c17915d in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../src/gtest/src/gtest.cc:2402
    #5 0x55fc3c164a6b in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../src/gtest/src/gtest.cc:2438
    #6 0x55fc3c109e75 in testing::Test::Run() ../src/gtest/src/gtest.cc:2474
    #7 0x55fc3c10c54e in testing::TestInfo::Run() ../src/gtest/src/gtest.cc:2656
    #8 0x55fc3c10ecdc in testing::TestCase::Run() ../src/gtest/src/gtest.cc:2774
    #9 0x55fc3c12a6b6 in testing::internal::UnitTestImpl::RunAllTests() ../src/gtest/src/gtest.cc:4649
    #10 0x55fc3c17cd76 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../src/gtest/src/gtest.cc:2402
    #11 0x55fc3c168443 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../src/gtest/src/gtest.cc:2438
    #12 0x55fc3c121cd8 in testing::UnitTest::Run() ../src/gtest/src/gtest.cc:4257
    #13 0x55fc3c1a1b91 in RUN_ALL_TESTS() ../src/gtest/include/gtest/gtest.h:2233
    #14 0x55fc3c1a1a51 in main ../src/gtest/src/gtest_main.cc:37
    #15 0x7fd224759bba in __libc_start_main ../csu/libc-start.c:308
    #16 0x55fc3c0e5dc9 in _start (/home/daenzer/src/mesa-git/mesa/build-amd64-sanitize/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test+0x96dc9)

v2:
* Use two casts instead of changing the argument type (Jason Ekstrand)

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
Reviewed-by: Adam Jackson <ajax at redhat.com>

---

 src/util/fast_idiv_by_const.c                                 | 2 +-
 src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/fast_idiv_by_const.c b/src/util/fast_idiv_by_const.c
index 7b93316268c..4f0f6b769b8 100644
--- a/src/util/fast_idiv_by_const.c
+++ b/src/util/fast_idiv_by_const.c
@@ -172,7 +172,7 @@ util_compute_fast_udiv_info(uint64_t D, unsigned num_bits, unsigned UINT_BITS)
 static inline int64_t
 sign_extend(int64_t x, unsigned SINT_BITS)
 {
-   return (x << (64 - SINT_BITS)) >> (64 - SINT_BITS);
+   return (int64_t)((uint64_t)x << (64 - SINT_BITS)) >> (64 - SINT_BITS);
 }
 
 struct util_fast_sdiv_info
diff --git a/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp b/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp
index b0b3669e73b..222346230e4 100644
--- a/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp
+++ b/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp
@@ -48,7 +48,7 @@ strunc(int64_t x, unsigned num_bits)
    if (num_bits == 64)
       return x;
 
-   return (x << (64 - num_bits)) >> (64 - num_bits);
+   return (int64_t)((uint64_t)x << (64 - num_bits)) >> (64 - num_bits);
 }
 
 static inline bool