From 990952b6644e79da08c6925bddedff7c3b3ebe8a Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Sun, 19 Jun 2016 07:08:27 +0000 Subject: [PATCH] Fix various undefined behavior found by UBSan. * Fix non-null violation in strstream.cpp Overflow was calling memcpy with a null parameter and a size of 0. * Fix std/atomics/atomics.flag/ tests: a.test_and_set() was reading from an uninitialized atomic, but wasn't using the value. The tests now clear the flag before performing the first test_and_set. This allows UBSAN to test that clear doesn't read an invalid value. * Fix std/experimental/algorithms/alg.random.sample/sample.pass.cpp The tests were dereferencing a past-the-end pointer to an array so that they could do pointer arithmetic with it. Instead of dereference the iterator I changed the tests to use the special 'base()' test iterator method. * Add -fno-sanitize=float-divide-by-zero to suppress division by zero UBSAN diagnostics. The tests that cause float division by zero are explicitly aware that they are doing that. Since this is well defined for IEEE floats suppress the warnings for now. llvm-svn: 273107 --- libcxx/src/strstream.cpp | 6 ++++- libcxx/test/libcxx/test/config.py | 3 ++- .../atomics.flag/atomic_flag_clear.pass.cpp | 2 ++ .../atomic_flag_clear_explicit.pass.cpp | 20 +++++++++----- .../std/atomics/atomics.flag/clear.pass.cpp | 26 ++++++++++++------- .../alg.random.sample/sample.pass.cpp | 12 ++++----- 6 files changed, 45 insertions(+), 24 deletions(-) diff --git a/libcxx/src/strstream.cpp b/libcxx/src/strstream.cpp index ea728138db66..83702fc72e80 100644 --- a/libcxx/src/strstream.cpp +++ b/libcxx/src/strstream.cpp @@ -11,6 +11,7 @@ #include "algorithm" #include "climits" #include "cstring" +#include "__debug" _LIBCPP_BEGIN_NAMESPACE_STD @@ -167,7 +168,10 @@ strstreambuf::overflow(int_type __c) buf = new char[new_size]; if (buf == nullptr) return int_type(EOF); - memcpy(buf, eback(), static_cast(old_size)); + if (old_size != 0) { + _LIBCPP_ASSERT(eback(), "overflow copying from NULL"); + memcpy(buf, eback(), static_cast(old_size)); + } ptrdiff_t ninp = gptr() - eback(); ptrdiff_t einp = egptr() - eback(); ptrdiff_t nout = pptr() - pbase(); diff --git a/libcxx/test/libcxx/test/config.py b/libcxx/test/libcxx/test/config.py index 4d4eb066e026..5632e535d4b7 100644 --- a/libcxx/test/libcxx/test/config.py +++ b/libcxx/test/libcxx/test/config.py @@ -620,12 +620,13 @@ class Configuration(object): blacklist = os.path.join(self.libcxx_src_root, 'test/ubsan_blacklist.txt') self.cxx.flags += ['-fsanitize=undefined', - '-fno-sanitize=vptr,function', + '-fno-sanitize=vptr,function,float-divide-by-zero', '-fno-sanitize-recover=all', '-fsanitize-blacklist=' + blacklist] self.cxx.compile_flags += ['-O3'] self.env['UBSAN_OPTIONS'] = 'print_stacktrace=1' self.config.available_features.add('ubsan') + self.config.available_features.add('sanitizer-new-delete') elif san == 'Thread': self.cxx.flags += ['-fsanitize=thread'] self.config.available_features.add('tsan') diff --git a/libcxx/test/std/atomics/atomics.flag/atomic_flag_clear.pass.cpp b/libcxx/test/std/atomics/atomics.flag/atomic_flag_clear.pass.cpp index 64093d639e44..22bbbd6af535 100644 --- a/libcxx/test/std/atomics/atomics.flag/atomic_flag_clear.pass.cpp +++ b/libcxx/test/std/atomics/atomics.flag/atomic_flag_clear.pass.cpp @@ -23,12 +23,14 @@ int main() { { std::atomic_flag f; + f.clear(); f.test_and_set(); atomic_flag_clear(&f); assert(f.test_and_set() == 0); } { volatile std::atomic_flag f; + f.clear(); f.test_and_set(); atomic_flag_clear(&f); assert(f.test_and_set() == 0); diff --git a/libcxx/test/std/atomics/atomics.flag/atomic_flag_clear_explicit.pass.cpp b/libcxx/test/std/atomics/atomics.flag/atomic_flag_clear_explicit.pass.cpp index e1a9349c9394..1a212c6f352a 100644 --- a/libcxx/test/std/atomics/atomics.flag/atomic_flag_clear_explicit.pass.cpp +++ b/libcxx/test/std/atomics/atomics.flag/atomic_flag_clear_explicit.pass.cpp @@ -22,38 +22,44 @@ int main() { { - std::atomic_flag f; - f.test_and_set(); + std::atomic_flag f; // uninitialized first + atomic_flag_clear_explicit(&f, std::memory_order_relaxed); + assert(f.test_and_set() == 0); atomic_flag_clear_explicit(&f, std::memory_order_relaxed); assert(f.test_and_set() == 0); } { std::atomic_flag f; - f.test_and_set(); + atomic_flag_clear_explicit(&f, std::memory_order_release); + assert(f.test_and_set() == 0); atomic_flag_clear_explicit(&f, std::memory_order_release); assert(f.test_and_set() == 0); } { std::atomic_flag f; - f.test_and_set(); + atomic_flag_clear_explicit(&f, std::memory_order_seq_cst); + assert(f.test_and_set() == 0); atomic_flag_clear_explicit(&f, std::memory_order_seq_cst); assert(f.test_and_set() == 0); } { volatile std::atomic_flag f; - f.test_and_set(); + atomic_flag_clear_explicit(&f, std::memory_order_relaxed); + assert(f.test_and_set() == 0); atomic_flag_clear_explicit(&f, std::memory_order_relaxed); assert(f.test_and_set() == 0); } { volatile std::atomic_flag f; - f.test_and_set(); + atomic_flag_clear_explicit(&f, std::memory_order_release); + assert(f.test_and_set() == 0); atomic_flag_clear_explicit(&f, std::memory_order_release); assert(f.test_and_set() == 0); } { volatile std::atomic_flag f; - f.test_and_set(); + atomic_flag_clear_explicit(&f, std::memory_order_seq_cst); + assert(f.test_and_set() == 0); atomic_flag_clear_explicit(&f, std::memory_order_seq_cst); assert(f.test_and_set() == 0); } diff --git a/libcxx/test/std/atomics/atomics.flag/clear.pass.cpp b/libcxx/test/std/atomics/atomics.flag/clear.pass.cpp index 65051af790d2..255af8f176ea 100644 --- a/libcxx/test/std/atomics/atomics.flag/clear.pass.cpp +++ b/libcxx/test/std/atomics/atomics.flag/clear.pass.cpp @@ -22,50 +22,58 @@ int main() { { - std::atomic_flag f; - f.test_and_set(); + std::atomic_flag f; // uninitialized + f.clear(); + assert(f.test_and_set() == 0); f.clear(); assert(f.test_and_set() == 0); } { std::atomic_flag f; - f.test_and_set(); + f.clear(std::memory_order_relaxed); + assert(f.test_and_set() == 0); f.clear(std::memory_order_relaxed); assert(f.test_and_set() == 0); } { std::atomic_flag f; - f.test_and_set(); + f.clear(std::memory_order_release); + assert(f.test_and_set() == 0); f.clear(std::memory_order_release); assert(f.test_and_set() == 0); } { std::atomic_flag f; - f.test_and_set(); + f.clear(std::memory_order_seq_cst); + assert(f.test_and_set() == 0); f.clear(std::memory_order_seq_cst); assert(f.test_and_set() == 0); } { volatile std::atomic_flag f; - f.test_and_set(); + f.clear(); + assert(f.test_and_set() == 0); f.clear(); assert(f.test_and_set() == 0); } { volatile std::atomic_flag f; - f.test_and_set(); + f.clear(std::memory_order_relaxed); + assert(f.test_and_set() == 0); f.clear(std::memory_order_relaxed); assert(f.test_and_set() == 0); } { volatile std::atomic_flag f; - f.test_and_set(); + f.clear(std::memory_order_release); + assert(f.test_and_set() == 0); f.clear(std::memory_order_release); assert(f.test_and_set() == 0); } { volatile std::atomic_flag f; - f.test_and_set(); + f.clear(std::memory_order_seq_cst); + assert(f.test_and_set() == 0); f.clear(std::memory_order_seq_cst); assert(f.test_and_set() == 0); } diff --git a/libcxx/test/std/experimental/algorithms/alg.random.sample/sample.pass.cpp b/libcxx/test/std/experimental/algorithms/alg.random.sample/sample.pass.cpp index 0f0784d6ee90..1a9f9b099b20 100644 --- a/libcxx/test/std/experimental/algorithms/alg.random.sample/sample.pass.cpp +++ b/libcxx/test/std/experimental/algorithms/alg.random.sample/sample.pass.cpp @@ -64,12 +64,12 @@ void test() { end = std::experimental::sample(PopulationIterator(ia), PopulationIterator(ia + is), SampleIterator(oa), os, g); - assert(&*end - oa == std::min(os, is)); + assert(end.base() - oa == std::min(os, is)); assert(std::equal(oa, oa + os, oa1)); end = std::experimental::sample(PopulationIterator(ia), PopulationIterator(ia + is), SampleIterator(oa), os, g); - assert(&*end - oa == std::min(os, is)); + assert(end.base() - oa == std::min(os, is)); assert(std::equal(oa, oa + os, oa2)); } @@ -85,7 +85,7 @@ void test_empty_population() { SampleIterator end = std::experimental::sample(PopulationIterator(ia), PopulationIterator(ia), SampleIterator(oa), os, g); - assert(&*end == oa); + assert(end.base() == oa); } template class PopulationIteratorType, class PopulationItem, @@ -100,7 +100,7 @@ void test_empty_sample() { SampleIterator end = std::experimental::sample(PopulationIterator(ia), PopulationIterator(ia + is), SampleIterator(oa), 0, g); - assert(&*end == oa); + assert(end.base() == oa); } template class PopulationIteratorType, class PopulationItem, @@ -119,8 +119,8 @@ void test_small_population() { end = std::experimental::sample(PopulationIterator(ia), PopulationIterator(ia + is), SampleIterator(oa), os, g); - assert(&*end - oa == std::min(os, is)); - assert(std::equal(oa, &*end, oa1)); + assert(end.base() - oa == std::min(os, is)); + assert(std::equal(oa, end.base(), oa1)); } int main() {