Live data from Hacker News

Nearly All Binary Searches and Mergesorts Are Broken (2006)

research.googleblog.com

71–80 of 95 posts

Re: Nearly All Binary Searches and Mergesorts Are Broken (2006)

#71

Earlier quoted context omitted.

Simply changing to size_t doesn't really fix this bug. You still have to use the low + (high - low) / 2 fix.

If "this bug" is the ability to sort >2^30 elements on a 64-bit machine, this bug IS addressed by changing the index type. Of course sorting 2^63 elements would require the different calculation.

Pointers are 64-bit on 64-bit machines, and so are the largest unsigned ints, hence the problem. On the other hand, you're probably searching at least 4-byte objects, so your actual list length isn't 64 bits but at most 62 bits even if you filled the memory space.

Re: Nearly All Binary Searches and Mergesorts Are Broken (2006)

#72
post #2

That line from the C/C++ "fix" is an atrocity; `low`, `mid`, and `high` should never have been declared as signed integers in the first place, since array indices are never negative. It's unfortunate that in Java there is no other option than to use signed ints.

Even with unsigned int (or even size_t), the C/C++ code still doesn't sit well with me. The addition can still overflow, and while unsigned overflow is well-defined, the result here is still nonsense. (i.e., while the result of (low + high) >> 1 during overflow will be well defined, it won't be the midpoint…) You might argue that you're never going to overflow a size_t on a 64-bit, maybe, but given that the correct c…

With size_t, it would never overflow, even on 32 bits.

The input is an array of int, ints are 4 bytes, 32 bit systems can only address 2^32 bytes, so the array is no more than 2^30 elements long, so in the worst case, low+high equals 2^31-3, less than even a signed it.

It could overflow if we pass it a char* instead but if you have a >2GB array of sorted single bytes, you probably have a problem somewhere else...

Re: Nearly All Binary Searches and Mergesorts Are Broken (2006)

#73

I wish it was worth a lot to be good at systems programming these days. It seems like the huge salaries are for rails senior devs. I spent like ten years getting really good at this stuff (the (low + high)/2 line jumped right out at me) but nowadays it feels like being really good at trivial pursuit. It's interesting how much things have changed in the last decade. I wonder what next decade's "Rails" will be? Could i…

The problem is these sorts of things are trivia. They are things that sufficiently smart tooling should handle for us so people can spend time building higher level constructs and less time worrying about individual bits. A smart compiler should have caught (a + b) / 2 and fixed it to be correct, there's no way the overflow situation is what the programmer wanted.

I'd accept a warning, but a C compiler shouldn't be making guesses at what the programmer intended. In another language? Sure, if it's consistent with other behaviors in that language.

Re: Nearly All Binary Searches and Mergesorts Are Broken (2006)

#74

Earlier quoted context omitted.

Agreed completely; the right fix is that anything indexing an array should be an unsigned size_t (or equivalent for your language of choice).

Simply changing to size_t doesn't really fix this bug. You still have to use the low + (high - low) / 2 fix.

It doesn't, by itself, fix the bug identified in the article (though it does avoid a memory safety issue). But it's still important. Indexes should never be signed, any more than pointers should.

Re: Nearly All Binary Searches and Mergesorts Are Broken (2006)

#75
post #2

That line from the C/C++ "fix" is an atrocity; `low`, `mid`, and `high` should never have been declared as signed integers in the first place, since array indices are never negative. It's unfortunate that in Java there is no other option than to use signed ints.

For C99, the correct type for an array index is size_t. unsigned int isn't guaranteed to be big enough, while size_t is guaranteed to be large enough for the target architecture.

Here's some code you can compile, it shows the problem:

  #include 
  #include 
  
  int main() {
    printf("size_t bytes: %u\n", sizeof(size_t));
    
    size_t high = SIZE_MAX;
    size_t low = high-1;
    size_t mid_correct = low+(high-low)/2;
    size_t mid_incorrect = (low+high)/2;
    
    printf("low: %.ju\n", low);
    printf("high: %.ju\n", high);
    printf("low+high: %.ju\n", low+high);
    printf("(low+high)/2 -- incorrect: %.ju\n", mid_incorrect);
    printf("low+(high-low)/2 -- correct: %.ju\n", mid_correct);
  }
On my 64-bit machine, I get:

  size_t bytes: 8
  low: 18446744073709551614
  high: 18446744073709551615
  low+high: 18446744073709551613
  (low+high)/2 -- incorrect: 9223372036854775806
  low+(high-low)/2 -- correct: 18446744073709551614

Re: Nearly All Binary Searches and Mergesorts Are Broken (2006)

#76
post #62

Earlier quoted context omitted.

All of those places will pay just as much for (as well as hire far more of) enterprise Java/Go/HTML/JS code monkeys There's nothing wrong with that, but picking 5 companies with their hands in damn-near anything is disingenuous.

Is your goal to be paid extremely well for doing systems programming relative to other people in the same company? Or to be paid extremely well for doing systems programming?

The latter: solid salary for sys

