Fix clone_index null return for IndexRowwiseMinMax (#5220)

Summary:
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5220

## TL;DR

`clone_index` returns `nullptr` for any `IndexRowwiseMinMax` or `IndexRowwiseMinMaxFP16` instance — a silent failure that leaks the allocated clone and its deep-copied inner index. We add the missing `return res;` and a regression test covering both subtypes.

## Bug

`clone_Index` in `clone_index.cpp` is a dispatch table of `else if` branches, one per index type. Every branch ends with `return res;` except one: the `IndexRowwiseMinMaxBase` branch allocates `res`, sets `own_fields = true`, deep-copies the inner index into `res->index`, and then falls through to the bottom of the function. The fallthrough hits `return nullptr;`.

The result: callers receive `nullptr` with no exception or warning, and the `res` pointer plus its inner clone are leaked.

## Impact

`clone_index` has 15+ callsites across the faiss codebase and is part of the public C++ and Python API. None of the callers we surveyed guard against `nullptr` — the contract is "clone returns a usable index or throws." With this bug:

- Python users see `clone_index(codec)` return `None`. The next attribute access on the result raises `AttributeError`, often deep in unrelated code where the connection to the clone call is not obvious.
- C++ users dereference the null pointer on the next method call, producing a segfault with no stack frame pointing at `clone_index`.
- Memory leaks: every failed clone leaks `res` (the `IndexRowwiseMinMax` shell) and `res->index` (the inner `IndexScalarQuantizer` deep copy).

The bug affects any pipeline that wraps a codec with `MinMax,*` or `MinMaxFP16,*` factory strings and then clones it — common when cloning a trained codec for parallel encoding, snapshot persistence, or sharded deployment.

## Fix

One line added:

```
} else if (
        const IndexRowwiseMinMaxBase* irmmb =
                dynamic_cast<const IndexRowwiseMinMaxBase*>(index)) {
    IndexRowwiseMinMaxBase* res = clone_IndexRowwiseMinMax(irmmb);
    res->own_fields = true;
    res->index = clone_Index(irmmb->index);
    return res;  // added
}
```

The fix restores symmetry with every other branch in the function. No new abstractions, no related cleanup — the missing return is purely an oversight from when the branch was added.

The `clone_IndexRowwiseMinMax` helper dispatches via `TRYCLONE` to both `IndexRowwiseMinMaxFP16` and `IndexRowwiseMinMax`, so both concrete subtypes are restored by this one line.

Reviewed By: mnorris11

Differential Revision: D105157714

fbshipit-source-id: ed29db91b51716133ef8ceb426bc92760a222fef
This commit is contained in:
Alibek Zhakubayev
2026-05-18 14:35:48 -07:00
committed by meta-codesync[bot]
parent ff1d543977
commit 8d3cc92536
2 changed files with 20 additions and 0 deletions
+1
View File
@@ -377,6 +377,7 @@ Index* Cloner::clone_Index(const Index* index) {
IndexRowwiseMinMaxBase* res = clone_IndexRowwiseMinMax(irmmb);
res->own_fields = true;
res->index = clone_Index(irmmb->index);
return res;
} else if (
dynamic_cast<const IndexAdditiveQuantizerFastScan*>(index) ||
dynamic_cast<const IndexAdditiveQuantizer*>(index) ||
+19
View File
@@ -86,3 +86,22 @@ class TestClone(unittest.TestCase):
self.do_test_clone("RQ3x4fs_32_Nlsq2x4")
self.do_test_clone("PLSQ2x3x4fs_Nlsq2x4")
self.do_test_clone("PRQ2x3x4fs_Nrq2x4")
def test_RowwiseMinMax(self):
# clone_Index for IndexRowwiseMinMaxBase was missing return res, so
# clone_index returned nullptr and leaked the clone allocation plus
# the deep-copied inner index.
d = 32
n = 200
rng = np.random.default_rng(42)
x = rng.standard_normal((n, d)).astype(np.float32)
for factory in ("MinMax,SQ8", "MinMaxFP16,SQ8"):
codec = faiss.index_factory(d, factory)
codec.train(x)
codes = codec.sa_encode(x)
codec2 = faiss.clone_index(codec)
self.assertEqual(type(codec), type(codec2))
codes2 = codec2.sa_encode(x)
np.testing.assert_array_equal(codes, codes2)