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.

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

[BUG] Illegal memory access to collision counter in concurrent_unordered_multimap #7413

Closed
gaohao95 opened this issue Feb 18, 2021 · 5 comments
Labels
bug Something isn't working

Comments

@gaohao95
Copy link
Contributor

gaohao95 commented Feb 18, 2021

Describe the bug
In cpp/src/hash/concurrent_unordered_multimap.cuh, member element m_collisions is allocated in host memory, but the host pointer is passed to atomicAdd in device function insert, here, and here.

Steps/Code to reproduce bug
Any code using concurrent_unordered_multimap with count_collisions set to true should observe illegal memory access in the kernel.

@gaohao95 gaohao95 added Needs Triage Need team to review and classify bug Something isn't working labels Feb 18, 2021
@jrhemstad jrhemstad removed the Needs Triage Need team to review and classify label Feb 19, 2021
@jrhemstad
Copy link
Contributor

jrhemstad commented Feb 19, 2021

Since the concurrent_unordered_multimap is not public facing and the count_collisions functionality is not used internally, this will be fairly low priority to fix. Happy to review any PRs w/ the fix.

@gaohao95
Copy link
Contributor Author

Since the concurrent_unordered_multimap is not public facing and the count_collisions functionality is not used internally, this will be fairly low priority to fix.

No problem at all. It's not a blocker for me as well. Just want to raise an issue here so that it can get tracked.

@jrhemstad
Copy link
Contributor

No problem at all. It's not a blocker for me as well. Just want to raise an issue here so that it can get tracked.

Thanks. I think this oversight is a holdover from when the concurrent_unordered_multimap object itself was allocated in managed memory.

@github-actions
Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@vyasr
Copy link
Contributor

vyasr commented Jul 25, 2022

No longer relevant as of #10642.

@vyasr vyasr closed this as completed Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants