Add input validation to IndexBinaryHNSW search preconditions (#4979)

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

These checks guard against invalid runtime state that would otherwise
cause undefined behavior or confusing errors during search operations.
They are not redundant with deserialization-time checks — they protect
against programmatic misuse at query time.

Specific validations and their consequences if absent:

1. get_distance_computer: Verify storage is IndexBinaryFlat via
   dynamic_cast. Without this, a nullptr or wrong-type storage would
   pass a garbage pointer to dispatch_HammingComputer, leading to
   memory corruption. Deserialization already validates storage type,
   but this also guards against programmatic construction with the
   wrong storage type. Converted from FAISS_ASSERT (which aborts or
   is compiled out) to FAISS_THROW_IF_NOT_MSG for safe error
   propagation.

2. ntotal > 0 in base_level_only search: The entrypoint selection loop
   constructs std::uniform_int_distribution<idx_t>(0, ntotal - 1).
   When ntotal == 0, this becomes distribution(0, -1) — undefined
   behavior per the C++ standard ([rand.dist.uni.int]). An empty
   index is valid state that can arise after deserialization or
   construction, but searching it in base_level_only mode must be
   rejected.

3. num_base_level_search_entrypoints > 0: This is a public, user-
   settable parameter (default 32). If set to 0, the entrypoint
   selection loop at line 388 never executes, nearest[i] stays at -1,
   and the downstream FAISS_THROW at line 397 fires with the
   misleading message "Could not find a valid entrypoint" — obscuring
   the actual misconfiguration. The early check provides an actionable
   error message naming the offending parameter.

Reviewed By: mnorris11

Differential Revision: D97845054

fbshipit-source-id: d57d364889ce6a6bcfda741d6541c4cdbac44e85
This commit is contained in:
Justin Gibbs
2026-03-26 12:17:04 -07:00
committed by meta-codesync[bot]
parent d26c15d954
commit ba1981ca4c
2 changed files with 90 additions and 1 deletions
+10 -1
View File
@@ -319,7 +319,9 @@ struct BuildDistanceComputer {
DistanceComputer* IndexBinaryHNSW::get_distance_computer() const {
IndexBinaryFlat* flat_storage = dynamic_cast<IndexBinaryFlat*>(storage);
FAISS_ASSERT(flat_storage != nullptr);
FAISS_THROW_IF_NOT_MSG(
flat_storage != nullptr,
"IndexBinaryHNSW requires IndexBinaryFlat storage");
BuildDistanceComputer bd;
return dispatch_HammingComputer(code_size, bd, flat_storage);
}
@@ -356,6 +358,13 @@ void IndexBinaryHNSWCagra::search(
if (!base_level_only) {
IndexBinaryHNSW::search(n, x, k, distances, labels, params);
} else {
FAISS_THROW_IF_NOT_MSG(
ntotal > 0, "IndexBinaryHNSWCagra: cannot search empty index");
FAISS_THROW_IF_NOT_MSG(
num_base_level_search_entrypoints > 0,
"IndexBinaryHNSWCagra: "
"num_base_level_search_entrypoints must be > 0");
float* distances_f = (float*)distances;
using RH = HeapBlockResultHandler<HNSW::C>;
+80
View File
@@ -14,6 +14,7 @@
#include <faiss/Index.h>
#include <faiss/IndexBinary.h>
#include <faiss/IndexBinaryHNSW.h>
#include <faiss/IndexFlat.h>
#include <faiss/IndexHNSW.h>
#include <faiss/IndexIVFFlat.h>
@@ -1844,3 +1845,82 @@ TEST(ReadIndexDeserialize, BinaryMultiHashMapIdOutOfRange) {
expect_binary_read_throws_with(buf, "multi hash map: id=");
}
// ---- IndexBinaryHNSW runtime safety checks ----
TEST(ReadIndexDeserialize, BinaryHNSWGetDistanceComputerNonFlatThrows) {
// Construct an IndexBinaryHNSW with non-flat storage and verify
// get_distance_computer throws instead of asserting.
IndexBinaryHNSW idx;
idx.storage = nullptr;
idx.own_fields = false;
// storage is nullptr so dynamic_cast will fail
EXPECT_THROW(
{
try {
idx.get_distance_computer();
} catch (const faiss::FaissException& e) {
EXPECT_NE(
std::string(e.what()).find("IndexBinaryFlat"),
std::string::npos);
throw;
}
},
faiss::FaissException);
}
TEST(ReadIndexDeserialize, BinaryHNSWCagraEmptyIndexSearch) {
IndexBinaryHNSWCagra idx(16, 4);
idx.base_level_only = true;
// ntotal is 0, searching should throw
std::vector<int32_t> distances(1);
std::vector<idx_t> labels(1);
std::vector<uint8_t> query(2); // d=16 -> code_size=2
EXPECT_THROW(
{
try {
idx.search(
1,
query.data(),
1,
distances.data(),
labels.data());
} catch (const faiss::FaissException& e) {
EXPECT_NE(
std::string(e.what()).find("empty index"),
std::string::npos);
throw;
}
},
faiss::FaissException);
}
TEST(ReadIndexDeserialize, BinaryHNSWCagraZeroEntrypoints) {
IndexBinaryHNSWCagra idx(16, 4);
// Add a vector first (add requires base_level_only == false)
std::vector<uint8_t> vec(2, 0xFF);
idx.add(1, vec.data());
idx.base_level_only = true;
idx.num_base_level_search_entrypoints = 0;
std::vector<int32_t> distances(1);
std::vector<idx_t> labels(1);
std::vector<uint8_t> query(2);
EXPECT_THROW(
{
try {
idx.search(
1,
query.data(),
1,
distances.data(),
labels.data());
} catch (const faiss::FaissException& e) {
EXPECT_NE(
std::string(e.what()).find(
"num_base_level_search_entrypoints"),
std::string::npos);
throw;
}
},
faiss::FaissException);
}