Repository: incubator-impala
Updated Branches:
  refs/heads/master 25ba76287 -> 927034682


IMPALA-5245: fix ASAN buffer-allocator-test

* Use the allocator_may_return_null=1 ASAN option so that the allocation
  just fails instead of crashing the process.
* Work around an bug in the implementation of the option where
  posix_memalign() does not return ENOMEM. I filed an upstream bug
  https://bugs.llvm.org/show_bug.cgi?id=32968.

Change-Id: I2f295f841b8b6e7996d54bdf2dfc0a092dcb864c
Reviewed-on: http://gerrit.cloudera.org:8080/6819
Reviewed-by: Jim Apple <[email protected]>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/fd62a7f7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/fd62a7f7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/fd62a7f7

Branch: refs/heads/master
Commit: fd62a7f774676a1d32f25a1805e75c969963027a
Parents: 25ba762
Author: Tim Armstrong <[email protected]>
Authored: Mon May 8 08:03:38 2017 -0700
Committer: Impala Public Jenkins <[email protected]>
Committed: Tue May 9 01:25:03 2017 +0000

----------------------------------------------------------------------
 be/src/runtime/bufferpool/buffer-allocator-test.cc | 6 ------
 be/src/runtime/bufferpool/system-allocator.cc      | 5 +++++
 be/src/testutil/mem-util.h                         | 1 +
 be/src/util/aligned-new.h                          | 1 +
 be/src/util/bloom-filter.cc                        | 1 +
 bin/run-backend-tests.sh                           | 2 +-
 bin/start-catalogd.sh                              | 2 +-
 bin/start-impalad.sh                               | 2 +-
 bin/start-statestored.sh                           | 2 +-
 9 files changed, 12 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd62a7f7/be/src/runtime/bufferpool/buffer-allocator-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/bufferpool/buffer-allocator-test.cc 
b/be/src/runtime/bufferpool/buffer-allocator-test.cc
index 4aff720..167298d 100644
--- a/be/src/runtime/bufferpool/buffer-allocator-test.cc
+++ b/be/src/runtime/bufferpool/buffer-allocator-test.cc
@@ -177,12 +177,6 @@ TEST_F(SystemAllocatorTest, LargeAllocFailure) {
 }
 
 int main(int argc, char** argv) {
-#ifdef ADDRESS_SANITIZER
-  // These tests are disabled for address sanitizer builds.
-  // TODO: fix IMPALA-5245 and re-enable.
-  cerr << "Buffer Allocator Test Skipped (IMPALA-5245)" << endl;
-  return 0;
-#endif
   ::testing::InitGoogleTest(&argc, argv);
   impala::InitCommonRuntime(argc, argv, true, impala::TestInfo::BE_TEST);
   int result = 0;

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd62a7f7/be/src/runtime/bufferpool/system-allocator.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/bufferpool/system-allocator.cc 
b/be/src/runtime/bufferpool/system-allocator.cc
index 756170a..8bbce70 100644
--- a/be/src/runtime/bufferpool/system-allocator.cc
+++ b/be/src/runtime/bufferpool/system-allocator.cc
@@ -120,6 +120,11 @@ Status SystemAllocator::AllocateViaMalloc(int64_t len, 
uint8_t** buffer_mem) {
   // This ensures that it can be backed by a whole pages, rather than parts of 
pages.
   size_t alignment = use_huge_pages ? HUGE_PAGE_SIZE : SMALL_PAGE_SIZE;
   int rc = posix_memalign(reinterpret_cast<void**>(buffer_mem), alignment, 
len);
+#ifdef ADDRESS_SANITIZER
+  // Workaround ASAN bug where posix_memalign returns 0 even when allocation 
fails.
+  // It should instead return ENOMEM. See 
https://bugs.llvm.org/show_bug.cgi?id=32968.
+  if (rc == 0 && *buffer_mem == nullptr && len != 0) rc = ENOMEM;
+#endif
   if (rc != 0) {
     return Status(TErrorCode::BUFFER_ALLOCATION_FAILED, len,
         Substitute("posix_memalign() failed to allocate buffer: $0", 
GetStrErrMsg()));

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd62a7f7/be/src/testutil/mem-util.h
----------------------------------------------------------------------
diff --git a/be/src/testutil/mem-util.h b/be/src/testutil/mem-util.h
index b4ce9ea..8436ad3 100644
--- a/be/src/testutil/mem-util.h
+++ b/be/src/testutil/mem-util.h
@@ -36,6 +36,7 @@ inline uint8_t* AllocateAligned(size_t size) {
   if (posix_memalign(&ptr, 64, size) != 0) {
     LOG(FATAL) << "Failed to allocate " << size;
   }
+  DCHECK(ptr != nullptr);
   return reinterpret_cast<uint8_t*>(ptr);
 }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd62a7f7/be/src/util/aligned-new.h
----------------------------------------------------------------------
diff --git a/be/src/util/aligned-new.h b/be/src/util/aligned-new.h
index b9197d9..42b2036 100644
--- a/be/src/util/aligned-new.h
+++ b/be/src/util/aligned-new.h
@@ -46,6 +46,7 @@ struct alignas(ALIGNMENT) AlignedNew {
       LOG(ERROR) << "Failed to allocate aligned memory; return code " << 
alloc_failed;
       throw std::bad_alloc();
     }
+    DCHECK(result != nullptr);
     return result;
   }
 };

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd62a7f7/be/src/util/bloom-filter.cc
----------------------------------------------------------------------
diff --git a/be/src/util/bloom-filter.cc b/be/src/util/bloom-filter.cc
index 3e930fa..d8d149e 100644
--- a/be/src/util/bloom-filter.cc
+++ b/be/src/util/bloom-filter.cc
@@ -49,6 +49,7 @@ BloomFilter::BloomFilter(const int log_heap_space)
   DCHECK_EQ(malloc_failed, 0) << "Malloc failed. log_heap_space: " << 
log_heap_space
                               << " log_num_buckets_: " << log_num_buckets_
                               << " alloc_size: " << alloc_size;
+  DCHECK(directory_ != nullptr);
   memset(directory_, 0, alloc_size);
 }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd62a7f7/bin/run-backend-tests.sh
