classification
Title: obj2ast's error handling can lead to python crashing with a C-level assertion failure
Type: crash Stage: patch review
Components: Interpreter Core Versions: Python 3.1, Python 3.2, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, benjamin.peterson, brett.cannon, dmalcolm, georg.brandl, ncoghlan
Priority: normal Keywords: patch

Created on 2010-11-11 20:53 by dmalcolm, last changed 2010-11-20 01:39 by benjamin.peterson. This issue is now closed.

Files
File name Uploaded Description Edit
py3k-issue-10391-fix-ast-error-handling.patch dmalcolm, 2010-11-11 20:56
Messages (4)
msg120967 - (view) Author: Dave Malcolm (dmalcolm) (Python committer) Date: 2010-11-11 20:53
In various places within the generated Python/Python-ast.c, error handling generates a repr() and raises exceptions accordingly.

Currently in py3k the generated code uses PyBytes_AS_STRING() on the repr.  My understanding is that repr() should be a PyUnicodeObject, not a PyBytesObject.  This seems to be unchanged from r63682, which was a mass-change of PyString to PyBytes from 2 years ago.

This leads to a python crashing with an assertion failure:
test_compile_ast (__main__.TestSpecifics) ... python: Python/Python-ast.c:5835: obj2ast_expr: Assertion `((((((PyObject*)(tmp))->ob_type))->tp_flags & ((1L<<27))) != 0)' failed.

when invoking compile() on certain (malformed) trees of ast objects.
msg120968 - (view) Author: Dave Malcolm (dmalcolm) (Python committer) Date: 2010-11-11 20:56
The attached patch:
  - extends the ast error-handling selftest with code that triggers this crash (on unpatched code)
  - fixes Parser/asdl_c.py to generate code using _PyUnicode_AS_STRING instead
  - contains the generated changes to Python/Python-ast.c

FWIW, it's not clear to what extent _PyUnicode_AS_STRING is deprecated, Include/unicodeobject.h currently has:
   *** This API is for interpreter INTERNAL USE ONLY and will likely
   *** be removed or changed for Python 3.1.
but I hope it won't be, it's far too useful.
msg120971 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-11-11 21:22
IMO a better patch would be to use the %R format specification:
  PyErr_Format(
      PyExc_TypeError, "expected some sort of mod, but got %R", obj);
And no need to use PyObject_Repr and the temporary variable!
msg121588 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2010-11-20 01:39
r86538
History
Date User Action Args
2010-11-20 01:39:03benjamin.petersonsetstatus: open -> closed
resolution: fixed
messages: + msg121588
2010-11-11 21:22:43amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg120971
2010-11-11 20:57:28dmalcolmsetnosy: + brett.cannon, georg.brandl, ncoghlan, benjamin.peterson
2010-11-11 20:56:12dmalcolmsetfiles: + py3k-issue-10391-fix-ast-error-handling.patch
keywords: + patch
messages: + msg120968

stage: patch review
2010-11-11 20:53:42dmalcolmcreate