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] Allocating columns with small negative lengths succeeds #12116

Closed
revans2 opened this issue Nov 10, 2022 · 0 comments · Fixed by #12118
Closed

[BUG] Allocating columns with small negative lengths succeeds #12116

revans2 opened this issue Nov 10, 2022 · 0 comments · Fixed by #12118
Labels
bug Something isn't working Spark Functionality that helps Spark RAPIDS

Comments

@revans2
Copy link
Contributor

revans2 commented Nov 10, 2022

Describe the bug
This is directly related to rapidsai/rmm#1156. I recently tried to add in some extra error checking on top of cudf::strings::concatenate because I wanted to be ready in case #12087 didn't work out. As a part of this I tested overflowing by just a very small amount 4 characters more than the maximum amount. To my shock the call to concatenate passed but trying to convert that result to a column_view threw an exception saying that a column_view cannot have a negative length. It appears that the negative allocation is turned into a very large positive allocation because C++ inserts in an implicit cast with no warnings, then rapidsai/rmm#1156 kicks in and causes the allocation to be converted to 0 which returns a nullptr and succeeds. My concern is not so much with strings directly. Even if #12087 is fixed the same kind of thing can happen for any allocation that overflows by a small amount.

Steps/Code to reproduce bug
Allocate a numeric_column of INT8 with a length of -4. It will work and there will be no exceptions until you try to turn it into a column_view, which might not happen if you are trying to use the column directly to pull out raw pointers/etc.

Expected behavior
I would like very much if it were not possible to allocate negative length columns. If we have the assertion on a column_view why do we not have the same thing for a column? This way even if the RMM issue is fixed don't have to wonder about why we are trying to allocate really large allocations from RMM that failed, but instead we can take the next step and know that something overflowed and we need to look at who is trying to allocate a negative column.

@revans2 revans2 added bug Something isn't working Needs Triage Need team to review and classify labels Nov 10, 2022
@revans2 revans2 added the Spark Functionality that helps Spark RAPIDS label Nov 10, 2022
rapids-bot bot pushed a commit that referenced this issue Nov 15, 2022
This fixes #12116
This just adds in a few checks for negative sizes to avoid any issues with rounding errors and also helps us detect errors sooner. It will not fix small negative allocations for device buffers directly.

Authors:
  - Robert (Bobby) Evans (https://github.com/revans2)

Approvers:
  - Karthikeyan (https://github.com/karthikeyann)
  - David Wendt (https://github.com/davidwendt)
  - Nghia Truong (https://github.com/ttnghia)

URL: #12118
@bdice bdice removed the Needs Triage Need team to review and classify label Mar 4, 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 Spark Functionality that helps Spark RAPIDS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants