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

Fix cudf::repeat logic when count is zero #13459

Merged
merged 1 commit into from
May 30, 2023

Conversation

davidwendt
Copy link
Contributor

Description

Fixes logic in cudf::repeat to check for count==0 before using it to check for possible out-of-bounds output size result.
Added a specific gtest for this as well.

Closes #13458

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@davidwendt davidwendt added bug Something isn't working 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels May 26, 2023
@davidwendt davidwendt self-assigned this May 26, 2023
@davidwendt davidwendt requested a review from a team as a code owner May 26, 2023 16:06
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

I would support this fix going into 23.06. It's very low risk and fixes a crash.

Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

I verified this fixes the crashes we saw in our Spark integration test. Thanks for the quick turnaround, @davidwendt!

Note that we would like to see this in 23.06 to unblock our release pipelines there.

@bdice bdice changed the base branch from branch-23.08 to branch-23.06 May 26, 2023 21:29
@bdice bdice requested review from a team as code owners May 26, 2023 21:29
@bdice bdice requested review from vyasr and brandon-b-miller and removed request for a team May 26, 2023 21:29
@bdice bdice changed the base branch from branch-23.06 to branch-23.08 May 26, 2023 21:30
@bdice bdice removed request for a team, vyasr, hyperbolic2346 and brandon-b-miller May 26, 2023 21:30
@bdice
Copy link
Contributor

bdice commented May 26, 2023

Oops, I made a mistake (and fixed it). I changed the target branch to 23.06 and reverted it. We do need to rebase this over branch-23.06. Luckily it's only a single commit so this is not too tough. @davidwendt Can you rebase and change the target? cc: @GregoryKimball since this fix is needed during freeze.

@davidwendt davidwendt changed the base branch from branch-23.08 to branch-23.06 May 28, 2023 22:36
@github-actions github-actions bot added ci CMake CMake build issue Java Affects Java cuDF API. Python Affects Python cuDF API. labels May 28, 2023
@davidwendt davidwendt changed the base branch from branch-23.06 to branch-23.08 May 28, 2023 22:42
@davidwendt
Copy link
Contributor Author

My git foo is weak so I'll need help rebasing this PR.

@github-actions github-actions bot removed ci Python Affects Python cuDF API. CMake CMake build issue Java Affects Java cuDF API. labels May 30, 2023
@davidwendt davidwendt changed the base branch from branch-23.08 to branch-23.06 May 30, 2023 14:48
@bdice
Copy link
Contributor

bdice commented May 30, 2023

@davidwendt the rebase looks good to me. I think we’ll need an admin merge once CI passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] cudf::repeat crashes with SIGFPE when count == 0
7 participants