classification
Title: segfaults calling warnings.warn() with non-string message
Type: crash Stage:
Components: Interpreter Core Versions: Python 3.0
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: ajaksu2, benjamin.peterson, brett.cannon, dalcinl
Priority: release blocker Keywords: patch

Created on 2008-08-21 22:30 by dalcinl, last changed 2008-09-02 04:02 by brett.cannon. This issue is now closed.

Files
File name Uploaded Description Edit
_warnings.c.diff ajaksu2, 2008-08-22 15:03 patch for warnings.warn(non-string) segfault
fix_warn_funky_types.diff brett.cannon, 2008-08-22 18:49 Fix the crasher plus toss in some more error checking
Messages (10)
msg71694 - (view) Author: Lisandro Dalcin (dalcinl) Date: 2008-08-21 22:30
from warnings import warn

warn("hello world") # -> Success
warn(UserWarning)   # -> Segmentation fault
warn(None)          # -> Segmentation fault
warn(1)             # -> Segmentation fault
msg71709 - (view) Author: Daniel Diniz (ajaksu2) Date: 2008-08-22 00:27
Two small clues.

First, a backtrace:

#0  0xb7df102a in strcmp () from /lib/tls/i686/cmov/libc.so.6
#1  0x0809e678 in warn_explicit (category=0x81dd140, message=0xb7ac58f4,
filename=0xb7acced0, lineno=1, module=0xb7f53300,
    registry=0xb7ac9e94, sourceline=0x0) at Python/_warnings.c:393
#2  0x0809f1df in do_warn (message=0x81fbd78, category=0x81dd140,
stack_level=1) at Python/_warnings.c:606
#3  0x0809f37d in warnings_warn (self=0xb7aceab4, args=0xb7af0a7c,
kwds=0x0) at Python/_warnings.c:628
#4  0x081624ee in PyCFunction_Call (func=0xb7acace4, arg=0xb7af0a7c,
kw=0x0) at Objects/methodobject.c:84
#5  0x080b3633 in call_function (pp_stack=0xbfd51f44, oparg=1) at
Python/ceval.c:3403
#6  0x080ae776 in PyEval_EvalFrameEx (f=0x82b5e6c, throwflag=0) at
Python/ceval.c:2205
#7  0x080b1ac8 in PyEval_EvalCodeEx (co=0xb7ade988, globals=0xb7f4f5d4,
locals=0xb7f4f5d4, args=0x0, argcount=0, kws=0x0,
    kwcount=0, defs=0x0, defcount=0, kwdefs=0x0, closure=0x0) at
Python/ceval.c:2840
#8  0x080a686f in PyEval_EvalCode (co=0xb7ade988, globals=0xb7f4f5d4,
locals=0xb7f4f5d4) at Python/ceval.c:519
#9  0x080df486 in run_mod (mod=0x82ba910, filename=0x81a09e4 "<stdin>",
globals=0xb7f4f5d4, locals=0xb7f4f5d4,
    flags=0xbfd52370, arena=0x8216df8) at Python/pythonrun.c:1553
#10 0x080dd67e in PyRun_InteractiveOneFlags (fp=0xb7ec7440,
filename=0x81a09e4 "<stdin>", flags=0xbfd52370)
    at Python/pythonrun.c:958
#11 0x080dd1e0 in PyRun_InteractiveLoopFlags (fp=0xb7ec7440,
filename=0x81a09e4 "<stdin>", flags=0xbfd52370)
    at Python/pythonrun.c:870
#12 0x080dd038 in PyRun_AnyFileExFlags (fp=0xb7ec7440,
filename=0x81a09e4 "<stdin>", closeit=0, flags=0xbfd52370)
    at Python/pythonrun.c:839
#13 0x080ef6ba in Py_Main (argc=1, argv=0xb7f22028) at Modules/main.c:592
#14 0x0805a689 in main (argc=1, argv=0xbfd534c4) at ./Modules/python.c:57


