| From 9b987f8c98763ee569bde90b5268b43474ca106c Mon Sep 17 00:00:00 2001 |
| From: Christopher Swenson <chris@caswenson.com> |
| Date: Fri, 27 Feb 2015 14:55:49 +0800 |
| Subject: [PATCH] Fix timsort invariant loop re: Envisage article |
| |
| See http://envisage-project.eu/proving-android-java-and-python-sorting-algorithm-is-broken-and-how-to-fix-it/ |
| |
| We use a "runLen" array of size 128, so it should be nearly impossible |
| to have our implementation overflow. |
| |
| But in any case, the fix is relatively simple -- checking two extra |
| conditions in the invariant calculation. |
| |
| I also took this opportunity to remove some redundancy in the |
| left/right merge logic in the invariant loop. |
| --- |
| timsort.h | 74 +++++++++++++++++++++++++++++++++------------------------------ |
| 1 file changed, 39 insertions(+), 35 deletions(-) |
| |
| diff --git a/timsort.h b/timsort.h |
| index efa3aab..795f272 100644 |
| --- a/timsort.h |
| +++ b/timsort.h |
| @@ -392,62 +392,66 @@ static void TIM_SORT_MERGE(SORT_TYPE *dst, const TIM_SORT_RUN_T *stack, const in |
| |
| static int TIM_SORT_COLLAPSE(SORT_TYPE *dst, TIM_SORT_RUN_T *stack, int stack_curr, TEMP_STORAGE_T *store, const size_t size) |
| { |
| - while (1) |
| - { |
| - int64_t A, B, C; |
| + while (1) { |
| + int64_t A, B, C, D; |
| + int ABC, BCD, BD, CD; |
| + |
| /* if the stack only has one thing on it, we are done with the collapse */ |
| - if (stack_curr <= 1) break; |
| + if (stack_curr <= 1) { |
| + break; |
| + } |
| + |
| /* if this is the last merge, just do it */ |
| - if ((stack_curr == 2) && |
| - (stack[0].length + stack[1].length == (int64_t) size)) |
| - { |
| + if ((stack_curr == 2) && (stack[0].length + stack[1].length == size)) { |
| TIM_SORT_MERGE(dst, stack, stack_curr, store); |
| stack[0].length += stack[1].length; |
| stack_curr--; |
| break; |
| } |
| /* check if the invariant is off for a stack of 2 elements */ |
| - else if ((stack_curr == 2) && (stack[0].length <= stack[1].length)) |
| - { |
| + else if ((stack_curr == 2) && (stack[0].length <= stack[1].length)) { |
| TIM_SORT_MERGE(dst, stack, stack_curr, store); |
| stack[0].length += stack[1].length; |
| stack_curr--; |
| break; |
| - } |
| - else if (stack_curr == 2) |
| + } else if (stack_curr == 2) { |
| break; |
| + } |
| |
| - A = stack[stack_curr - 3].length; |
| - B = stack[stack_curr - 2].length; |
| - C = stack[stack_curr - 1].length; |
| + B = stack[stack_curr - 3].length; |
| + C = stack[stack_curr - 2].length; |
| + D = stack[stack_curr - 1].length; |
| |
| - /* check first invariant */ |
| - if (A <= B + C) |
| - { |
| - if (A < C) |
| - { |
| - TIM_SORT_MERGE(dst, stack, stack_curr - 1, store); |
| - stack[stack_curr - 3].length += stack[stack_curr - 2].length; |
| - stack[stack_curr - 2] = stack[stack_curr - 1]; |
| - stack_curr--; |
| - } |
| - else |
| - { |
| - TIM_SORT_MERGE(dst, stack, stack_curr, store); |
| - stack[stack_curr - 2].length += stack[stack_curr - 1].length; |
| - stack_curr--; |
| - } |
| + if (stack_curr >= 4) { |
| + A = stack[stack_curr - 4].length; |
| + ABC = (A <= B + C); |
| + } else { |
| + ABC = 0; |
| } |
| - /* check second invariant */ |
| - else if (B <= C) |
| - { |
| + |
| + BCD = (B <= C + D) || ABC; |
| + CD = (C <= D); |
| + BD = (B < D); |
| + |
| + /* Both invariants are good */ |
| + if (!BCD && !CD) { |
| + break; |
| + } |
| + |
| + /* left merge */ |
| + if (BCD && !CD) { |
| + TIM_SORT_MERGE(dst, stack, stack_curr - 1, store); |
| + stack[stack_curr - 3].length += stack[stack_curr - 2].length; |
| + stack[stack_curr - 2] = stack[stack_curr - 1]; |
| + stack_curr--; |
| + } else { |
| + /* right merge */ |
| TIM_SORT_MERGE(dst, stack, stack_curr, store); |
| stack[stack_curr - 2].length += stack[stack_curr - 1].length; |
| stack_curr--; |
| } |
| - else |
| - break; |
| } |
| + |
| return stack_curr; |
| } |
| |
| -- |
| 2.3.5 |
| |