[llvm] r300721 - [MathExtras] Fix undefined behavior (shift by bit width)

[Kostya Serebryany via llvm-commits]

Thanks!
I was just going to complain :)

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/4299/steps/check-llvm%20ubsan/logs/stdio

[ RUN      ] MathExtras.onesMask
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/MathExtras.h:208:30:
runtime error: shift exponent 32 is too large for 32-bit type
'unsigned int'
    #0 0x58f194 in unsigned int llvm::maskTrailingOnes<unsigned
int>(unsigned int)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/MathExtras.h:208:30
    #1 0x57bfb0 in (anonymous
namespace)::MathExtras_onesMask_Test::TestBody()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/unittests/Support/MathExtrasTest.cpp:75:3
    #2 0x79e2a9 in testing::Test::Run()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2474:5
    #3 0x79ed68 in testing::TestInfo::Run()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
    #4 0x79f462 in testing::TestCase::Run()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
    #5 0x7a5933 in testing::internal::UnitTestImpl::RunAllTests()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
    #6 0x7a55cb in testing::UnitTest::Run()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4257:10
    #7 0x794603 in main
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:51:10
    #8 0x7f357461182f in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #9 0x430478 in _start
(/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/unittests/Support/SupportTests+0x430478)


On Wed, Apr 19, 2017 at 10:46 AM, Benjamin Kramer via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: d0k
> Date: Wed Apr 19 12:46:15 2017
> New Revision: 300721
>
> URL: http://llvm.org/viewvc/llvm-project?rev=300721&view=rev
> Log:
> [MathExtras] Fix undefined behavior (shift by bit width)
>
> While there add some unit tests for uint64_t. Found by ubsan.
>
> Modified:
>     llvm/trunk/include/llvm/Support/MathExtras.h
>     llvm/trunk/unittests/Support/MathExtrasTest.cpp
>
> Modified: llvm/trunk/include/llvm/Support/MathExtras.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
> llvm/Support/MathExtras.h?rev=300721&r1=300720&r2=300721&view=diff
> ============================================================
> ==================
> --- llvm/trunk/include/llvm/Support/MathExtras.h (original)
> +++ llvm/trunk/include/llvm/Support/MathExtras.h Wed Apr 19 12:46:15 2017
> @@ -205,7 +205,7 @@ template <typename T> T maskTrailingOnes
>    static_assert(std::is_unsigned<T>::value, "Invalid type!");
>    const unsigned Bits = CHAR_BIT * sizeof(T);
>    assert(N <= Bits && "Invalid bit index");
> -  return -T(N != 0) & (T(-1) >> (Bits - N));
> +  return N == 0 ? 0 : (T(-1) >> (Bits - N));
>  }
>
>  /// \brief Create a bitmask with the N left-most bits set to 1, and all
> other
>
> Modified: llvm/trunk/unittests/Support/MathExtrasTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/
> Support/MathExtrasTest.cpp?rev=300721&r1=300720&r2=300721&view=diff
> ============================================================
> ==================
> --- llvm/trunk/unittests/Support/MathExtrasTest.cpp (original)
> +++ llvm/trunk/unittests/Support/MathExtrasTest.cpp Wed Apr 19 12:46:15
> 2017
> @@ -84,6 +84,11 @@ TEST(MathExtras, onesMask) {
>
>    EXPECT_EQ(0xFFFFFFFFU, maskTrailingOnes<uint32_t>(32U));
>    EXPECT_EQ(0xFFFFFFFFU, maskLeadingOnes<uint32_t>(32U));
> +  EXPECT_EQ(0xFFFFFFFFFFFFFFFFULL, maskTrailingOnes<uint64_t>(64U));
> +  EXPECT_EQ(0xFFFFFFFFFFFFFFFFULL, maskLeadingOnes<uint64_t>(64U));
> +
> +  EXPECT_EQ(0x0000FFFFFFFFFFFFULL, maskTrailingOnes<uint64_t>(48U));
> +  EXPECT_EQ(0xFFFFFFFFFFFF0000ULL, maskLeadingOnes<uint64_t>(48U));
>  }
>
>  TEST(MathExtras, findFirstSet) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170419/5f588c48/attachment.html>