[compiler-rt] r287078 - fixing binary search for cases when element is not in array
Kostya Serebryany via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 17 10:24:04 PST 2016
On Thu, Nov 17, 2016 at 10:23 AM, Kostya Serebryany <kcc at google.com> wrote:
> I don't agree with this change.
> Instead of changing the semantics of the existing function
> InternalBinarySearch
> I would prefer to introduce a new one (InternalLowerBound).
> Of course, you could introduce one via another.
>
... implement one via another.
>
> On Tue, Nov 15, 2016 at 8:03 PM, Mike Aizatsky via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: aizatsky
>> Date: Tue Nov 15 22:03:27 2016
>> New Revision: 287078
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=287078&view=rev
>> Log:
>> fixing binary search for cases when element is not in array
>>
>> Subscribers: kubabrecka
>>
>> Differential Revision: https://reviews.llvm.org/D26707
>>
>> Modified:
>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_stackdepot.cc
>> compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_common_test.cc
>>
>> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sa
>> nitizer_common/sanitizer_common.h?rev=287078&r1=287077&r2=
>> 287078&view=diff
>> ============================================================
>> ==================
>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h (original)
>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h Tue Nov 15
>> 22:03:27 2016
>> @@ -635,20 +635,19 @@ void InternalSort(Container *v, uptr siz
>> }
>> }
>>
>> +// Works like std::lower_bound: finds the first element that is not less
>> +// than the val.
>> template<class Container, class Value, class Compare>
>> uptr InternalBinarySearch(const Container &v, uptr first, uptr last,
>> const Value &val, Compare comp) {
>> - uptr not_found = last + 1;
>> - while (last >= first) {
>> + while (last > first) {
>> uptr mid = (first + last) / 2;
>> if (comp(v[mid], val))
>> first = mid + 1;
>> - else if (comp(val, v[mid]))
>> - last = mid - 1;
>> else
>> - return mid;
>> + last = mid;
>> }
>> - return not_found;
>> + return first;
>> }
>>
>> // Represents a binary loaded into virtual memory (e.g. this can be an
>>
>> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_stackdepot.cc
>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sa
>> nitizer_common/sanitizer_stackdepot.cc?rev=287078&r1=287077&
>> r2=287078&view=diff
>> ============================================================
>> ==================
>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_stackdepot.cc
>> (original)
>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_stackdepot.cc Tue
>> Nov 15 22:03:27 2016
>> @@ -155,7 +155,7 @@ StackTrace StackDepotReverseMap::Get(u32
>> IdDescPair pair = {id, nullptr};
>> uptr idx = InternalBinarySearch(map_, 0, map_.size(), pair,
>> IdDescPair::IdComparator);
>> - if (idx > map_.size())
>> + if (idx > map_.size() || map_[idx].id != id)
>> return StackTrace();
>> return map_[idx].desc->load();
>> }
>>
>> Modified: compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_
>> common_test.cc
>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sa
>> nitizer_common/tests/sanitizer_common_test.cc?rev=287078&r1=
>> 287077&r2=287078&view=diff
>> ============================================================
>> ==================
>> --- compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_common_test.cc
>> (original)
>> +++ compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_common_test.cc
>> Tue Nov 15 22:03:27 2016
>> @@ -10,6 +10,8 @@
>> // This file is a part of ThreadSanitizer/AddressSanitizer runtime.
>> //
>> //===------------------------------------------------------
>> ----------------===//
>> +#include <algorithm>
>> +
>> #include "sanitizer_common/sanitizer_allocator_internal.h"
>> #include "sanitizer_common/sanitizer_common.h"
>> #include "sanitizer_common/sanitizer_flags.h"
>> @@ -172,13 +174,52 @@ bool UptrLess(uptr a, uptr b) {
>>
>> TEST(SanitizerCommon, InternalBinarySearch) {
>> static const uptr kSize = 5;
>> - uptr arr[kSize];
>> - for (uptr i = 0; i < kSize; i++) arr[i] = i * i;
>> + int arr[kSize];
>> + arr[0] = 1;
>> + arr[1] = 3;
>> + arr[2] = 5;
>> + arr[3] = 7;
>> + arr[4] = 11;
>> +
>> + EXPECT_EQ(0u, InternalBinarySearch(arr, 0, kSize, 0, UptrLess));
>> + EXPECT_EQ(0u, InternalBinarySearch(arr, 0, kSize, 1, UptrLess));
>> + EXPECT_EQ(1u, InternalBinarySearch(arr, 0, kSize, 2, UptrLess));
>> + EXPECT_EQ(1u, InternalBinarySearch(arr, 0, kSize, 3, UptrLess));
>> + EXPECT_EQ(2u, InternalBinarySearch(arr, 0, kSize, 4, UptrLess));
>> + EXPECT_EQ(2u, InternalBinarySearch(arr, 0, kSize, 5, UptrLess));
>> + EXPECT_EQ(3u, InternalBinarySearch(arr, 0, kSize, 6, UptrLess));
>> + EXPECT_EQ(3u, InternalBinarySearch(arr, 0, kSize, 7, UptrLess));
>> + EXPECT_EQ(4u, InternalBinarySearch(arr, 0, kSize, 8, UptrLess));
>> + EXPECT_EQ(4u, InternalBinarySearch(arr, 0, kSize, 9, UptrLess));
>> + EXPECT_EQ(4u, InternalBinarySearch(arr, 0, kSize, 10, UptrLess));
>> + EXPECT_EQ(4u, InternalBinarySearch(arr, 0, kSize, 11, UptrLess));
>> + EXPECT_EQ(5u, InternalBinarySearch(arr, 0, kSize, 12, UptrLess));
>> +}
>> +
>> +TEST(SanitizerCommon, InternalBinarySearchVsLowerBound) {
>> + std::vector<int> data;
>> + auto create_item = [] (size_t i, size_t j) {
>> + auto v = i * 10000 + j;
>> + return ((v << 6) + (v >> 6) + 0x9e3779b9) % 100;
>> + };
>> + for (size_t i = 0; i < 1000; ++i) {
>> + data.resize(i);
>> + for (size_t j = 0; j < i; ++j) {
>> + data[j] = create_item(i, j);
>> + }
>>
>> - for (uptr i = 0; i < kSize; i++)
>> - ASSERT_EQ(InternalBinarySearch(arr, 0, kSize, i * i, UptrLess), i);
>> + std::sort(data.begin(), data.end());
>>
>> - ASSERT_EQ(InternalBinarySearch(arr, 0, kSize, 7, UptrLess), kSize +
>> 1);
>> + for (size_t j = 0; j < i; ++j) {
>> + int val = create_item(i, j);
>> + for (auto to_find : {val - 1, val, val + 1}) {
>> + uptr expected =
>> + std::lower_bound(data.begin(), data.end(), to_find) -
>> data.begin();
>> + EXPECT_EQ(expected, InternalBinarySearch(data.data(), 0,
>> data.size(),
>> + to_find,
>> std::less<int>()));
>> + }
>> + }
>> + }
>> }
>>
>> #if SANITIZER_LINUX && !SANITIZER_ANDROID
>>
>>
>> _______________________________________________
>> 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/20161117/37a0ba59/attachment.html>
More information about the llvm-commits
mailing list