shifts of more bits than size of type are UB; clang 5.0 unleashed the…

… raptors
chistopher · May 10, 2019 · 2dd23aa · 2dd23aa
1 parent bd53c6f
commit 2dd23aa
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 15 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -3,7 +3,7 @@ language: cpp
 env:
   global:
     - CMAKE_OPTIONS="-DOPTION_BUILD_EXAMPLES=ON -DOPTION_BUILD_TESTS=ON"
-    - OMP_NUM_THREADS=4
+    - OMP_NUM_THREADS=2
 
 matrix:
   include:
@@ -13,20 +13,26 @@ matrix:
 #      osx_image: xcode9.1
 #      env: CMAKE_CONFIGURATION=debug BUILD_DIR=build-debug
 
+
+# clang needs a little hint to find its own omp library
     - os: linux
       compiler: clang
-      env: CMAKE_CONFIGURATION=release BUILD_DIR=build
+      env:
+        - CMAKE_CONFIGURATION=release BUILD_DIR=build
+        - LD_LIBRARY_PATH=/usr/local/clang/lib
 
     - os: linux
       compiler: clang
-      env: CMAKE_CONFIGURATION=debug BUILD_DIR=build-debug
+      env:
+        - CMAKE_CONFIGURATION=debug BUILD_DIR=build-debug
+        - LD_LIBRARY_PATH=/usr/local/clang/lib
 
 # ubuntu 14.04 (travis default) ships with too old gcc; updating is ugly
     - os: linux
       compiler: gcc
       env:
         - CMAKE_CONFIGURATION=release BUILD_DIR=build
-        - MATRIX_EVAL="CC=gcc-5 && CXX=g++-5"
+        - MATRIX_EVAL="CC=gcc-5 CXX=g++-5"
       addons:
         apt:
           sources:
@@ -37,15 +43,13 @@ matrix:
 
     - os: windows
       env:
+        - CMAKE_CONFIGURATION=release BUILD_DIR=build
         - CMAKE_GENERATOR_OVERRIDE="Visual Studio 15 2017 Win64"
-        - BUILD_DIR=build
-        - CMAKE_CONFIGURATION=release
 
     - os: windows
       env:
+        - CMAKE_CONFIGURATION=debug BUILD_DIR=build-debug
         - CMAKE_GENERATOR_OVERRIDE="Visual Studio 15 2017 Win64"
-        - BUILD_DIR=build-debug
-        - CMAKE_CONFIGURATION=debug
 
 before_script:
   - eval "${MATRIX_EVAL}"

diff --git a/source/girgs/include/girgs/BitManipulationGeneric.inl b/source/girgs/include/girgs/BitManipulationGeneric.inl
@@ -116,7 +116,9 @@ struct Deposit {
         unsigned int result = 0u;
         unsigned int bit = 0;
 
-        for(auto l = 0u; l*D < 32 + D; l++) {
+        // stop if next level would be (partly) unrepresentable
+        // thus we encode the same number of bits for each coordinate
+        for(auto l = 0u; (l+1)*D <= 32; l++) {
             for(auto d = 0u; d != D; d++) {
                 result |= ((coords[d] >> l) & 1) << bit++;
             }

diff --git a/source/tests/girgs-test/BitManipulation_test.cpp b/source/tests/girgs-test/BitManipulation_test.cpp
@@ -80,10 +80,14 @@ static std::array<uint32_t, D> ReferenceExtract(uint32_t x) {
 TYPED_TEST(BitManipulationTest, DepositeSingle) {
     COMMON_DEFS
 
+    const auto bits = 32 / D;
+    // use 64 bit (at 1ull) to prevent shifts larger or equal to size of type
+    const auto max_coord = static_cast<uint32_t>((1ull << bits) - 1);
+    std::uniform_int_distribution<unsigned> distr(0, max_coord);
+
     for(int d = 0; d < D; d++) {
-        const auto bits = 32 / D + (d < 32 % D);
+
         const auto allowedBits = girgs::BitPattern<D>::kEveryDthBit << d;
-        std::uniform_int_distribution<unsigned> distr(0, (1u << bits) - 1);
 
         for(int i=0; i < 1000; i++) {
             coord[d] = distr(prng);
@@ -94,18 +98,21 @@ TYPED_TEST(BitManipulationTest, DepositeSingle) {
             ASSERT_EQ(tested, ref) << coord;
         }
 
-        coord[d] = 0;
+        coord[d] = 0; // reset coord to all 0's again
     }
 }
 
 TYPED_TEST(BitManipulationTest, DepositeAll) {
     COMMON_DEFS
 
+    const auto bits = 32 / D;
+    // use 64 bit (at 1ull) to prevent shifts larger or equal to size of type
+    const auto max_coord = static_cast<uint32_t>((1ull << bits) - 1);
+    std::uniform_int_distribution<unsigned> distr(0, max_coord);
+
     std::uniform_int_distribution<unsigned> dim_distr(0, D - 1);
     for(int i=0; i < 10000; i++) {
         const auto d = dim_distr(prng);
-        const auto bits = 32 / D + (d < 32 % D);
-        std::uniform_int_distribution<unsigned> distr(0, (1u << bits) - 1);
 
         coord[d] = distr(prng);
         const auto tested = Impl::deposit(coord);
@@ -157,7 +164,10 @@ TYPED_TEST(BitManipulationTest, ExtractAll) {
 TYPED_TEST(BitManipulationTest, XRef) {
     COMMON_DEFS
 
-    std::uniform_int_distribution<unsigned> distr;
+    auto max_bits = (32/D)*D;
+    // use 64 bit (at 1ull) to prevent shifts larger or equal to size of type
+    auto max_cell = static_cast<uint32_t>((1ull << max_bits) - 1);
+    std::uniform_int_distribution<uint32_t> distr(0, max_cell);
     for(int i=0; i < 10000; i++) {
         const auto cell = distr(prng);