From ca51d2a44864572dc34d02b2b600d3e79bdccb0a Mon Sep 17 00:00:00 2001 From: David Wendt Date: Mon, 30 Jan 2023 11:34:51 -0500 Subject: [PATCH] Fix AllocateLikeTest gtests reading uninitialized null-mask --- cpp/src/copying/copy.cpp | 9 ++---- cpp/tests/copying/utility_tests.cpp | 50 ++++++++++++++++++++++------- 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/cpp/src/copying/copy.cpp b/cpp/src/copying/copy.cpp index 00147277231..2bcfeae20c4 100644 --- a/cpp/src/copying/copy.cpp +++ b/cpp/src/copying/copy.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2022, NVIDIA CORPORATION. + * Copyright (c) 2019-2023, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -125,16 +125,11 @@ std::unique_ptr allocate_like(column_view const& input, CUDF_EXPECTS(is_fixed_width(input.type()), "Expects only fixed-width type column"); mask_state allocate_mask = should_allocate_mask(mask_alloc, input.nullable()); - auto op = [&](auto const& child) { return allocate_like(child, size, mask_alloc, stream, mr); }; - auto begin = thrust::make_transform_iterator(input.child_begin(), op); - std::vector> children(begin, begin + input.num_children()); - return std::make_unique(input.type(), size, rmm::device_buffer(size * size_of(input.type()), stream, mr), detail::create_null_mask(size, allocate_mask, stream, mr), - state_null_count(allocate_mask, input.size()), - std::move(children)); + state_null_count(allocate_mask, input.size())); } } // namespace detail diff --git a/cpp/tests/copying/utility_tests.cpp b/cpp/tests/copying/utility_tests.cpp index c9d4d1a78b3..94ea759b96d 100644 --- a/cpp/tests/copying/utility_tests.cpp +++ b/cpp/tests/copying/utility_tests.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2022, NVIDIA CORPORATION. + * Copyright (c) 2019-2023, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -192,26 +192,52 @@ TYPED_TEST_SUITE(AllocateLikeTest, numeric_types); TYPED_TEST(AllocateLikeTest, ColumnNumericTestSameSize) { // For same size as input - cudf::size_type size = 10; - cudf::mask_state state = cudf::mask_state::ALL_VALID; - auto input = make_numeric_column(cudf::data_type{cudf::type_to_id()}, size, state); - auto expected = make_numeric_column( - cudf::data_type{cudf::type_to_id()}, size, cudf::mask_state::ALL_VALID); + cudf::size_type size = 10; + + auto input = make_numeric_column( + cudf::data_type{cudf::type_to_id()}, size, cudf::mask_state::UNALLOCATED); auto got = cudf::allocate_like(input->view()); - CUDF_TEST_EXPECT_COLUMN_PROPERTIES_EQUAL(*expected, *got); + CUDF_TEST_EXPECT_COLUMN_PROPERTIES_EQUAL(*input, *got); + + input = make_numeric_column( + cudf::data_type{cudf::type_to_id()}, size, cudf::mask_state::ALL_VALID); + got = cudf::allocate_like(input->view()); + EXPECT_EQ(input->type(), got->type()); + EXPECT_EQ(input->size(), got->size()); + EXPECT_EQ(input->nullable(), got->nullable()); + EXPECT_EQ(input->num_children(), got->num_children()); + // CUDF_TEST_EXPECT_COLUMN_PROPERTIES_EQUAL includes checking the null-count property. + // This value will be incorrect since the null mask will contain uninitialized bits + // and the null-count set to UNKNOWN_NULL_COUNT on return from allocate_like(). + // This means any subsequent call to null_count() will try to compute the null-count + // using the uninitialized null-mask. } TYPED_TEST(AllocateLikeTest, ColumnNumericTestSpecifiedSize) { - // For same size as input + // For different size as input cudf::size_type size = 10; cudf::size_type specified_size = 5; - cudf::mask_state state = cudf::mask_state::ALL_VALID; - auto input = make_numeric_column(cudf::data_type{cudf::type_to_id()}, size, state); - auto expected = make_numeric_column( - cudf::data_type{cudf::type_to_id()}, specified_size, cudf::mask_state::ALL_VALID); + + auto state = cudf::mask_state::UNALLOCATED; + auto input = make_numeric_column(cudf::data_type{cudf::type_to_id()}, size, state); + auto expected = + make_numeric_column(cudf::data_type{cudf::type_to_id()}, specified_size, state); auto got = cudf::allocate_like(input->view(), specified_size); CUDF_TEST_EXPECT_COLUMN_PROPERTIES_EQUAL(*expected, *got); + + input = make_numeric_column( + cudf::data_type{cudf::type_to_id()}, size, cudf::mask_state::ALL_VALID); + got = cudf::allocate_like(input->view(), specified_size); + EXPECT_EQ(input->type(), got->type()); + EXPECT_EQ(specified_size, got->size()); + EXPECT_EQ(input->nullable(), got->nullable()); + EXPECT_EQ(input->num_children(), got->num_children()); + // CUDF_TEST_EXPECT_COLUMN_PROPERTIES_EQUAL includes checking the null-count property. + // This value will be incorrect since the null mask will contain uninitialized bits + // and the null-count set to UNKNOWN_NULL_COUNT on return from allocate_like(). + // This means any subsequent call to null_count() will try to compute the null-count + // using the uninitialized null-mask. } CUDF_TEST_PROGRAM_MAIN()