
Nearly all binary search and merge sort implementations are broken (2006) - arto
http://googleresearch.blogspot.com/2006/06/extra-extra-read-all-about-it-nearly.html
======
DarkShikari
_It is not sufficient merely to prove a program correct; you have to test it
too. Moreover, to be really certain that a program is correct, you have to
test it for all possible input values, but this is seldom feasible._

This statement is tantamount to saying "you don't merely need to prove
Fermat's Last Theorem, you also have to test it for all possible input
values". By this line of reasoning, most of mathematics should be thrown out.

If you've proven a program correct, _it is correct for all input values_ ,
whether you have tested them or not. If your proof ignores complexities such
as the range of data values, it isn't a correct proof to begin with. And yes,
proving programs correct is often possible -- in fact, the highest levels of
software verification actually require explicit proofs of correctness for much
of the code; see
[http://en.wikipedia.org/wiki/Evaluation_Assurance_Level#EAL7...](http://en.wikipedia.org/wiki/Evaluation_Assurance_Level#EAL7:_Formally_Verified_Design_and_Tested)

~~~
jrockway
Mathematics sits atop a very small set of trusted axioms. Computer hardware is
not so reliable; the parts are made in China as cheaply as possible. So even
if you've proven something correct in theory, it doesn't matter because of all
the additional variables that the real world introduces.

The big insights in computer science look very much like mathematics; the
mechanics of a binary search work perfectly in theory. But in practice, it's
easy to make a simple mistake in implementation and accidentally throw your
proof out the window. The proof remains valid, but your implementation isn't
doing what the proof says.

The fact that a famous book on proving correct and then implementing a binary
search ended up being slightly wrong underscores how easy it is to make this
mistake. (I find binary search a little annoying because of the integer
division. Do you round up or round down? What does your language
implementation do? Now you see why you need to mentally prove that you've
written the right algorithm, and then test to make sure your computer is doing
what you think you're telling it to.)

~~~
bad_user
Actually most bugs are being introduced in the translation between pseudocode
and your working programming language. For instance pseudocode does not deal
with 32-bit integers that can overflow.

But that's not the fault of the algorithm, or the fault of its proof and it
also has nothing to do with the origin of your computer parts.

Also, the "mechanics" of binary search work perfectly in practice. I see no
evidence to the contrary, either in your comment or in the above article and
that same implementation described works perfectly in Python.

~~~
arundelo
Upvoted for the first two paragraphs, but I can't make sense of the second
sentence of the third paragraph. The algorithm given in the article works in
Python because Python has different overflow behavior than Java, C, or C++.
When you say you "see no evidence", it seems like you are saying the
implementation given in the article _does_ work in Java etc.

~~~
nknight
Technically, the naïve implementation is perfectly fine for C/C++, so long as
size_t is at least 268 bits long on your platform. :)

------
ColinWright
Previous, extensive discussion: <http://news.ycombinator.com/item?id=1130463>

I once ran a programming challenge based on this, but I discontinued when I
started to get threats of physical violence from people whose code didn't pass
the tests.

The Dictionary of Algorithms and Data Structures on the NIST site has a
correction I submitted on exactly this point:

<http://xlinux.nist.gov/dads/HTML/binarySearch.html>

And my blog post:
[http://www.solipsys.co.uk/new/BinarySearchReconsidered.html?...](http://www.solipsys.co.uk/new/BinarySearchReconsidered.html?HN)

------
Wilduck
An interesting article that links back to this one:

[http://reprog.wordpress.com/2010/04/19/are-you-one-of-
the-10...](http://reprog.wordpress.com/2010/04/19/are-you-one-of-
the-10-percent/)

The claim is that even ignoring overflow, only 10% of programmers can
correctly implement a binary search. When I tried it, I thought I got it
working, but it was later pointed out that I didn't handle empty lists
correctly.

Programming correctly is hard.

~~~
finnw
I hope that means "10% get it right first time" and not "only 10% can do it at
all, even given a computer and a whole day to do it."

Couldn't resist that challenge (although the thread is nearly 2 years old.) I
think mine works, at least it gives the right answer for all my test cases.

~~~
mrich
It's even hard to come up with all the relevant test cases - if you miss a
corner case in the implementation, it is likely you will oversee it also in
the test case generation. Only an exhaustive search would somehow be able to
discover this.

With e.g. templates in C++, this is possible in some cases. You can test a
template version using the limited range of an unsigned char, while your real
implementation will use a uint64.

But instead of implementing e.g. a binary search, I prefer to take a proven
implementation from e.g. the STL and adapt it for my needs. Experience and
programmer lazyness have taught me this is a good way :)

------
lutorm
While true, it seems that if your array is _that_ close to the inherent size
limit in indexing by ints, this is at most a temporary fix. How fast does your
sorting requirements go from 2^31 to 2^32? It seems that you should be using
size_t here, not int.

~~~
loeg
size_t is actually 32-bit on some 64-bit platforms; you want uint64_t.

~~~
kragen
If that's true, that's a bug in the platform, not the program. The whole point
of size_t is to be however many bits you need to represent the size of things
in your address space.

------
jbarham
Nice to see that the Go gets it right:

    
    
            func Search(n int, f func(int) bool) int {
                    // Define f(-1) == false and f(n) == true.
                    // Invariant: f(i-1) == false, f(j) == true.
                    i, j := 0, n
                    for i < j {
                            h := i + (j-i)/2 // avoid overflow when computing h
                            // i ≤ h < j
                            if !f(h) {
                                    i = h + 1 // preserves f(i-1) == false
                            } else {
                                    j = h // preserves f(j) == true
                            }
                    }
                    // i == j, f(i-1) == false, and f(j) (= f(i)) == true  =>  answer is i.
                    return i
            }
    

[http://code.google.com/p/go/source/browse/src/pkg/sort/searc...](http://code.google.com/p/go/source/browse/src/pkg/sort/search.go?name=weekly.2012-01-27#57)

~~~
nknight
I expect at least one of the Go authors was well aware of this very incident
(the Java breakage), it was fairly well publicized before Go ever saw the
light of day.

------
pasbesoin
Bypass the borked formatting (original is JS dependent; link below is to
Google text-only cache):

[http://webcache.googleusercontent.com/search?strip=1&q=c...](http://webcache.googleusercontent.com/search?strip=1&q=cache:http%3A%2F%2Fgoogleresearch.blogspot.com%2F2006%2F06%2Fextra-
extra-read-all-about-it-nearly.html)

IIRC, this made the rounds a few years ago, in case it sounds familiar.

~~~
justincormack
Thanks. The site is so broken on the ipad, asks if you want a static view
which goes to the front page, or an "unsupported" dynamic view which actually
turns out to work and is the same page. Google this is so broken! Just make
html that works in a browser!

------
ChuckMcM
I'm tempted to solve it with:

    
    
      low/2 + high/2 + (low & high & 1);
    

That side steps the overflow condition completely.

~~~
redthrowaway
It would sidestep the overflow, but _prima facie_ it appears to be slower than
the provided:

    
    
        low + (high - low)/2;
    

Your solution takes, depending on the architecture, between several more and
nearly double the trips to the ALU.

Gimmie a sec; I've got bugger all to do at work. I'll compile it and profile
it.

~~~
ChuckMcM
Would be interested in seeing the profile. Re-ordering it to the following
(which the compiler might do too)

    
    
      low&high&1 + low>>1 + high>>1
    

Should compute in a single cycle if the register coloring is working in the
pipeline. The <reg> >> 1 come out of the barrel shifter stage, the low&high&1
resolves in the load, so you end up with a single sum of three operands. Since
its being stored in a separate register that would avoid a write stall in the
pipeline as well.

------
pantaloons
I'm tempted to suggest that the fixed C/C++ version is still not correct with
respect to this issue, namely on architectures where INT_MAX == UINT_MAX. (Or
really INT_MAX <= UINT_MAX < 2 * INT_MAX, which is valid in C99 for INT_MAX >=
65535.)

~~~
caf
INT_MAX == UINT_MAX is the only valid failing case, because elsewhere in the
standard integer types are restricted to pure binary representations. This
means that if UINT_MAX is greater than INT_MAX, it must be at least INT_MAX *
2 + 1.

~~~
pantaloons
That can't actually be taken for granted, relevant Usenet discussion begins
here:

[http://groups.google.com/group/comp.std.c/browse_thread/thre...](http://groups.google.com/group/comp.std.c/browse_thread/thread/6f1c599324bb2c2e/cd31b3e5003c45f9)

~~~
caf
I concur with Dan Pop's interpretation.

------
yason
I don't like the index variables and splitting in half part. I like to write
my binary searches with bit patterns:

1) figure out the number of bits required to cover the largest index (= a
little bit of cheap bit-twiddling)

3) initialize your index to 0

2) flip a bit on in index, starting from the highest bit available from step
1)

