[m-rev.] for review: fix possible integer overflow in binary search
Sebastian Godelet
sebastian.godelet+github at gmail.com
Sun Dec 21 13:34:54 AEDT 2014
In this discussion:
http://www.mercurylang.org/list-archives/reviews/2014-December/017547.html
It was argued that an integer overflow for calculating Mid cannot
happen:
> If you are worried about integer overflow when calculating Mid,
> the standard fix for that is to compute it as Lo + (Hi - Lo) / 2.
> However, since all our ints are signed, there is no need for that;
But in the current Mercury implementation this is not the case,
as demonstrated in this gist:
https://gist.github.com/sebgod/ee1172fd736289aaeaa8
The implementations in the specific grades are taken from this
article:
http://googleresearch.blogspot.de/2006/06/extra-extra-read-all-about-it-nearly.html
But this diff uses the
> the standard fix for that is to compute it as Lo + (Hi - Lo) / 2.
together with the >> 1 optimisation.
Maybe a mid_rounding_down/2 or similar could be included in the int
module.
library/array.m:
library/bt_array.m:
see above.
Sebastian.
--
diff --git a/library/array.m b/library/array.m
index 4beecec..baf978d 100644
--- a/library/array.m
+++ b/library/array.m
@@ -2078,7 +2078,10 @@ array.bsearch_2(Array, Lo, Hi, El, Compare,
Result) :- ;
% Otherwise find the middle element of the range
% and check against that.
- Mid = (Lo + Hi) >> 1, % `>> 1' is hand-optimized `div 2'.
+ % NOTE: Use Lo + (Hi - Lo) >> 1 instead of (Lo + Hi) >> 1
to
+ % prevent an integer overflow.
+
+ Mid = Lo + (Hi - Lo) >> 1, % `>> 1' is hand-optimized
`div 2'. array.lookup(Array, Mid, XMid),
Compare(XMid, El, Comp),
(
diff --git a/library/bt_array.m b/library/bt_array.m
index 315b62e..3fec7a7 100644
--- a/library/bt_array.m
+++ b/library/bt_array.m
@@ -417,8 +417,10 @@ bsearch_2(A, Lo, Hi, El, Compare, I) :-
% little more expensive, and we know that we're always dividing
% by a power of 2. Until such time as we implement strength
reduction, % the >> 1 stays.
+ % NOTE: Use Lo + (Hi - Lo) >> 1 instead of (Lo + Hi) >> 1 to
prevent
+ % an integer overlow.
- Mid = (Lo + Hi) >> 1,
+ Mid = Lo + (Hi - Lo) >> 1,
lookup(A, Mid, XMid),
Compare(XMid, El, Comp),
( Comp = (<),
More information about the reviews
mailing list