Merge pull request #550 from LLNL/hotfix/zero-byte-tracking

Release v5.0.1 hotfix

CHANGELOG.md

@@ -6,7 +6,12 @@ The format is based on [Keep a
 Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [Develop Branch]
+## [v5.0.1] - 2021-03-31
+
+### Fixed
+
+- Fixed UM-851 where zero-byte allocations were sometimes incorrectly reported
+  as not being found
 
 ## [v5.0.0] - 2020-11-18
 

CMakeLists.txt

@@ -11,7 +11,7 @@ cmake_policy(SET CMP0057 NEW)
 
 project(Umpire
   LANGUAGES CXX C
-  VERSION 5.0.0)
+  VERSION 5.0.1)
 
 set(UMPIRE_VERSION_RC "")
 
@@ -60,7 +60,6 @@ set(CMAKE_CUDA_STANDARD 11)
 if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "PGI")
     message(STATUS "Setting CMAKE_CXX_EXTENSIONS to ON for PGI Compiler")
     set( CMAKE_CXX_EXTENSIONS ON )
-  endif()
 endif()
 
 if (ENABLE_CUDA)

README.md

@@ -1,4 +1,4 @@
-# <img src="https://cdn.rawgit.com/LLNL/Umpire/develop/share/umpire/logo/umpire-logo.png" width="128" valign="middle" alt="Umpire"/>  Umpire v5.0.0
+# <img src="https://cdn.rawgit.com/LLNL/Umpire/develop/share/umpire/logo/umpire-logo.png" width="128" valign="middle" alt="Umpire"/>  Umpire v5.0.1
 
 [![Travis Build Status](https://travis-ci.com/LLNL/Umpire.svg?branch=develop)](https://travis-ci.com/LLNL/Umpire)
 [![Azure Pipelines Build Status](https://dev.azure.com/davidbeckingsale/Umpire/_apis/build/status/LLNL.Umpire?branchName=develop)](https://dev.azure.com/davidbeckingsale/Umpire/_build/latest?definitionId=1&branchName=develop)

RELEASE

@@ -1,4 +1,4 @@
-Umpire Version 5.0.0
+Umpire Version 5.0.1
 
 Copyright (c) 2016-20, Lawrence Livermore National Security, LLC. 
 Produced at the Lawrence Livermore National Laboratory.

RELEASE_NOTES.md

@@ -1,3 +1,8 @@
+# v5.0.1
+
+- Fixed bug where zero-byte allocations from Umpire were sometimes incorrectly
+  reported as not being Umpire allocations
+
 # v5.0.0
 
 - Memory Resource header and source files for HIP

docs/sphinx/conf.py

@@ -98,7 +98,7 @@
 # The short X.Y version.
 version = u'5.0'
 # The full version, including alpha/beta/rc tags.
-release = u'5.0.0'
+release = u'5.0.1'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.

docs/sphinx/conf.py.in

@@ -98,7 +98,7 @@ author = u'David Beckingsale'
 # The short X.Y version.
 version = u'5.0'
 # The full version, including alpha/beta/rc tags.
-release = u'5.0.0'
+release = u'5.0.1'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.

scripts/make_release_tarball.sh

@@ -7,7 +7,7 @@
 ##############################################################################
 
 TAR_CMD=gtar
-VERSION=5.0.0
+VERSION=5.0.1
 
 git archive --prefix=umpire-${VERSION}/ -o umpire-${VERSION}.tar HEAD 2> /dev/null
 

src/umpire/tpl/judy/judy.c

@@ -322,6 +322,7 @@ void judy_free( Judy * judy, void * block, int type ) {
 
 unsigned int judy_key( Judy * judy, unsigned char * buff, unsigned int max ) {
     judyvalue * dest = ( judyvalue * )buff;
+    judyvalue num_bits;
     unsigned int len = 0, idx = 0, depth;
     int slot, off, type;
     judyvalue value;
@@ -393,7 +394,20 @@ unsigned int judy_key( Judy * judy, unsigned char * buff, unsigned int max ) {
 
             case JUDY_radix:
                 if( judy->depth ) {
-                    dest[depth] |= ( judyvalue )slot << ( JUDY_key_size - ( ++len & JUDY_key_mask ) ) * 8;
+                    //
+                    // Convert key and len sizes, specified in # of bytes, to
+                    // the number of bits for a bit shift operation. This is
+                    // the reason for the '* 8' multipliers below.
+                    //
+                    num_bits = (JUDY_key_size - ( ++len & JUDY_key_mask )) * 8;
+
+                    //
+                    // Insure shift amount stays within JUDY_key_size word
+                    //
+                    num_bits &= (JUDY_key_size * 8) - 1;
+
+                    dest[depth] |= ( judyvalue )slot << num_bits;
+
                     if( !( len & JUDY_key_mask ) ) {
                         depth++;
                     }
@@ -549,7 +563,7 @@ JudySlot * judy_slot( Judy * judy, const unsigned char * buff, unsigned int max
                         return NULL;
                     }
                 }
-        
+
 
                 next = table[slot & 0x0F];
                 continue;

tests/tpl/CMakeLists.txt

@@ -13,11 +13,20 @@ blt_add_test(
   NAME judy_smoke_tests
   COMMAND judy_smoke_tests)
 
+blt_add_executable(
+  NAME judy_test
+  SOURCES judy_test.cpp
+  DEPENDS_ON umpire_tpl_judy gtest)
+
+blt_add_test(
+  NAME judy_test
+  COMMAND judy_test)
+
 blt_add_executable(
   NAME cli11_smoke_test
   SOURCES cli11_smoke_test.cpp
   DEPENDS_ON umpire_tpl_CLI11 gtest)
 
 blt_add_test(
   NAME cli11_smoke_help
-  COMMAND cli11_smoke_test)
+  COMMAND cli11_smoke_test)

tests/tpl/judy_test.cpp

@@ -0,0 +1,95 @@
+//////////////////////////////////////////////////////////////////////////////
+// Copyright (c) 2016-20, Lawrence Livermore National Security, LLC and Umpire
+// project contributors. See the COPYRIGHT file for details.
+//
+// SPDX-License-Identifier: (MIT)
+//////////////////////////////////////////////////////////////////////////////
+
+#include "gtest/gtest.h"
+
+#include <cstdint>
+#include <sstream>
+#include <string>
+
+#include "umpire/tpl/judy/judy.h"
+
+class JudyTest : public ::testing::Test {
+ protected:
+  //  judy_open:  open a new judy array returning a judy object.
+  JudyTest() : array{ judy_open(key_length, key_depth) }
+  {
+  }
+
+  ~JudyTest()
+  {
+    judy_close(array);
+  }
+
+  void TearDown() override
+  {
+  }
+
+  const unsigned int key_depth{1};
+  const unsigned int key_length{sizeof(uintptr_t)};
+  const uint64_t bad_judy_base{ 0x0000200000070000 };
+  const uint64_t max_keys{1000};
+  Judy* array;
+};
+
+TEST_F(JudyTest, Construction)
+{
+  EXPECT_TRUE(array != nullptr);
+}
+
+TEST_F(JudyTest, WriteValues)
+{
+  EXPECT_TRUE(array != nullptr);
+
+  for (uint64_t key = bad_judy_base; key < bad_judy_base+max_keys; key++) {
+    uint64_t k{ key };
+
+    // judy_cell:  insert a string into the judy array, return cell pointer.
+    JudySlot* cell{ judy_cell(array, reinterpret_cast<unsigned char*>(&k), key_length) };
+    EXPECT_TRUE(cell != nullptr);
+
+    uint64_t* val{ reinterpret_cast<uint64_t*>(cell) };
+    *val = key+1;    // Just make up a non-zero value, doesn't matter what
+  }
+
+  for (uint64_t key = bad_judy_base; key < bad_judy_base+max_keys; key++) {
+    {
+      uint64_t k{ key };
+
+      // judy_strt: retrieve the cell pointer greater than or equal to given key
+      JudySlot* cell{ judy_strt(array, reinterpret_cast<unsigned char*>(&k), key_length) };
+      EXPECT_TRUE(cell != nullptr);
+
+      uint64_t* val{ reinterpret_cast<uint64_t*>(cell) };
+
+      EXPECT_EQ(*val, key+1);
+
+      k = 0;
+      judy_key(array, reinterpret_cast<unsigned char*>(&k), key_length);
+
+      EXPECT_EQ(k, key );
+    }
+
+    {
+      uint64_t k{ key };
+
+      // judy_slot: retrieve the cell pointer, or return NULL for a given key.
+      JudySlot* cell{
+        judy_slot(array, reinterpret_cast<unsigned char*>(&k), key_length) };
+      EXPECT_TRUE(cell != nullptr);
+
+      // judy_key:   retrieve the string value for the most recent judy query.
+      uint64_t* val{ reinterpret_cast<uint64_t*>(cell) };
+
+      EXPECT_EQ(*val, key+1);
+
+      k = 0;
+      judy_key(array, reinterpret_cast<unsigned char*>(&k), key_length);
+      EXPECT_EQ(k, key );
+    }
+  }
+}

tests/unit/strategy/CMakeLists.txt

@@ -25,3 +25,12 @@ blt_add_executable(
 blt_add_test(
   NAME fixed_pool_tests
   COMMAND fixed_pool_tests)
+
+blt_add_executable(
+  NAME zero_byte_handler_tests
+  SOURCES zero_byte_handler_tests.cpp
+  DEPENDS_ON ${strategy_tests_depends})
+
+blt_add_test(
+  NAME zero_byte_handler_tests
+  COMMAND zero_byte_handler_tests)

tests/unit/strategy/zero_byte_handler_tests.cpp

@@ -0,0 +1,31 @@
+//////////////////////////////////////////////////////////////////////////////
+// Copyright (c) 2016-20, Lawrence Livermore National Security, LLC and Umpire
+// project contributors. See the COPYRIGHT file for details.
+//
+// SPDX-License-Identifier: (MIT)
+//////////////////////////////////////////////////////////////////////////////
+
+#include "gtest/gtest.h"
+
+#include <vector>
+
+#include "umpire/ResourceManager.hpp"
+#include "umpire/Allocator.hpp"
+
+TEST(ZeroByteHandlerTest, Construction)
+{
+  std::size_t max_allocations{1000};
+  auto& rm = umpire::ResourceManager::getInstance();
+  auto alloc = rm.getAllocator("HOST");
+  std::vector<void*> allocations;
+
+  std::size_t i{0};
+  for ( ; i < max_allocations; i++) {
+    ASSERT_NO_THROW(allocations.push_back(alloc.allocate(0)));
+  }
+
+  for ( auto& a : allocations ) {
+    ASSERT_TRUE(rm.hasAllocator(a) == true);
+    ASSERT_NO_THROW(alloc.deallocate(a));
+  }
+}