pack-bitmap: reject pseudo-merge "sampleRate" of 0

The "bitmapPseudoMerge.*.sampleRate" configuration controls what
fraction of unstable commits are included in each pseudo-merge group.
The config validation accepts values in the range `[0, 1]`, but a value
of exactly 0 causes a division by zero in `select_pseudo_merges_1()`:

    if (j % (uint32_t)(1.0 / group->sample_rate))

When `sample_rate` is 0, `1.0 / 0.0` produces `+inf`, and casting
infinity to `uint32_t` is undefined behavior in C. On most platforms
this yields 0, making the subsequent modulo operation (`j % 0`) a
fatal arithmetic trap.

This path was not previously reachable because an earlier bug caused
all pseudo-merge candidates to be classified as "stable" (where the
sampling rate is not used), regardless of their actual commit date. Now
that the date classification is fixed, the unstable path is exercised
and the division by zero can fire.

Fix this by changing the validation to require a strict lower bound and
thus reject 0.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/bitmap-pseudo-merge.adoc b/Documentation/config/bitmap-pseudo-merge.adoc
index 1f264eca99b8470aaa2e85f616368df60feee65f..6bf52c80ba786cd35a7f30d4e3d6b3f06166f747 100644 (file)
--- a/Documentation/config/bitmap-pseudo-merge.adoc
+++ b/Documentation/config/bitmap-pseudo-merge.adoc
@@ -47,8 +47,8 @@ will be updated more often than a reference pointing at an old commit.
 bitmapPseudoMerge.<name>.sampleRate::
        Determines the proportion of non-bitmapped commits (among
        reference tips) which are selected for inclusion in an
-       unstable pseudo-merge bitmap. Must be between `0` and `1`
-       (inclusive). The default is `1`.
+       unstable pseudo-merge bitmap. Must be greater than `0` and
+       less than or equal to `1`. The default is `1`.
 
 bitmapPseudoMerge.<name>.threshold::
        Determines the minimum age of non-bitmapped commits (among

diff --git a/pseudo-merge.c b/pseudo-merge.c
index d79e5fb649a8b59df980bba8ac38c20c37b989e4..75bed04360274486b3d90c38a300011b7b700e46 100644 (file)
--- a/pseudo-merge.c
+++ b/pseudo-merge.c
@@ -169,8 +169,8 @@ static int pseudo_merge_config(const char *var, const char *value,
                }
        } else if (!strcmp(key, "samplerate")) {
                group->sample_rate = git_config_double(var, value, ctx->kvi);
-               if (!(0 <= group->sample_rate && group->sample_rate <= 1)) {
-                       warning(_("%s must be between 0 and 1, using default"), var);
+               if (!(0 < group->sample_rate && group->sample_rate <= 1)) {
+                       warning(_("%s must be between 0 (exclusive) and 1, using default"), var);
                        group->sample_rate = DEFAULT_PSEUDO_MERGE_SAMPLE_RATE;
                }
        } else if (!strcmp(key, "threshold")) {

diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 0032a16606bd701366736d50593bf148c033ae2c..5bfbbd4214e1cf74ee9f4dddc185ede0bdc8661e 100755 (executable)
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -639,7 +639,7 @@ test_expect_success 'pseudo-merge commits are correctly classified by date' '
        )
 '
 
-test_expect_failure 'sampleRate=0 does not cause division by zero' '
+test_expect_success 'sampleRate=0 does not cause division by zero' '
        test_when_finished "rm -fr pseudo-merge-sample-rate-zero" &&
        git init pseudo-merge-sample-rate-zero &&
        (