facebookresearch / faiss

A library for efficient similarity search and clustering of dense vectors.
https://faiss.ai
MIT License
31.63k stars 3.65k forks source link

Risk of causing bugs due to inconsistency in interfaces of `AlignedTableTightAlloc` #3994

Closed GKxxUCAS closed 4 weeks ago

GKxxUCAS commented 4 weeks ago

In faiss/utils/AlignedTable.h, in the class AlignedTableTightAlloc, we can see from the function resize that the pointer member ptr may be set to null. The copy constructor has taken this into consideration and has performed a check if (numel > 0) before passing ptr to memcpy (which was added in 74ee67a). However, the function clear does not seem to have realized this.

Maybe a check should also be performed in clear before calling memset?

pankajsingh88 commented 4 weeks ago

Thank you submitting the issue!

I've confirmed that calling clear with numel == 0 causes invalid-null-argument error. Can be reproduced via:

  faiss::AlignedTableTightAlloc<int> atta;
  atta.clear();
pankajsingh88 commented 4 weeks ago

https://github.com/facebookresearch/faiss/pull/3997

pankajsingh88 commented 4 weeks ago

closing since the fix https://github.com/facebookresearch/faiss/pull/3997 has landed.