| msg64234 - (view) |
Author: Rolland Dudemaine (rolland) |
Date: 2008-03-21 10:27 |
| In many files, the following code is present (with slight variations, but the important part is there) : static PyObject * objargs_mktuple(va_list va) { int i, n = 0; va_list countva; PyObject *result, *tmp; #ifdef VA_LIST_IS_ARRAY memcpy(countva, va, sizeof(va_list)); #else #ifdef __va_copy __va_copy(countva, va); #else countva = va; #endif #endif ... memcpy() is accessing va_list before it is initialized. Before the first access to a va_list type variable, and after the last access to that variable, calls to va_start() and va_end() must be made to initialize and free the variable. Such behaviour should be corrected in the following files : - Objects/abstract.c, line 1901 - Objects/stringobject.c, line 162 - getargs.c, line 66 - getargs.c, line 1188 - modsupport.c, line 479 |
|
|
| msg64250 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2008-03-21 18:26 |
| Can you provide a patch for 2.6 against the latest svn checkout of the trunk please? |
|
|
| msg64483 - (view) |
Author: Alexander Belopolsky (belopolsky) * |
Date: 2008-03-25 14:30 |
| This is not a bug. All the reported functions expect va_list argument to be initialized before being called. AFAICT, they are consistently used in this way. For example, PyObject * PyObject_CallFunctionObjArgs(PyObject *callable, ...) { PyObject *args, *tmp; va_list vargs; if (callable == NULL) return null_error(); /* count the args */ va_start(vargs, callable); args = objargs_mktuple(vargs); va_end(vargs); if (args == NULL) return NULL; tmp = PyObject_Call(callable, args, NULL); Py_DECREF(args); return tmp; } This may need to be clarified in the docs. For example, PyString_FromFormatV does not mention that vargs needs to be initialized: <http://docs.python.org/dev/c- api/string.html#PyString_FromFormatV>. On the other hand, this may be obvious to most C programmers. I suggest to close this issue as invalid. |
|
|
| msg64487 - (view) |
Author: Alexander Belopolsky (belopolsky) * |
Date: 2008-03-25 15:14 |
| On the second thought the macro dance highlighted by OP belongs to pyport.h. Attached patch defines Py_VA_COPY macro and uses it to simplify va_list copying code. |
|
|
| msg64500 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2008-03-25 17:23 |
| Looks like a good idea to me |
|
|
| msg64523 - (view) |
Author: Rolland Dudemaine (rolland) |
Date: 2008-03-25 21:40 |
| This is what I meant. The initialization should be done by calling va_start(count_va); as you described. In the files and lines I reported though, this is not called. I'll file a patch for it soon. --Rolland Dudemaine Alexander Belopolsky wrote: > Alexander Belopolsky <belopolsky@users.sourceforge.net> added the comment: > > This is not a bug. All the reported functions expect va_list argument > to be initialized before being called. AFAICT, they are consistently > used in this way. For example, > > PyObject * > PyObject_CallFunctionObjArgs(PyObject *callable, ...) > { > PyObject *args, *tmp; > va_list vargs; > > if (callable == NULL) > return null_error(); > > /* count the args */ > va_start(vargs, callable); > args = objargs_mktuple(vargs); > va_end(vargs); > if (args == NULL) > return NULL; > tmp = PyObject_Call(callable, args, NULL); > Py_DECREF(args); > > return tmp; > } > > This may need to be clarified in the docs. For example, PyString_FromFormatV does not mention that vargs needs to be > initialized: <http://docs.python.org/dev/c- > api/string.html#PyString_FromFormatV>. On the other hand, this may be > obvious to most C programmers. > > I suggest to close this issue as invalid. > > ---------- > nosy: +belopolsky > > __________________________________ > Tracker <report@bugs.python.org> > <http://bugs.python.org/issue2443> > __________________________________ > |
|
|
| msg64588 - (view) |
Author: Rolland Dudemaine (rolland) |
Date: 2008-03-27 13:05 |
| Actually, this thing is more complex to solve than I thought. Specifically, as described in http://www.opengroup.org/onlinepubs/007908775/xsh/stdarg.h.html stdarg requires that variable argument functions have at least one fixed argument. This is implied by the declaration of "void va_start(va_list ap, argN);". As explained in the original ticket description, and also described before in the above link, va_start() must be called before any call to va_arg(), and this includes any access to the argument list using __va_copy namely. The problem is that at least objargs_mktuple(), line 2649 of Objects/abstract.c does not have a first fixed argument. |
|
|
| msg70468 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-07-31 02:03 |
| What's the status of this? |
|
|
| msg84605 - (view) |
Author: Daniel Stutzbach (stutzbach) |
Date: 2009-03-30 18:28 |
| Rolland, The va_list is initialized by the function that calls objargs_mktuple. va_start() and va_end() need to be called in the function that takes "..." as a parameter, and it is. Not a bug, but +1 on Alexander's patch to consolidate all the #ifdef's for cleanliness. |
|
|
| msg88611 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2009-05-31 21:29 |
| Reducing the priority and updating the target releases, since from the discussion there doesn't appear to be a bug here. |
|
|
| msg99987 - (view) |
Author: Alexander Belopolsky (Alexander.Belopolsky) |
Date: 2010-02-24 01:27 |
| Updated the patch. In a year one more copy of va_copy macro dance have cropped in. Any reason this is still languishing? |
|
|
| msg99989 - (view) |
Author: Alexander Belopolsky (Alexander.Belopolsky) |
Date: 2010-02-24 01:33 |
| Rolland, Please don't revert the title. The consensus is that there is no uninitialized access to va_list. |
|
|
| msg113597 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-08-11 09:27 |
| Looks like a good idea. Don't other compilers have __va_copy equivalents? Apparently, C99 defines va_copy(), which we could use conditionally. |
|
|
| msg113608 - (view) |
Author: Alexander Belopolsky (belopolsky) * |
Date: 2010-08-11 16:56 |
| I updated the patch for 3.x. I agree that using va_copy where available makes sense, but let's leave this type of improvements for the future. |
|
|
| msg113635 - (view) |
Author: Alexander Belopolsky (belopolsky) * |
Date: 2010-08-11 22:12 |
| Committed in r83949. |
|
|