Re: Nearly All Binary Searches and Mergesorts Are Broken (2006)

#77
post #32

I'm surprised about the suggested solution for java: int mid = (low + high) >>> 1; I suppose things like this is why the ">>>" (unsigned shift) operator exists - but it's a bit odd when the value it works on is considered signed by the language, and implemented as two-compliment signed in memory. What's interesting to me is that this allows the sum to overflow, and would fail with the ">>" operator as far as I can te…

I agree it would be very surprising to see that line in a java codebase. >>> seems more like an answer to a java trivia question than something you'd come across on a regular basis.

Thinking a bit more about this, I think what feels off about it to me, is the invisible "type gymnastics" - it's something that might be a bad idea, but feel more natural, in assembly (that byte is what you decide it represents at any given moment). The individual numbers are signed ints, the sum overflows and is treated as an "unsigned two's complement binary" and shifted down to a signed int...

It feels like subtle subversion of java's admittedly strange type system for numbers (mix of raw integer types and boxed numbers is never going to be pretty...).

Re: Nearly All Binary Searches and Mergesorts Are Broken (2006)

#78
post #2

That line from the C/C++ "fix" is an atrocity; `low`, `mid`, and `high` should never have been declared as signed integers in the first place, since array indices are never negative. It's unfortunate that in Java there is no other option than to use signed ints.

For C99, the correct type for an array index is size_t. unsigned int isn't guaranteed to be big enough, while size_t is guaranteed to be large enough for the target architecture.

The range of size_t only comes into play if the array can potentially be huge. If the array is small, there may be other considerations. Suppose you have a displaced array with negative indices: p[-2], p[-1], p[0], p[1], ... size_t is good for nothing here.

The unsigned types are generally awful; they have a big discontinuity right at zero, so you have to be careful with arithmetic even if it involves small values.

If I'm working with int and I know that all quantities fit into two decimal digits, then there isn't possibly any problem. Not so with unsigned int or size_t or what have you.

If a, b, c are small integers in a signed type then I know I can rewrite a as a - c , following straight algebraic rules of derivation. Not so if these are unsigned; moving c from the right side to the left may break the inequality because a may be smaller than c.

Re: Nearly All Binary Searches and Mergesorts Are Broken (2006)

#79
post #30

> int mid = low + ((high - low) / 2); That may fix things in the case of searching a sorted array, but binary search can be used more generally than that. I think that fix might not work for some of the more general applications of binary search. For instance, suppose f(n) is an increasing function from the signed integers to the signed integers, with f(a) 0, and you want to find an n in (a,b), if such n exists, such…

Bullet proof:

    mid = lo / 2 + hi / 2;
Well, that is, if your algorithm can tolerate a mid == 6, for hi == 7 and lo == 7. :) :) :)

Cough, cough; seriously though: here is a variant based on the above idea which takes care of the remainders, avoiding that problem:

   ;; TXR Lisp
   (defun mid (lo hi)
     (tree-bind ((lq lr) . (hq hr)) (cons (trunc-rem lo 2) (trunc-rem hi 2))
       (+ lq hq (trunc (+ lr hr) 2))))
trunc-rem has toward-zero truncation, with a remainder that is harmonized to that.

Based on my testing in the REPL, this is behaving sensibly.

Adding the quotients, and then adding to them the sum of the remainders, truncated by two, seems to be doing the trick.

(mid 7 7) is 7, (mid -7 -7) is -7 and various other cases are all sensible. (mid k (+ 2 k)) is yielding (+ 1 k), for both values being negative, either being zero, and zero-crossing cases. (mid k k) seems to be k for all k.

I think as of ISO C99, the / and % operators truncate toward zero. Unless I'm gravely mistaken, this is then nicely expressible in C as:

  lo / 2 + hi / 2 + ((lo % 2) + (hi % 2)) / 2;
It's mathematically well founded: we have added the truncations, and then continue working with the remainders. This is because the following expression in fact expresses the exact result.

  trunc(lo,2) + trunc(hi,2) + (rem(lo, 2) + rem(hi, 2))/2
where / is exact rational division! Dividing the remainders by two just continues the inexact division, completing it to exactness. What we're doing differently in the machine calculation is using truncating division on the sum of the remainders, which we need because mid is to be an integer result.

Re: Nearly All Binary Searches and Mergesorts Are Broken (2006)

#80
post #42

Earlier quoted context omitted.

Huh, is this the stuff you're talking about: http://www.diku.dk/hjemmesider/ansatte/henglein/papers/henglein2011a.pdf http://www.diku.dk/hjemmesider/ansatte/henglein/papers/henglein2011c.pdf https://www.youtube.com/watch?v=sz9ZlZIRDAg https://hackage.haskell.org/package/discrimination I didn't know about this until I read your reply and googled; I thought you were just obliquely referring to radix sort or something l…

Yes that is it. And it's really a variation on radix sort as well. They're all in the same general family. I can't believe I got down voted for this thread. What does it take? I guess I should just post it.

[deleted]
Post reply on HN