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] cudf::test::print seg faults on nested structs #13200

Closed
vyasr opened this issue Apr 21, 2023 · 2 comments · Fixed by #13243
Closed

[BUG] cudf::test::print seg faults on nested structs #13200

vyasr opened this issue Apr 21, 2023 · 2 comments · Fixed by #13243
Labels
bug Something isn't working

Comments

@vyasr
Copy link
Contributor

vyasr commented Apr 21, 2023

Describe the bug
When used to print out nested structs, the cudf::test::print utility triggers a segmentation fault rather than printing in some cases.

Steps/Code to reproduce bug
Add these two lines:

    cudf::test::print(output, std::cerr);                                                                                                                                                                  
    //cudf::test::print(expected_structs, std::cerr);        

just before this assertion in the NestedStruct_Sliced test. The result should be fine, until the second line is commented out. Then the code will seg fault.

Expected behavior
No seg fault

@vyasr vyasr added bug Something isn't working Needs Triage Need team to review and classify labels Apr 21, 2023
@ttnghia
Copy link
Contributor

ttnghia commented Apr 26, 2023

This is not a bug, but rather a feature 😛

At this line, the content of expected_structs is taken over, moved into expected_struct_structs. As such, printing it will crash and this is expected.

@davidwendt
Copy link
Contributor

I think that taking ownership of the parameters in this case is unexpected and the constructor should be copying the wrapper data instead of moving it.

  auto expected_struct_structs =
    cudf::test::structs_column_wrapper{{expected_structs}, cudf::test::iterators::null_at(2)};
// expected_structs is now empty/destroyed

Created PR #13243 to fix this.
The will make the corresponding constructors in structs_column_wrapper consistent with lists_column_wrapper which makes a copy of it's parameters in similar constructors.

rapids-bot bot pushed a commit that referenced this issue May 2, 2023
…#13243)

Fixes the `cudf::test::structs_column_wrapper` constructors that accept `column_wrapper` instances to copy the underlying column rather than move it. The constructor signatures do not provide any indication that the passed wrappers will be destroyed. Also, the other nested column wrapper constructors make copies unless the parameters are decorated with move declarations (i.e. `&&`). 

Closes #13200

Authors:
  - David Wendt (https://github.com/davidwendt)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - https://github.com/nvdbaranec

URL: #13243
@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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants