The existing one is incorrect anyway. It literally fails when element you look for is not in the array. I doubt it is ever the intended behavior. <br><div class="gmail_quote"><div dir="ltr">On Thu, Nov 17, 2016 at 10:24 Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg">On Thu, Nov 17, 2016 at 10:23 AM, Kostya Serebryany <span dir="ltr" class="gmail_msg"><<a href="mailto:kcc@google.com" class="gmail_msg" target="_blank">kcc@google.com</a>></span> wrote:<br class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail_msg">I don't agree with this change. <div class="gmail_msg">Instead of changing the semantics of the existing function<span style="font-size:12.8px" class="gmail_msg"> </span><span style="font-size:12.8px" class="gmail_msg">InternalBinarySearch</span></div><div class="gmail_msg"><span style="font-size:12.8px" class="gmail_msg">I would prefer to introduce a new one (</span><span style="font-size:12.8px" class="gmail_msg">InternalLowerBound). </span></div><div class="gmail_msg"><span style="font-size:12.8px" class="gmail_msg">Of course, you could introduce one via another. </span></div></div></blockquote></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">... implement <span style="font-size:12.8px" class="gmail_msg">one via another. </span> </div></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="m_5891977038232642898gmail-HOEnZb gmail_msg"><div class="m_5891977038232642898gmail-h5 gmail_msg"><div class="gmail_extra gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg">On Tue, Nov 15, 2016 at 8:03 PM, Mike Aizatsky via llvm-commits <span dir="ltr" class="gmail_msg"><<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: aizatsky<br class="gmail_msg">
Date: Tue Nov 15 22:03:27 2016<br class="gmail_msg">
New Revision: 287078<br class="gmail_msg">
<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=287078&view=rev" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project?rev=287078&view=rev</a><br class="gmail_msg">
Log:<br class="gmail_msg">
fixing binary search for cases when element is not in array<br class="gmail_msg">
<br class="gmail_msg">
Subscribers: kubabrecka<br class="gmail_msg">
<br class="gmail_msg">
Differential Revision: <a href="https://reviews.llvm.org/D26707" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D26707</a><br class="gmail_msg">
<br class="gmail_msg">
Modified:<br class="gmail_msg">
compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h<br class="gmail_msg">
compiler-rt/trunk/lib/sanitizer_common/sanitizer_stackdepot.cc<br class="gmail_msg">
compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_common_test.cc<br class="gmail_msg">
<br class="gmail_msg">
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h?rev=287078&r1=287077&r2=287078&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h?rev=287078&r1=287077&r2=287078&view=diff</a><br class="gmail_msg">
==============================================================================<br class="gmail_msg">
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h (original)<br class="gmail_msg">
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h Tue Nov 15 22:03:27 2016<br class="gmail_msg">
@@ -635,20 +635,19 @@ void InternalSort(Container *v, uptr siz<br class="gmail_msg">
}<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
+// Works like std::lower_bound: finds the first element that is not less<br class="gmail_msg">
+// than the val.<br class="gmail_msg">
template<class Container, class Value, class Compare><br class="gmail_msg">
uptr InternalBinarySearch(const Container &v, uptr first, uptr last,<br class="gmail_msg">
const Value &val, Compare comp) {<br class="gmail_msg">
- uptr not_found = last + 1;<br class="gmail_msg">
- while (last >= first) {<br class="gmail_msg">
+ while (last > first) {<br class="gmail_msg">
uptr mid = (first + last) / 2;<br class="gmail_msg">
if (comp(v[mid], val))<br class="gmail_msg">
first = mid + 1;<br class="gmail_msg">
- else if (comp(val, v[mid]))<br class="gmail_msg">
- last = mid - 1;<br class="gmail_msg">
else<br class="gmail_msg">
- return mid;<br class="gmail_msg">
+ last = mid;<br class="gmail_msg">
}<br class="gmail_msg">
- return not_found;<br class="gmail_msg">
+ return first;<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
// Represents a binary loaded into virtual memory (e.g. this can be an<br class="gmail_msg">
<br class="gmail_msg">
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_stackdepot.cc<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_stackdepot.cc?rev=287078&r1=287077&r2=287078&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_stackdepot.cc?rev=287078&r1=287077&r2=287078&view=diff</a><br class="gmail_msg">
==============================================================================<br class="gmail_msg">
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_stackdepot.cc (original)<br class="gmail_msg">
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_stackdepot.cc Tue Nov 15 22:03:27 2016<br class="gmail_msg">
@@ -155,7 +155,7 @@ StackTrace StackDepotReverseMap::Get(u32<br class="gmail_msg">
IdDescPair pair = {id, nullptr};<br class="gmail_msg">
uptr idx = InternalBinarySearch(map_, 0, map_.size(), pair,<br class="gmail_msg">
IdDescPair::IdComparator);<br class="gmail_msg">
- if (idx > map_.size())<br class="gmail_msg">
+ if (idx > map_.size() || map_[idx].id != id)<br class="gmail_msg">
return StackTrace();<br class="gmail_msg">
return map_[idx].desc->load();<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
Modified: compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_common_test.cc<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_common_test.cc?rev=287078&r1=287077&r2=287078&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_common_test.cc?rev=287078&r1=287077&r2=287078&view=diff</a><br class="gmail_msg">
==============================================================================<br class="gmail_msg">
--- compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_common_test.cc (original)<br class="gmail_msg">
+++ compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_common_test.cc Tue Nov 15 22:03:27 2016<br class="gmail_msg">
@@ -10,6 +10,8 @@<br class="gmail_msg">
// This file is a part of ThreadSanitizer/AddressSanitizer runtime.<br class="gmail_msg">
//<br class="gmail_msg">
//===----------------------------------------------------------------------===//<br class="gmail_msg">
+#include <algorithm><br class="gmail_msg">
+<br class="gmail_msg">
#include "sanitizer_common/sanitizer_allocator_internal.h"<br class="gmail_msg">
#include "sanitizer_common/sanitizer_common.h"<br class="gmail_msg">
#include "sanitizer_common/sanitizer_flags.h"<br class="gmail_msg">
@@ -172,13 +174,52 @@ bool UptrLess(uptr a, uptr b) {<br class="gmail_msg">
<br class="gmail_msg">
TEST(SanitizerCommon, InternalBinarySearch) {<br class="gmail_msg">
static const uptr kSize = 5;<br class="gmail_msg">
- uptr arr[kSize];<br class="gmail_msg">
- for (uptr i = 0; i < kSize; i++) arr[i] = i * i;<br class="gmail_msg">
+ int arr[kSize];<br class="gmail_msg">
+ arr[0] = 1;<br class="gmail_msg">
+ arr[1] = 3;<br class="gmail_msg">
+ arr[2] = 5;<br class="gmail_msg">
+ arr[3] = 7;<br class="gmail_msg">
+ arr[4] = 11;<br class="gmail_msg">
+<br class="gmail_msg">
+ EXPECT_EQ(0u, InternalBinarySearch(arr, 0, kSize, 0, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(0u, InternalBinarySearch(arr, 0, kSize, 1, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(1u, InternalBinarySearch(arr, 0, kSize, 2, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(1u, InternalBinarySearch(arr, 0, kSize, 3, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(2u, InternalBinarySearch(arr, 0, kSize, 4, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(2u, InternalBinarySearch(arr, 0, kSize, 5, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(3u, InternalBinarySearch(arr, 0, kSize, 6, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(3u, InternalBinarySearch(arr, 0, kSize, 7, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(4u, InternalBinarySearch(arr, 0, kSize, 8, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(4u, InternalBinarySearch(arr, 0, kSize, 9, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(4u, InternalBinarySearch(arr, 0, kSize, 10, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(4u, InternalBinarySearch(arr, 0, kSize, 11, UptrLess));<br class="gmail_msg">
+ EXPECT_EQ(5u, InternalBinarySearch(arr, 0, kSize, 12, UptrLess));<br class="gmail_msg">
+}<br class="gmail_msg">
+<br class="gmail_msg">
+TEST(SanitizerCommon, InternalBinarySearchVsLowerBound) {<br class="gmail_msg">
+ std::vector<int> data;<br class="gmail_msg">
+ auto create_item = [] (size_t i, size_t j) {<br class="gmail_msg">
+ auto v = i * 10000 + j;<br class="gmail_msg">
+ return ((v << 6) + (v >> 6) + 0x9e3779b9) % 100;<br class="gmail_msg">
+ };<br class="gmail_msg">
+ for (size_t i = 0; i < 1000; ++i) {<br class="gmail_msg">
+ data.resize(i);<br class="gmail_msg">
+ for (size_t j = 0; j < i; ++j) {<br class="gmail_msg">
+ data[j] = create_item(i, j);<br class="gmail_msg">
+ }<br class="gmail_msg">
<br class="gmail_msg">
- for (uptr i = 0; i < kSize; i++)<br class="gmail_msg">
- ASSERT_EQ(InternalBinarySearch(arr, 0, kSize, i * i, UptrLess), i);<br class="gmail_msg">
+ std::sort(data.begin(), data.end());<br class="gmail_msg">
<br class="gmail_msg">
- ASSERT_EQ(InternalBinarySearch(arr, 0, kSize, 7, UptrLess), kSize + 1);<br class="gmail_msg">
+ for (size_t j = 0; j < i; ++j) {<br class="gmail_msg">
+ int val = create_item(i, j);<br class="gmail_msg">
+ for (auto to_find : {val - 1, val, val + 1}) {<br class="gmail_msg">
+ uptr expected =<br class="gmail_msg">
+ std::lower_bound(data.begin(), data.end(), to_find) - data.begin();<br class="gmail_msg">
+ EXPECT_EQ(expected, InternalBinarySearch(data.data(), 0, data.size(),<br class="gmail_msg">
+ to_find, std::less<int>()));<br class="gmail_msg">
+ }<br class="gmail_msg">
+ }<br class="gmail_msg">
+ }<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
#if SANITIZER_LINUX && !SANITIZER_ANDROID<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
_______________________________________________<br class="gmail_msg">
llvm-commits mailing list<br class="gmail_msg">
<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg">
</blockquote></div><br class="gmail_msg"></div>
</div></div></blockquote></div></div></div></blockquote></div>