Skip to content

BUG: Fix memory access violations in is_lexsorted #18005

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
Merged
merged 3 commits into from
Oct 28, 2017
Merged

BUG: Fix memory access violations in is_lexsorted #18005

merged 3 commits into from
Oct 28, 2017
+5 −4

Conversation

cgohlke
Copy link
Contributor

@cgohlke cgohlke commented Oct 27, 2017

Fixes the following segfault during tests:

running: pytest --skip-slow --skip-network X:\Python36\lib\site-packages\pandas
============================= test session starts =============================
platform win32 -- Python 3.6.3, pytest-3.2.3, py-1.4.34, pluggy-0.4.0
rootdir: ., inifile:
plugins: pep8-1.0.6, faulthandler-1.3.1, cov-2.5.1, palladium-1.1.0.1, celery-4.1.0
collected 15967 items / 2 skipped

. ...........................................X.....................................Windows fatal exception: access violation

Current thread 0x000012d4 (most recent call first):
  File "X:\Python36\lib\site-packages\pandas\tests\test_algos.py", line 1236 in test_is_lexsorted
  File "X:\Python36\lib\site-packages\_pytest\python.py", line 143 in pytest_pyfunc_call
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 614 in execute
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 265 in __init__
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 248 in _wrapped_call
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 613 in execute
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 334 in <lambda>
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 339 in _hookexec
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 745 in __call__
  File "X:\Python36\lib\site-packages\_pytest\python.py", line 1169 in runtest
  File "X:\Python36\lib\site-packages\_pytest\runner.py", line 112 in pytest_runtest_call
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 614 in execute
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 265 in __init__
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 248 in _wrapped_call
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 613 in execute
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 334 in <lambda>
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 339 in _hookexec
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 745 in __call__
  File "X:\Python36\lib\site-packages\_pytest\runner.py", line 182 in <lambda>
  File "X:\Python36\lib\site-packages\_pytest\runner.py", line 196 in __init__
  File "X:\Python36\lib\site-packages\_pytest\runner.py", line 182 in call_runtest_hook
  File "X:\Python36\lib\site-packages\_pytest\runner.py", line 162 in call_and_report
  File "X:\Python36\lib\site-packages\_pytest\runner.py", line 82 in runtestprotocol
  File "X:\Python36\lib\site-packages\_pytest\runner.py", line 68 in pytest_runtest_protocol
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 614 in execute
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 265 in __init__
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 248 in _wrapped_call
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 613 in execute
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 265 in __init__
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 248 in _wrapped_call
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 613 in execute
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 265 in __init__
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 248 in _wrapped_call
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 613 in execute
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 334 in <lambda>
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 339 in _hookexec
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 745 in __call__
  File "X:\Python36\lib\site-packages\_pytest\main.py", line 169 in pytest_runtestloop
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 614 in execute
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 334 in <lambda>
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 339 in _hookexec
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 745 in __call__
  File "X:\Python36\lib\site-packages\_pytest\main.py", line 146 in _main
  File "X:\Python36\lib\site-packages\_pytest\main.py", line 110 in wrap_session
  File "X:\Python36\lib\site-packages\_pytest\main.py", line 139 in pytest_cmdline_main
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 614 in execute
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 334 in <lambda>
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 339 in _hookexec
  File "X:\Python36\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 745 in __call__
  File "X:\Python36\lib\site-packages\_pytest\config.py", line 58 in main
  File "X:\Python36\lib\site-packages\pandas\util\_tester.py", line 22 in test
  File "<string>", line 1 in <module>

@jreback
Copy link
Contributor

jreback commented Oct 27, 2017

hmm, is this on a 32-bit machine?

@cgohlke
Copy link
Contributor Author

cgohlke commented Oct 27, 2017

The test is running on 64-bit Python 3.6. But I expect the same on 32-bit since the function expects 'int64' but gets 'int32' and also indexes vecs[0][-1]

@jreback
Copy link
Contributor

jreback commented Oct 27, 2017

is your setup differerent from appveyor: https://ci.appveyor.com/project/pandas-dev/pandas/build/1.0.5863

?

@jreback
Copy link
Contributor

jreback commented Oct 27, 2017

note I don't doubt this is an issue, rather seeing what we are not testing.