Then, this behavior:
Python 3.0b3+ (py3k:65930M, Aug 21 2008, 21:23:08)
[GCC 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import _warnings
[40709 refs]
>>> _warnings.warn(0)
__main__:1: UserWarning: 0
[40739 refs]
>>> _warnings.warn(12345)
__main__:1: UserWarning: 12345
[40744 refs]
>>> _warnings.warn(AttributeError)
__main__:1: UserWarning: <class 'AttributeError'>
[40750 refs]
>>> import warnings
[41483 refs]
>>> warnings.warn(0)
[41483 refs]
>>> warnings.warn(12345)
[41483 refs]
>>> warnings.warn(10101)
Segmentation fault

That is, _warnings.warn(spam) works OK and avoids the
warnings.warn(spam) crash for values already called by the former.
msg71721 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-08-22 04:08
If you search for _PyUnicode_AsString() in Python/_warnings.c you will
find several places that assume that the proper measures have been taken
to make sure the object is a string. All of those places need to be
fixed so that if a string is not passed in then one is grabbed.

And the reason this turned out as a segfault is for a missing error
return value just before the strcmp() call.
msg71750 - (view) Author: Daniel Diniz (ajaksu2) Date: 2008-08-22 15:03
Brett,
I don't think I know C (and CPython) enough to fix this. I was able to
get rid of this specific segfault with this:

-            const char *text_char = _PyUnicode_AsString(text);
+            const char *text_char =
_PyUnicode_AsString(PyObject_Str(text));

But I have no idea whether I should also incref/decref the PyObject_Str.
msg71763 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-08-22 18:08
On Fri, Aug 22, 2008 at 8:03 AM, Daniel Diniz <report@bugs.python.org> wrote:
>
> Daniel Diniz <ajaksu@gmail.com> added the comment:
>
> Brett,
> I don't think I know C (and CPython) enough to fix this. I was able to
> get rid of this specific segfault with this:
>
> -            const char *text_char = _PyUnicode_AsString(text);
> +            const char *text_char =
> _PyUnicode_AsString(PyObject_Str(text));
>
> But I have no idea whether I should also incref/decref the PyObject_Str.
>

That's along the lines of what needs to be done (and what I was
planning on doing), although you need to do more error checking on the
return values. Plus the patch I am cooking up adds more checks in the
code for the return value of _PyUnicode_AsString().
msg71765 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-08-22 18:49
The patch doesn't actually bother with a translation as the code causing
issue is only there to prevent infinite recursion. So if the object
being used is not a string, then there is no need to worry as it is not
part of the infinite recursion problem.

I also added a bunch of missing error checks.
msg71766 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-08-22 18:56
Brett, is this patch ready for review?
msg71773 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-08-22 19:59
That's why the keyword is set. =)
msg71774 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-08-22 20:03
On Fri, Aug 22, 2008 at 2:59 PM, Brett Cannon <report@bugs.python.org> wrote:
>
> Brett Cannon <brett@python.org> added the comment:
>
> That's why the keyword is set. =)

Ah. I missed that. :) The patch looks fine.
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue3639>
> _______________________________________
>

-- 
Cheers,
Benjamin Peterson
"There's no place like 127.0.0.1."
msg72312 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-09-02 04:02
Checked in r66140.
History
Date User Action Args
2008-09-02 04:02:12brett.cannonsetstatus: open -> closed
resolution: accepted
messages: + msg72312
2008-09-01 14:25:26benjamin.petersonsetkeywords: - needs review
2008-08-22 20:03:10benjamin.petersonsetmessages: + msg71774
2008-08-22 19:59:35brett.cannonsetmessages: + msg71773
2008-08-22 18:56:43benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg71766
2008-08-22 18:49:04brett.cannonsetkeywords: + needs review
files: + fix_warn_funky_types.diff
messages: + msg71765
2008-08-22 18:08:45brett.cannonsetmessages: + msg71763
2008-08-22 15:03:11ajaksu2setfiles: + _warnings.c.diff
keywords: + patch
messages: + msg71750
2008-08-22 04:08:19brett.cannonsetmessages: + msg71721
2008-08-22 00:27:18ajaksu2setnosy: + ajaksu2
messages: + msg71709
2008-08-21 22:32:11benjamin.petersonsetpriority: release blocker
assignee: brett.cannon
nosy: + brett.cannon
2008-08-21 22:30:26dalcinlcreate