Define Py_VA_COPY macro as a cross-platform replacement for gcc __va_copy

@abalkin

BPO	2443
Nosy	@abalkin, @pitrou, @tiran, @benjaminp, @bitdancer
Files	issue2443.diff: Patch against revision 61893 issue2443a.diff: Patch against revision 78398 issue2443-py3k.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/abalkin'
closed_at = <Date 2010-08-11.22:12:59.423>
created_at = <Date 2008-03-21.10:27:37.295>
labels = ['interpreter-core', 'type-feature']
title = 'Define Py_VA_COPY macro as a cross-platform replacement for gcc __va_copy'
updated_at = <Date 2010-08-11.22:12:59.422>
user = 'https://bugs.python.org/rolland'

bugs.python.org fields:

activity = <Date 2010-08-11.22:12:59.422>
actor = 'belopolsky'
assignee = 'belopolsky'
closed = True
closed_date = <Date 2010-08-11.22:12:59.423>
closer = 'belopolsky'
components = ['Interpreter Core']
creation = <Date 2008-03-21.10:27:37.295>
creator = 'rolland'
dependencies = []
files = ['9849', '16349', '18477']
hgrepos = []
issue_num = 2443
keywords = ['patch']
message_count = 15.0
messages = ['64234', '64250', '64483', '64487', '64500', '64523', '64588', '70468', '84605', '88611', '99987', '99989', '113597', '113608', '113635']
nosy_count = 7.0
nosy_names = ['belopolsky', 'pitrou', 'christian.heimes', 'benjamin.peterson', 'stutzbach', 'rolland', 'r.david.murray']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue2443'
versions = ['Python 3.2']

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

Can you provide a patch for 2.6 against the latest svn checkout of the
trunk please?

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.

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.

Looks like a good idea to me

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 <[email protected]> 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 <[email protected]>
<http://bugs.python.org/issue2443\>

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.

What's the status of this?

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.

Reducing the priority and updating the target releases, since from the
discussion there doesn't appear to be a bug here.

Updated the patch. In a year one more copy of va_copy macro dance have cropped in. Any reason this is still languishing?

Rolland,

Please don't revert the title. The consensus is that there is no uninitialized access to va_list.

Looks like a good idea. Don't other compilers have __va_copy equivalents?
Apparently, C99 defines va_copy(), which we could use conditionally.

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.

Committed in r83949.

Navigation Menu

Repository navigation

Define Py_VA_COPY macro as a cross-platform replacement for gcc __va_copy #46695

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Define Py_VA_COPY macro as a cross-platform replacement for gcc __va_copy #46695

Description

Activity

rolland commented on Mar 21, 2008

tiran commented on Mar 21, 2008

abalkin commented on Mar 25, 2008

abalkin commented on Mar 25, 2008

tiran commented on Mar 25, 2008

rolland commented on Mar 25, 2008

rolland commented on Mar 27, 2008

benjaminp commented on Jul 31, 2008

stutzbach commented on Mar 30, 2009

bitdancer commented on May 31, 2009

AlexanderBelopolsky commented on Feb 24, 2010

AlexanderBelopolsky commented on Feb 24, 2010

pitrou commented on Aug 11, 2010

abalkin commented on Aug 11, 2010

abalkin commented on Aug 11, 2010

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions