Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
GKxxUCAS opened this issue Oct 28, 2024 · 3 comments
Closed

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

GKxxUCAS opened this issue Oct 28, 2024 · 3 comments
Labels
bug Implementation

Comments

@GKxxUCAS
Copy link

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
Copy link
Contributor

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 pankajsingh88 mentioned this issue Oct 28, 2024
Closed
pankajsingh88 added a commit to pankajsingh88/faiss that referenced this issue Oct 28, 2024
@pankajsingh88 @facebook-github-bot
add validity check AlignedTableTightAlloc clear method
93aa0e9 
Summary:
Avoid null pointer error by checking that `numel > 0`.
It's equivalent of checking`ptr != null` but keeping numel check for consistency.

Bug reported in: facebookresearch#3994

Differential Revision: D65073502
@pankajsingh88
Copy link
Contributor

#3997

pankajsingh88 added a commit to pankajsingh88/faiss that referenced this issue Oct 28, 2024
@pankajsingh88 @facebook-github-bot
add validity check AlignedTableTightAlloc clear method (facebookresea…
52248c2 
…rch#3997)

Summary:

Avoid null pointer error by checking that `numel > 0`.
It's equivalent of checking`ptr != null` but keeping numel check for consistency.

Bug reported in: facebookresearch#3994

Reviewed By: mnorris11

Differential Revision: D65073502
facebook-github-bot pushed a commit that referenced this issue Oct 28, 2024
@pankajsingh88 @facebook-github-bot
add validity check AlignedTableTightAlloc clear method (#3997)
5539039 
Summary:
Pull Request resolved: #3997

Avoid null pointer error by checking that `numel > 0`.
It's equivalent of checking`ptr != null` but keeping numel check for consistency.

Bug reported in: #3994

Reviewed By: mnorris11

Differential Revision: D65073502

fbshipit-source-id: f9aa56cbcd28fc352d45ef5b8727fa857fbe5345
@pankajsingh88
Copy link
Contributor

closing since the fix #3997 has landed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Implementation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pankajsingh88 @GKxxUCAS