classification
Title: cPickle: stack underflow in load_pop()
Type: crash Stage: resolved
Components: Extension Modules Versions: Python 3.1, Python 3.2, Python 2.7, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: alexandre.vassalotti, collinwinter, eric.smith, flox, haypo, jackdied, pitrou
Priority: normal Keywords: patch

Created on 2009-12-08 02:59 by haypo, last changed 2010-01-10 03:04 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
cpickle_load_pop-3.patch haypo, 2009-12-11 11:40
issue7455_cpickle_load_pop.diff flox, 2009-12-19 00:12 Patch, apply to trunk
issue7455_cpickle_load_pop_py3k.diff flox, 2009-12-19 00:13 Patch, apply to py3k
Messages (9)
msg96106 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2009-12-08 02:59
load_pop() pops a value if the stack length is >= 0. The test is wrong:
if the length is zero, the stack is empty and stackUnderflow() have to
be called.

Example:

  $ ../../python -c "import cPickle; cPickle.loads('0')"
  Erreur de segmentation
msg96107 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2009-12-08 06:20
Can you add a test for this?
msg96121 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2009-12-08 10:48
> Can you add a test for this?

I wasn't sure of the right place to add the test. I tried
Lib/test/pickletester.py, but it doesn't work because UnpicklingError
is not defined there.

Well, here is a new version of my patch with an unit test.
msg96245 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2009-12-11 11:40
Oops, cpickle_load_pop-2.patch contains an useless variable (picklingError 
= pickle.UnpicklingError). Here is the version 3 of my patch.
msg96273 - (view) Author: Jack Diederich (jackdied) * (Python committer) Date: 2009-12-11 21:01
This seems to have been introduced in r72930 when the stackUnderflow()
was moved from the top of the function to the bottom.  It used to test
for len > 0.

Question, should cPickle and pickle be raising the same error here? 
UnpicklingError is defined in pickle.py and never used but cPickle.c
uses it frequently.
msg96585 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2009-12-18 21:29
The patch works correctly on 2.7.

It should be applied on trunk, and backported to other branches, too.

(issue7542 is marked as duplicate of this one)
msg96593 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2009-12-19 00:12
I moved the test to test.pickletester, so that it can be used for
all 3 tests:
 test_pickle
 test_cpickle
 test_xpickle

(and backported a test which was only on py3k)
msg97361 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-01-07 18:05
Committed in all four branches. Thank you Victor and Florent!
msg97495 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-01-10 03:04
> Committed in all four branches.

trunk: r77352
History
Date User Action Args
2010-01-22 19:54:08eric.smithlinkissue7758 superseder
2010-01-10 03:04:15hayposetmessages: + msg97495
2010-01-07 18:05:01pitrousetstatus: open -> closed

nosy: + pitrou
messages: + msg97361

resolution: fixed
stage: patch review -> resolved
2010-01-07 17:27:30floxsetfiles: - issue7455_silence_py3k_warning.diff
2009-12-21 22:24:24floxsetpriority: normal
stage: test needed -> patch review
2009-12-19 00:13:42floxsetfiles: + issue7455_silence_py3k_warning.diff
2009-12-19 00:13:10floxsetfiles: - issue7455_silence_py3k_warning.diff
2009-12-19 00:13:02floxsetfiles: + issue7455_cpickle_load_pop_py3k.diff
2009-12-19 00:12:37floxsetfiles: + issue7455_cpickle_load_pop.diff

messages: + msg96593
2009-12-18 21:48:31floxsetfiles: + issue7455_silence_py3k_warning.diff
2009-12-18 21:29:53floxsettype: crash
2009-12-18 21:29:36floxsetnosy: + flox

messages: + msg96585
versions: + Python 2.6, Python 3.1, Python 3.2
2009-12-11 21:01:22jackdiedsetnosy: + jackdied, collinwinter
messages: + msg96273
2009-12-11 13:10:15pitrousetnosy: + alexandre.vassalotti
2009-12-11 11:40:42hayposetfiles: + cpickle_load_pop-3.patch

messages: + msg96245
2009-12-11 11:39:53hayposetfiles: - cpickle_load_pop-2.patch
2009-12-11 11:39:47hayposetfiles: - cpickle_load_pop.patch
2009-12-08 10:49:00hayposetfiles: + cpickle_load_pop-2.patch

messages: + msg96121
2009-12-08 06:20:13eric.smithsetnosy: + eric.smith

messages: + msg96107
stage: test needed
2009-12-08 02:59:17haypocreate