Skip to content

Commit

Permalink
Add in negative size checks for columns (#12118)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
revans2 authored Nov 15, 2022
1 parent bae9e39 commit 186e129
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 1 deletion.
3 changes: 3 additions & 0 deletions cpp/include/cudf/column/column.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ class column {
* @note This constructor is primarily intended for use in column factory
* functions.
*
* @throws cudf::logic_error if `size < 0`
*
* @param[in] dtype The element type
* @param[in] size The number of elements in the column
* @param[in] data The column's data
Expand All @@ -133,6 +135,7 @@ class column {
_null_count{null_count},
_children{std::move(children)}
{
CUDF_EXPECTS(size >= 0, "Column size cannot be negative.");
}

/**
Expand Down
7 changes: 6 additions & 1 deletion cpp/include/cudf/column/column_factories.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ std::unique_ptr<column> make_empty_column(type_id id);
*
* @throws std::bad_alloc if device memory allocation fails
* @throws cudf::logic_error if `type` is not a numeric type
* @throws cudf::logic_error if `size < 0`
*
* @param[in] type The desired numeric element type
* @param[in] size The number of elements in the column
Expand Down Expand Up @@ -119,6 +120,7 @@ std::unique_ptr<column> make_numeric_column(
* @note The column's null count is determined by the requested null mask `state`.
*
* @throws cudf::logic_error if `type` is not a `fixed_point` type.
* @throws cudf::logic_error if `size < 0`
*
* @param[in] type The desired `fixed_point` element type.
* @param[in] size The number of elements in the column.
Expand Down Expand Up @@ -176,6 +178,7 @@ std::unique_ptr<column> make_fixed_point_column(
*
* @throws std::bad_alloc if device memory allocation fails
* @throws cudf::logic_error if `type` is not a timestamp type
* @throws cudf::logic_error if `size < 0`
*
* @param[in] type The desired timestamp element type
* @param[in] size The number of elements in the column
Expand Down Expand Up @@ -234,6 +237,7 @@ std::unique_ptr<column> make_timestamp_column(
*
* @throws std::bad_alloc if device memory allocation fails
* @throws cudf::logic_error if `type` is not a duration type
* @throws cudf::logic_error if `size < 0`
*
* @param[in] type The desired duration element type
* @param[in] size The number of elements in the column
Expand Down Expand Up @@ -292,6 +296,7 @@ std::unique_ptr<column> make_duration_column(
*
* @throws std::bad_alloc if device memory allocation fails
* @throws cudf::logic_error if `type` is not a fixed width type
* @throws cudf::logic_error if `size < 0`
*
* @param[in] type The desired fixed width type
* @param[in] size The number of elements in the column
Expand Down Expand Up @@ -366,7 +371,7 @@ std::unique_ptr<column> make_fixed_width_column(
* @param[in] stream CUDA stream used for device memory operations and kernel launches.
* @param[in] mr Device memory resource used for allocation of the column's `null_mask` and children
* columns' device memory.
* @return Constructed strings column
* @return Constructed strings column
*/
std::unique_ptr<column> make_strings_column(
cudf::device_span<thrust::pair<const char*, size_type> const> strings,
Expand Down
5 changes: 5 additions & 0 deletions cpp/src/column/column_factories.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ std::unique_ptr<column> make_numeric_column(data_type type,
{
CUDF_FUNC_RANGE();
CUDF_EXPECTS(is_numeric(type), "Invalid, non-numeric type.");
CUDF_EXPECTS(size >= 0, "Column size cannot be negative.");

return std::make_unique<column>(type,
size,
Expand All @@ -97,6 +98,7 @@ std::unique_ptr<column> make_fixed_point_column(data_type type,
{
CUDF_FUNC_RANGE();
CUDF_EXPECTS(is_fixed_point(type), "Invalid, non-fixed_point type.");
CUDF_EXPECTS(size >= 0, "Column size cannot be negative.");

return std::make_unique<column>(type,
size,
Expand All @@ -115,6 +117,7 @@ std::unique_ptr<column> make_timestamp_column(data_type type,
{
CUDF_FUNC_RANGE();
CUDF_EXPECTS(is_timestamp(type), "Invalid, non-timestamp type.");
CUDF_EXPECTS(size >= 0, "Column size cannot be negative.");

return std::make_unique<column>(type,
size,
Expand All @@ -133,6 +136,7 @@ std::unique_ptr<column> make_duration_column(data_type type,
{
CUDF_FUNC_RANGE();
CUDF_EXPECTS(is_duration(type), "Invalid, non-duration type.");
CUDF_EXPECTS(size >= 0, "Column size cannot be negative.");

return std::make_unique<column>(type,
size,
Expand Down Expand Up @@ -166,6 +170,7 @@ std::unique_ptr<column> make_dictionary_from_scalar(scalar const& s,
rmm::mr::device_memory_resource* mr)
{
if (size == 0) return make_empty_column(type_id::DICTIONARY32);
CUDF_EXPECTS(size >= 0, "Column size cannot be negative.");
CUDF_EXPECTS(s.is_valid(stream), "cannot create a dictionary with a null key");
return make_dictionary_column(
make_column_from_scalar(s, 1, stream, mr),
Expand Down

0 comments on commit 186e129

Please sign in to comment.