Skip to content

Fix index overflow in edge cases of CAGRA graph optimize#435

Merged
rapids-bot[bot] merged 3 commits intorapidsai:branch-24.12from
achirkin:fix-cagra-optimize-integer-overflow
Nov 1, 2024
Merged

Fix index overflow in edge cases of CAGRA graph optimize#435
rapids-bot[bot] merged 3 commits intorapidsai:branch-24.12from
achirkin:fix-cagra-optimize-integer-overflow

Conversation

@achirkin
Copy link
Copy Markdown
Contributor

Force input_graph_degree, output_graph_degree, and graph_size variables to uint64_t.
Before the PR, they've been uint32_t, and the product of them could overflow. This would lead to cudaMemsetAsync not filling in a large fraction of the graph.

It's not known whether this bug has surfaced for anyone until now, but it's better to be safe than sorry.
The change shouldn't have any impact on performance.

@achirkin achirkin added bug Something isn't working non-breaking Introduces a non-breaking change labels Oct 30, 2024
@achirkin achirkin self-assigned this Oct 30, 2024
@achirkin achirkin requested a review from a team as a code owner October 30, 2024 07:59
@github-actions github-actions Bot added the cpp label Oct 30, 2024
Copy link
Copy Markdown
Contributor

@lowener lowener left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@achirkin
Copy link
Copy Markdown
Contributor Author

achirkin commented Nov 1, 2024

/merge

@rapids-bot rapids-bot Bot merged commit 71deb26 into rapidsai:branch-24.12 Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cpp non-breaking Introduces a non-breaking change

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants