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.

Sign up for GitHub

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

Jump to bottom

Fixes out-of-bounds access for small files in unzip #8498

Merged
merged 9 commits into from
Jun 17, 2021
Merged

Fixes out-of-bounds access for small files in unzip #8498

merged 9 commits into from
Jun 17, 2021

Conversation

elstehle
Copy link
Contributor

This PR fixes #8497.

@elstehle elstehle added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Jun 11, 2021
@elstehle elstehle requested a review from a team as a code owner June 11, 2021 16:44
@elstehle elstehle added the bug Something isn't working label Jun 11, 2021
@codecov
Copy link

codecov bot commented Jun 11, 2021 •
edited
Loading

Codecov Report

Merging #8498 (801efde) into branch-21.08 (93ce6c7) will increase coverage by 0.38%.
The diff coverage is 100.00%.

❗ Current head 801efde differs from pull request most recent head e408fad. Consider uploading reports for the commit e408fad to get more accurate results
Impacted file tree graph

@@               Coverage Diff                @@
##           branch-21.08    #8498      +/-   ##
================================================
+ Coverage         82.57%   82.96%   +0.38%     
================================================
  Files               109      109              
  Lines             17870    18197     +327     
================================================
+ Hits              14757    15097     +340     
+ Misses             3113     3100      -13
Impacted Files Coverage Δ
python/cudf/cudf/core/frame.py 93.41% <ø> (+0.45%) ⬆️
python/cudf/cudf/core/column/column.py 87.64% <100.00%> (+0.11%) ⬆️
python/cudf/cudf/core/column_accessor.py 96.93% <100.00%> (+0.10%) ⬆️
python/cudf/cudf/core/dataframe.py 90.98% <100.00%> (+0.33%) ⬆️
python/cudf/cudf/io/feather.py 100.00% <0.00%> (ø)
python/cudf/cudf/comm/serialize.py 0.00% <0.00%> (ø)
python/cudf/cudf/_fuzz_testing/io.py 0.00% <0.00%> (ø)
python/cudf/cudf/utils/applyutils.py 100.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/_version.py 0.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/io/tests/test_csv.py 100.00% <0.00%> (ø)
... and 46 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93ce6c7...e408fad. Read the comment docs.

@davidwendt
Copy link
Contributor

Is it possible to create a gtest for this?

@vuule vuule self-requested a review June 14, 2021 20:19
@vuule vuule added the cuIO cuIO issue label Jun 14, 2021
vuule
vuule reviewed Jun 14, 2021
View reviewed changes
cpp/src/io/comp/uncomp.cpp Outdated Show resolved Hide resolved
@elstehle elstehle mentioned this pull request Jun 14, 2021
Closed
@elstehle
Copy link
Contributor Author

Is it possible to create a gtest for this?

Have added a test now 👍

vuule
vuule reviewed Jun 15, 2021
View reviewed changes
cpp/tests/io/csv_test.cpp Outdated Show resolved Hide resolved
vuule
vuule requested changes Jun 16, 2021
View reviewed changes
Copy link
Contributor

@vuule vuule left a comment

Choose a reason for hiding this comment

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

Suggestion to move the test to Python to make it modifiable+simpler.

cpp/tests/io/csv_test.cpp Outdated Show resolved Hide resolved
@elstehle elstehle requested a review from a team as a code owner June 16, 2021 14:36
galipremsagar
galipremsagar approved these changes Jun 16, 2021 •
edited
Loading
View reviewed changes
@github-actions github-actions bot added the Python Affects Python cuDF API. label Jun 16, 2021
@vuule vuule self-requested a review June 16, 2021 17:39
vuule
vuule requested changes Jun 16, 2021
View reviewed changes
python/cudf/cudf/tests/test_csv.py Outdated Show resolved Hide resolved
@vuule
Copy link
Contributor

vuule commented Jun 16, 2021

Hmm, it looks like some unrelated changes to Python tests got pulled in.

@vuule vuule added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Jun 17, 2021
@devavret
Copy link
Contributor

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 46c13c1 into rapidsai:branch-21.08 Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@galipremsagar galipremsagar galipremsagar approved these changes

@vuule vuule vuule approved these changes

@charlesbluca charlesbluca charlesbluca approved these changes

@davidwendt davidwendt davidwendt approved these changes

@jrhemstad jrhemstad Awaiting requested review from jrhemstad

@isVoid isVoid Awaiting requested review from isVoid

Assignees
No one assigned
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working cuIO cuIO issue libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Segfault when reading small zipped CSV files
6 participants
@elstehle @davidwendt @vuule @devavret @galipremsagar @charlesbluca