----------------------------------------------------------------------
diff --git a/bin/run-backend-tests.sh b/bin/run-backend-tests.sh
index 74f457d..b07ce64 100755
--- a/bin/run-backend-tests.sh
+++ b/bin/run-backend-tests.sh
@@ -37,7 +37,7 @@ cd ${IMPALA_BE_DIR}
 cd ..
 
 export CTEST_OUTPUT_ON_FAILURE=1
-export ASAN_OPTIONS="handle_segv=0 detect_leaks=0"
+export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 allocator_may_return_null=1"
 export UBSAN_OPTIONS="print_stacktrace=1"
 UBSAN_OPTIONS="${UBSAN_OPTIONS} 
suppressions=${IMPALA_HOME}/bin/ubsan-suppressions.txt"
 export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin:${PATH}"

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd62a7f7/bin/start-catalogd.sh
----------------------------------------------------------------------
diff --git a/bin/start-catalogd.sh b/bin/start-catalogd.sh
index 41da5df..3eeaf2e 100755
--- a/bin/start-catalogd.sh
+++ b/bin/start-catalogd.sh
@@ -70,7 +70,7 @@ if ${CLUSTER_DIR}/admin is_kerberized; then
 fi
 
 . ${IMPALA_HOME}/bin/set-classpath.sh
-export ASAN_OPTIONS="handle_segv=0 detect_leaks=0"
+export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 allocator_may_return_null=1"
 export UBSAN_OPTIONS="print_stacktrace=1"
 UBSAN_OPTIONS="${UBSAN_OPTIONS} 
suppressions=${IMPALA_HOME}/bin/ubsan-suppressions.txt"
 export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin:${PATH}"

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd62a7f7/bin/start-impalad.sh
----------------------------------------------------------------------
diff --git a/bin/start-impalad.sh b/bin/start-impalad.sh
index 8eab812..6792cef 100755
--- a/bin/start-impalad.sh
+++ b/bin/start-impalad.sh
@@ -103,7 +103,7 @@ if ${CLUSTER_DIR}/admin is_kerberized; then
 fi
 
 . ${IMPALA_HOME}/bin/set-classpath.sh
-export ASAN_OPTIONS="handle_segv=0 detect_leaks=0"
+export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 allocator_may_return_null=1"
 export UBSAN_OPTIONS="print_stacktrace=1"
 UBSAN_OPTIONS="${UBSAN_OPTIONS} 
suppressions=${IMPALA_HOME}/bin/ubsan-suppressions.txt"
 export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin:${PATH}"

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd62a7f7/bin/start-statestored.sh
----------------------------------------------------------------------
diff --git a/bin/start-statestored.sh b/bin/start-statestored.sh
index d1eebca..85fe221 100755
--- a/bin/start-statestored.sh
+++ b/bin/start-statestored.sh
@@ -59,7 +59,7 @@ if ${CLUSTER_DIR}/admin is_kerberized; then
   fi
 fi
 
-export ASAN_OPTIONS="handle_segv=0 detect_leaks=0"
+export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 allocator_may_return_null=1"
 export UBSAN_OPTIONS="print_stacktrace=1"
 UBSAN_OPTIONS="${UBSAN_OPTIONS} 
suppressions=${IMPALA_HOME}/bin/ubsan-suppressions.txt"
 export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin:${PATH}"

Reply via email to