@cgohlke
Copy link
Contributor Author

cgohlke commented Oct 27, 2017

is your setup different from appveyor

I am using debug builds and enable heap verification during the tests.

@codecov
Copy link

codecov bot commented Oct 27, 2017

Codecov Report

Merging #18005 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18005      +/-   ##
==========================================
- Coverage   91.24%   91.23%   -0.01%     
==========================================
  Files         163      163              
  Lines       50173    50173              
==========================================
- Hits        45778    45774       -4     
- Misses       4395     4399       +4
Flag Coverage Δ
#multiple 89.04% <ø> (ø) ⬆️
#single 40.29% <ø> (-0.05%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.75% <0%> (-0.1%) ⬇️
pandas/io/msgpack/_version.py 44.65% <0%> (+1.9%) ⬆️

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 fc1e507...a2e566c. Read the comment docs.

@codecov
Copy link

codecov bot commented Oct 27, 2017 •
edited
Loading

Codecov Report

Merging #18005 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18005      +/-   ##
==========================================
- Coverage   91.24%   91.23%   -0.01%     
==========================================
  Files         163      163              
  Lines       50173    50176       +3     
==========================================
- Hits        45778    45777       -1     
- Misses       4395     4399       +4
Flag Coverage Δ
#multiple 89.04% <ø> (+0.01%) ⬆️
#single 40.28% <ø> (-0.06%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.75% <0%> (-0.1%) ⬇️
pandas/core/generic.py 92.42% <0%> (-0.03%) ⬇️
pandas/core/indexing.py 92.8% <0%> (-0.02%) ⬇️
pandas/core/resample.py 96.16% <0%> (+0.01%) ⬆️
pandas/core/dtypes/concat.py 99.14% <0%> (+0.01%) ⬆️
pandas/core/indexes/numeric.py 97.27% <0%> (+0.03%) ⬆️
pandas/io/packers.py 88.3% <0%> (+0.1%) ⬆️
pandas/io/msgpack/_version.py 44.65% <0%> (+1.9%) ⬆️

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 fc1e507...8335ea8. Read the comment docs.

jreback
jreback requested changes Oct 27, 2017
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

small comment

pandas/_libs/algos.pyx Outdated
@@ -99,11 +99,14 @@ def is_lexsorted(list list_of_arrays):
cdef int64_t **vecs = <int64_t**> malloc(nlevels * sizeof(int64_t*))
for i in range(nlevels):
arr = list_of_arrays[i]
if arr.dtype.name != 'int64':
Copy link
Contributor

Choose a reason for hiding this comment

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

can u make this an assert instead

Copy link
Contributor

Choose a reason for hiding this comment

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

this is private

@jreback jreback added Bug Windows Windows OS labels Oct 27, 2017
@jreback jreback added this to the 0.21.1 milestone Oct 27, 2017
@jreback jreback mentioned this pull request Oct 27, 2017
Closed
@jreback
Copy link
Contributor

jreback commented Oct 28, 2017

lgtm. will merge on green.

@jreback jreback merged commit 34abef2 into pandas-dev:master Oct 28, 2017
@jreback
Copy link
Contributor

jreback commented Oct 28, 2017

thanks @cgohlke

peterpanmj pushed a commit to peterpanmj/pandas that referenced this pull request Oct 31, 2017
@cgohlke @peterpanmj
BUG: Fix memory access violations in is_lexsorted (pandas-dev#18005)
b3e8f69
No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017
@cgohlke @No-Stream
BUG: Fix memory access violations in is_lexsorted (pandas-dev#18005)
83e5be2
TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Dec 8, 2017
@cgohlke @TomAugspurger
BUG: Fix memory access violations in is_lexsorted (pandas-dev#18005)
679d3b9 
(cherry picked from commit 34abef2)
TomAugspurger pushed a commit that referenced this pull request Dec 11, 2017
@cgohlke @TomAugspurger
BUG: Fix memory access violations in is_lexsorted (#18005)
c0ff0d1 
(cherry picked from commit 34abef2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Bug Windows Windows OS
Projects
None yet
Milestone
0.21.1
Development

Successfully merging this pull request may close these issues.
2 participants
@cgohlke @jreback