3) if data[index] is smaller than what you're looking for, leave the bit on

4) try with the next-lowest bit and loop to 2) until at bit #0

It's pretty easy to write it out in a way that's just obvious, instead of
requiring the reader to wrap his mind about which variable is the lowest and
highest bound and whether the indexes are inclusive/exclusive in which ends,
etc.

~~~
ColinWright
That's really cool - would you care to post some example code so we can see
the clarity obtained by this novel approach?

Did you come up with it yourself, or did you see it somewhere else?

~~~
gjm11
I think one of Jon Bentley's "Programming pearls" books contains a version of
binary search that works more or less that way and shows how to get there from
a naive and more obviously correct implementation by successive small
transformations. IIRC it ends up having a particularly tight inner loop. And
it doesn't, I think, require a power-of-2-sized array. My copy of the book is
at home and I'm at work so I can't check any of my recollections right now.

~~~
ColinWright
You're right - Programming Pearls, column 8 "Code Tuning." It's not quite like
this, and it's in some variant of BASIC, but when you squint, it's there.

Thanks.

~~~
gjm11
You're welcome. In the second edition, which is what I happen to have, it's
column 9 (because the old column 4 got split into two) and the code is in a
pseudocode that's much more like C with the semicolons deleted than like any
version of BASIC.

