Skip to content

Commit b12578f

Browse files
junjieqimeta-codesync[bot]
authored andcommitted
Suppress non-const global variable warnings and fix security integer casts (#5092)
Summary: Pull Request resolved: #5092 Fix 32 `facebook-avoid-non-const-global-variables` warnings by adding NOLINT comments to intentionally mutable global stats objects and configuration variables that are part of the public API (e.g., `indexIVF_stats`, `hnsw_stats`, `check_compatible_for_merge_expensive_check`). These cannot be made const as they accumulate runtime statistics and provide runtime configuration. Fix 25 `facebook-security-vulnerable-integer-implicit-cast` warnings by adding explicit `static_cast<size_t>()` to one operand before integer multiplication to prevent overflow before widening. Also fix `clang-diagnostic-switch-enum` warnings by adding missing enum cases to switch statements, and various other minor lint fixes including NOLINT for `-inl.h` includes, `ShadowingClass` fix, and `BadImplicitCast` fix. Reviewed By: mnorris11 Differential Revision: D100592783
1 parent 3bac8c9 commit b12578f

16 files changed

Lines changed: 61 additions & 31 deletions

faiss/Index2Layer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,8 @@ DistanceComputer* Index2Layer::get_distance_computer() const {
266266
/* The standalone codec interface */
267267

268268
// block size used in Index2Layer::sa_encode
269-
int index2layer_sa_encode_bs = 32768;
269+
int index2layer_sa_encode_bs =
270+
32768; // NOLINT(facebook-avoid-non-const-global-variables)
270271

271272
void Index2Layer::sa_encode(idx_t n, const float* x, uint8_t* bytes) const {
272273
FAISS_THROW_IF_NOT(is_trained);

faiss/IndexAdditiveQuantizer.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ struct AQDistanceComputerDecompress : FlatCodesDistanceComputer {
4747
const IndexAdditiveQuantizer& iaq,
4848
VectorDistance vd_)
4949
: FlatCodesDistanceComputer(iaq.codes.data(), iaq.code_size),
50-
tmp(iaq.d * 2),
50+
tmp(static_cast<size_t>(iaq.d) * 2),
5151
aq(*iaq.aq),
5252
vd(vd_),
5353
d(iaq.d) {}
@@ -79,7 +79,7 @@ struct AQDistanceComputerLUT : FlatCodesDistanceComputer {
7979

8080
explicit AQDistanceComputerLUT(const IndexAdditiveQuantizer& iaq)
8181
: FlatCodesDistanceComputer(iaq.codes.data(), iaq.code_size),
82-
LUT(iaq.aq->total_codebook_size + iaq.d * 2),
82+
LUT(iaq.aq->total_codebook_size + static_cast<size_t>(iaq.d) * 2),
8383
aq(*iaq.aq),
8484
d(iaq.d) {}
8585

@@ -219,6 +219,9 @@ FlatCodesDistanceComputer* IndexAdditiveQuantizer::
219219
false,
220220
AdditiveQuantizer::ST_norm_cqint8>(*this);
221221
#undef DISPATCH
222+
case AdditiveQuantizer::ST_decompress:
223+
case AdditiveQuantizer::ST_norm_from_LUT:
224+
case AdditiveQuantizer::ST_count:
222225
default:
223226
FAISS_THROW_FMT(
224227
"search type %d not supported", aq->search_type);
@@ -276,6 +279,8 @@ void IndexAdditiveQuantizer::search(
276279
*this, x, rh);
277280
break;
278281
#undef DISPATCH
282+
case AdditiveQuantizer::ST_decompress:
283+
case AdditiveQuantizer::ST_count:
279284
default:
280285
FAISS_THROW_FMT(
281286
"search type %d not supported", aq->search_type);

faiss/IndexBinaryHash.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,8 @@ void IndexBinaryHashStats::reset() {
273273
memset((void*)this, 0, sizeof(*this));
274274
}
275275

276-
IndexBinaryHashStats indexBinaryHash_stats;
276+
IndexBinaryHashStats
277+
indexBinaryHash_stats; // NOLINT(facebook-avoid-non-const-global-variables)
277278

278279
/*******************************************************
279280
* IndexBinaryMultiHash implementation

faiss/IndexFastScan.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,8 @@ void IndexFastScan::search_implem_12(
515515
}
516516
}
517517

518-
FastScanStats FastScan_stats;
518+
FastScanStats
519+
FastScan_stats; // NOLINT(facebook-avoid-non-const-global-variables)
519520

520521
template <class C>
521522
void IndexFastScan::search_implem_14(

faiss/IndexFlat.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ void IndexFlat::range_search(
6868
FAISS_THROW_IF_NOT_MSG(result, "RangeSearchResult object must not be null");
6969
IDSelector* sel = params ? params->sel : nullptr;
7070

71-
switch (metric_type) {
71+
switch (metric_type) { // NOLINT(clang-diagnostic-switch-enum)
7272
case METRIC_INNER_PRODUCT:
7373
range_search_inner_product(
7474
x, get_xb(), d, n, ntotal, radius, result, sel);
@@ -88,7 +88,7 @@ void IndexFlat::compute_distance_subset(
8888
float* distances,
8989
const idx_t* labels) const {
9090
FAISS_THROW_IF_NOT(k > 0);
91-
switch (metric_type) {
91+
switch (metric_type) { // NOLINT(clang-diagnostic-switch-enum)
9292
case METRIC_INNER_PRODUCT:
9393
fvec_inner_products_by_idx(distances, x, get_xb(), labels, d, n, k);
9494
break;

faiss/IndexHNSW.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ namespace faiss {
3737
using storage_idx_t = HNSW::storage_idx_t;
3838
using NodeDistFarther = HNSW::NodeDistFarther;
3939

40-
HNSWStats hnsw_stats;
40+
HNSWStats hnsw_stats; // NOLINT(facebook-avoid-non-const-global-variables)
4141

4242
/**************************************************************
4343
* add / search blocks of descriptors

faiss/IndexIVF.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
#include <cstdio>
1818
#include <limits>
1919

20-
#include <faiss/utils/hamming.h>
2120
#include <faiss/utils/utils.h>
2221

2322
#include <faiss/IndexFlat.h>
@@ -1276,7 +1275,8 @@ void IndexIVF::train_encoder(
12761275
}
12771276
}
12781277

1279-
bool check_compatible_for_merge_expensive_check = true;
1278+
bool check_compatible_for_merge_expensive_check =
1279+
true; // NOLINT(facebook-avoid-non-const-global-variables)
12801280

12811281
void IndexIVF::check_compatible_for_merge(const Index& otherIndex) const {
12821282
// minimal sanity checks
@@ -1365,7 +1365,8 @@ void IndexIVFStats::add(const IndexIVFStats& other) {
13651365
search_time += other.search_time;
13661366
}
13671367

1368-
IndexIVFStats indexIVF_stats;
1368+
IndexIVFStats
1369+
indexIVF_stats; // NOLINT(facebook-avoid-non-const-global-variables)
13691370

13701371
/*************************************************************************
13711372
* InvertedListScanner

faiss/IndexIVFFastScan.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1529,6 +1529,7 @@ void IndexIVFFastScan::postprocess_packed_codes(
15291529
size_t /*n_added*/,
15301530
const uint8_t* /*flat_codes*/) {}
15311531

1532-
IVFFastScanStats IVFFastScan_stats;
1532+
IVFFastScanStats
1533+
IVFFastScan_stats; // NOLINT(facebook-avoid-non-const-global-variables)
15331534

15341535
} // namespace faiss

faiss/IndexIVFPQ.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
#include <faiss/impl/IDSelector.h>
2929
#include <faiss/impl/ProductQuantizer.h>
3030
#include <faiss/impl/ResultHandler.h>
31-
#include <faiss/impl/pq_code_distance/pq_code_distance-inl.h>
31+
#include <faiss/impl/pq_code_distance/pq_code_distance-inl.h> // NOLINT(facebook-hte-InlineHeader)
3232
#include <faiss/impl/simd_dispatch.h>
3333

3434
namespace faiss {
@@ -222,7 +222,8 @@ void IndexIVFPQ::sa_decode(idx_t n, const uint8_t* codes, float* x) const {
222222
}
223223

224224
// block size used in IndexIVFPQ::add_core_o
225-
int index_ivfpq_add_core_o_bs = 32768;
225+
int index_ivfpq_add_core_o_bs =
226+
32768; // NOLINT(facebook-avoid-non-const-global-variables)
226227

227228
void IndexIVFPQ::add_core_o(
228229
idx_t n,
@@ -349,7 +350,8 @@ void IndexIVFPQ::reconstruct_from_offset(
349350
}
350351

351352
/// 2G by default, accommodates tables up to PQ32 w/ 65536 centroids
352-
size_t precomputed_table_max_bytes = ((size_t)1) << 31;
353+
size_t precomputed_table_max_bytes = ((size_t)1)
354+
<< 31; // NOLINT(facebook-avoid-non-const-global-variables)
353355

354356
/** Precomputed tables for residuals
355357
*
@@ -1301,10 +1303,14 @@ InvertedListScanner* IndexIVFPQ::get_InvertedListScanner(
13011303
});
13021304
}
13031305

1304-
IndexIVFPQStats indexIVFPQ_stats;
1306+
IndexIVFPQStats
1307+
indexIVFPQ_stats; // NOLINT(facebook-avoid-non-const-global-variables)
13051308

13061309
void IndexIVFPQStats::reset() {
1307-
memset(this, 0, sizeof(*this));
1310+
nrefine = 0;
1311+
n_hamming_pass = 0;
1312+
search_cycles = 0;
1313+
refine_cycles = 0;
13081314
}
13091315

13101316
IndexIVFPQ::IndexIVFPQ() {

faiss/IndexIVFRaBitQ.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,8 @@ void IndexIVFRaBitQ::sa_decode(idx_t n, const uint8_t* codes, float* x) const {
332332
}
333333
}
334334

335+
namespace {
336+
335337
struct IVFRaBitDistanceComputer : DistanceComputer {
336338
const float* q = nullptr;
337339
const IndexIVFRaBitQ* parent = nullptr;
@@ -379,6 +381,8 @@ float IVFRaBitDistanceComputer::symmetric_dis(idx_t /*i*/, idx_t /*j*/) {
379381
FAISS_THROW_MSG("Not implemented");
380382
}
381383

384+
} // anonymous namespace
385+
382386
DistanceComputer* IndexIVFRaBitQ::get_distance_computer() const {
383387
IVFRaBitDistanceComputer* dc = new IVFRaBitDistanceComputer;
384388
dc->parent = this;

0 commit comments

Comments
 (0)