[Python 2.4] PyInt_FromLong returning NULL
While using Zope 2.7 with Python 2.4 we discovered some strange behaviour of the security machinery. I could track this down to some Zope code in cAccessControl.c where an Unauthorized exception is raised because of a call to PyInt_FromLong(1) which returns NULL. What could be the reason that such a "stupid" call return NULL in a reproducable way? -aj
Andreas Jung <lists@andreas-jung.com> writes:
While using Zope 2.7 with Python 2.4 we discovered some strange behaviour of the security machinery. I could track this down to some Zope code in cAccessControl.c where an Unauthorized exception is raised because of a call to PyInt_FromLong(1) which returns NULL. What could be the reason that such a "stupid" call return NULL in a reproducable way?
A memory scribble somewhere else? Possibly a DECREF too many somewhere? Is an exception set? Have you tried a debug build? Etc. Cheers, mwh -- All obscurity will buy you is time enough to contract venereal diseases. -- Tim Peters, python-dev
Andreas Jung wrote:
While using Zope 2.7 with Python 2.4 we discovered some strange behaviour of the security machinery. I could track this down to some Zope code in cAccessControl.c where an Unauthorized exception is raised because of a call to PyInt_FromLong(1) which returns NULL. What could be the reason that such a "stupid" call return NULL in a reproducable way?
Ugh. Part of the problem is that all of those calls are unchecked, Dang us. If they were checked, then, who knows, we might have gotten informative exceptions. I'd say the first step should be to add checks. Jim -- Jim Fulton mailto:jim@zope.com Python Powered! CTO (540) 361-1714 http://www.python.org Zope Corporation http://www.zope.com http://www.zope.org
[Jim Fulton]
Ugh. Part of the problem is that all of those calls are unchecked, Dang us. If they were checked, then, who knows, we might have gotten informative exceptions.
They certainly "should" be checked, but as a pragmatic matter PyInt_FromLong(1) can't fail -- Python allocates an int object for 1 (and for about 100 other popular little integers) when it starts up, and PyInt_FromLong() just returns a new reference to these pre-existing objects whenever possible. So, wrt:
I'd say the first step should be to add checks
that's probably not going to help. I'd make it the fourth thing <wink>.
Tim Peters wrote:
[Jim Fulton]
Ugh. Part of the problem is that all of those calls are unchecked, Dang us. If they were checked, then, who knows, we might have gotten informative exceptions.
They certainly "should" be checked, but as a pragmatic matter PyInt_FromLong(1) can't fail -- Python allocates an int object for 1 (and for about 100 other popular little integers) when it starts up, and PyInt_FromLong() just returns a new reference to these pre-existing objects whenever possible.
I know. I'm sure that's why we don't bother. But, obviously, it can fail. Jim -- Jim Fulton mailto:jim@zope.com Python Powered! CTO (540) 361-1714 http://www.python.org Zope Corporation http://www.zope.com http://www.zope.org
[Tim]
... but as a pragmatic matter PyInt_FromLong(1) can't fail --
[Jim]
I know. I'm sure that's why we don't bother. But, obviously, it can fail.
I disagree -- it's not obvious at all. Looking at the code, it's far more likely that Andreas misunderstood the cause of the failure than that PyInt_FromLong(1) actually returned NULL. If it did return NULL, then it's got to be something as rare as bad code generation (for reasons I explained earlier), or a non-standard compilation that fiddled the NSMALLPOSINTS and/or NSMALLNEGINTS #defines to insane values. This is the entire expected path in PyInt_FromLong(1): PyObject * PyInt_FromLong(long ival) { register PyIntObject *v; #if NSMALLNEGINTS + NSMALLPOSINTS > 0 if (-NSMALLNEGINTS <= ival && ival < NSMALLPOSINTS) { v = small_ints[ival + NSMALLNEGINTS]; Py_INCREF(v); #ifdef COUNT_ALLOCS if (ival >= 0) quick_int_allocs++; else quick_neg_int_allocs++; #endif return (PyObject *) v; } #endif It's not possible for that to return NULL -- even if small_ints[] got corrupted, so that v == NULL, the Py_INCREF(v) would have blown up before the function could have returned.
Sorry, false alarm :-( There assignment of the NULL occurs in the if-clause of the corresponding code (I have overseen the ASSIGN call): if (! PyInt_Check(p)) { if (PyDict_Check(p)) { if (PyString_Check(name) || PyUnicode_Check(name)) { ASSIGN(p, PyObject_GetItem(p, name)); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ if (p == NULL) { puts("PyObject returned NULL"); PyErr_Clear(); } } else p = PyInt_FromLong((long)1); ...doing some further investigations on that. -aj
On Tue, Dec 07, 2004 at 01:02:51PM -0500, Tim Peters wrote:
I'd say the first step should be to add checks
pre-existing objects whenever possible. So, wrt: that's probably not going to help. I'd make it the fourth thing <wink>.
Is it possible that some other Python API call is raising an exception, but a NULL return isn't being checked for, and PyInt_FromLong() happens to be the first bit of code that does 'if (PyErr_Occurred())'? Though from a quick glance at PyInt_FromLong() and the macros it uses, I don't see any references to PyErr_Occurred()... --amk
[A.M. Kuchling]
Is it possible that some other Python API call is raising an exception, but a NULL return isn't being checked for, and PyInt_FromLong() happens to be the first bit of code that does 'if (PyErr_Occurred())'? Though from a quick glance at PyInt_FromLong() and the macros it uses, I don't see any references to PyErr_Occurred()...
A long stare wouldn't uncover any either <wink>. This isn't it. That 1 is passed as an argument is important too, which cuts out all but the simplest path thru this code. The (current Zope 2.7 branch) source for cAccessControl.c contains six calls to PyInt_FromLong(). Five of them pass the literal 1. The other passes a variable with value -1, 0, or 1. So regardless of which of these Andreas is talking about, it's going thru PyInt_FromLong's fast path.
participants (5)
-
A.M. Kuchling -
Andreas Jung -
Jim Fulton -
Michael Hudson -
Tim Peters