I wasn't quite right to say that the inner loop is very tight; rather, the
code assumes a particular size of array and unrolls the loop completely. But,
indeed, the size doesn't need to be a power of 2.

Here's the (pseudo)code, in case anyone cares. It's for an array of size 1000.
I've changed a variable name from "l" to "m" because of the usual l1I| thing.
I've removed a couple of helpful comments because anyone who cares enough to
bother reading them will probably have more fun figuring everything out on
their own. I've also elided some obvious repetitive code and formatted it
slightly differently from Bentley. Any errors in the code were probably
introduced by me.

    
    
      m = -1
      if (x[  511] < t) m = 1000-512
      if (x[m+256] < t) m += 256
      if (x[m+128] < t) m += 128
      /* ... */
      if (x[m+  2] < t) m +=   2
      if (x[m+  1] < t) m +=   1
      p = m+1
      if (p>1000 || x[p]!=t) p = -1 /* i.e., not found */
    

Bentley says this is "not for the faint of heart". I agree, but I do think
it's lovely. Though personally I'd be inclined to use a value of m offset by 1
from what Bentley does (initialize to 0, look up m+255, m+127, etc.) and save
a line of code and a couple of cycles.

------
tbrownaw
This bug requires either (1) low/mid/high are pointers [edit - I don't think
this is valid; IIRC pointer arithmetic is only sane for add/subtract an int
from a pointer, not divide a pointer or add two pointers]; or (2) item_count >
INT_MAX / 2, which means memory_size >= sizeof(your_array) == sizeof(item) *
item_count > INT_MAX / 2, which means either 1-byte or 2-byte array elements,
or that your ints are smaller than your pointers. So is it really "nearly
all"?

~~~
anorwell
The author is claiming that nearly all binary search implementations are wrong
under those specific conditions, not that binary search implementations are
wrong under nearly all conditions.

~~~
tbrownaw
...bah. I keep forgetting that 64-bit C/C++ compilers are generally LP64 (or
even LLP64) instead of ILP64, so "int" being possibly too small is in fact
typical.

Still. In the example (Java) implicit down-converting isn't allowed, so this
is a result of the spec putting arbitrary limits on array size (must be
indexed by nonnegative 'int' values). In C or C++ there is no such limit and I
think more modern compilers (llvm) will give a warning on loss of precision,
so either you have that warning ignored/disabled/unavailable or you have
32-bit code with a billion-element array. I guess what I'm thinking with this
is, with a sane language/compiler, the only way to trigger this should be to
fill half of your theoretically-logically-addressable memory with an array
with single-byte elements.

------
MarkSweep
I don't understand why I need Javscript enabled to merely read a blog post on
blogspots.

~~~
sesqu
To be fair, there's a classic version (but the link to it discards the article
id, for some unfathomable reason).

This works: [http://googleresearch.blogspot.com/2006/06/extra-extra-
read-...](http://googleresearch.blogspot.com/2006/06/extra-extra-read-all-
about-it-nearly.html?v=0)

------
jakubw
_The general lesson that I take away from this bug is humility: It is hard to
write even the smallest piece of code correctly, and our whole world runs on
big, complex pieces of code._

It surely does if everyone has to write their own binary search implementation
when most standard libraries have one and several of them have one that is
flexible enough (either due to the language's features or the binary search
routine itself) to not just search through a container but through a solution
space in an optimization problem. So I'm surprised the author hasn't concluded
with the lesson that would be somewhat more practical: _It is hard to write
even the smallest piece of code correctly_ , so don't do it and use whatever's
in your stack unless you have a good reason not to.

------
VMG
Why are overflows silent?

Shouldn't use cases where you want the overflow behavior be more explicit?

------
tonyg
So C, C++, and Java don't have good support for arithmetic. What else is new?

~~~
alexscheelmeyer
+1, this has nothing to do with binary search and everything to do with the
fact that almost any application written in those languages will have bugs
that do not show until numbers nearing INT_MAX are used. Another given is that
all applications that use recursion is bound to run into stack overflow
errors/bugs when used on big data.

------
finnw
Almost the same bug: [http://code.google.com/p/guava-
libraries/issues/detail?id=61...](http://code.google.com/p/guava-
libraries/issues/detail?id=616)

------
willvarfar
an old story but still well worth the re-reading

