variadic arguments not thread safe on amd64?

variadic arguments not thread safe on amd64?

[Matt Provost]

I've been trying to write a program with a logging thread that will
consume messages in 'printf format' passed via a struct. It seemed
that this should be possible using va_copy to copy the variadic
arguments but they would always come out as garbage. This is with gcc
4.1.2 on amd64. Reading through the amd64 ABI it's now clear that the
va_list is just a struct and the actual values are stored in
registers. So I imagine that when it switches threads the registers
are restored and the va_list isn't valid anymore. But I can't find any
documentation about whether the va_* macros were ever supposed to be
thread safe. It seems that they probably are everywhere except PPC and
amd64.

Is there a portable way to pass a va_list between threads?

Here's an example program, if you compile it on a 32 bit machine (or
even with -m32) it prints out both strings ok, but on amd64 it will
print nulls for the threaded case.

$ gcc -m64 -g -lpthread test.c
$ ./a.out hello world
debug: hello world
tdebug: hello world
$ gcc -m64 -g -lpthread test.c
$ ./a.out hello world
debug: hello world
tdebug: (null) (null)


#include <stdarg.h>
#include <pthread.h>

typedef struct log_s {
    const char *format;
    va_list ap;
} log_t;

log_t mylog;
pthread_mutex_t m;
pthread_cond_t c;

void printlog() {
    vprintf(mylog.format, mylog.ap);
}

void *tprintlog() {
    pthread_mutex_lock(&m);
    pthread_cond_wait(&c, &m);
    vprintf(mylog.format, mylog.ap);
    pthread_mutex_unlock(&m);
}

void debug(const char *format, ...) {
    va_list ap;
    mylog.format = format;
    va_start(ap, format);
    va_copy(mylog.ap, ap);
    printlog();
    va_end(ap);
}

void tdebug(const char *format, ...) {
    va_list ap;
    pthread_mutex_lock(&m);
    mylog.format = format;
    va_start(ap, format);
    va_copy(mylog.ap, ap);
    pthread_cond_signal(&c);
    pthread_mutex_unlock(&m);
}

int main(int argc, char *argv[]) {
    pthread_t t;

    debug("debug: %s %s\n", argv[1], argv[2]);

    pthread_mutex_init(&m, NULL);
    pthread_cond_init(&c, NULL);
    pthread_create(&t, NULL, tprintlog, NULL);

    sleep(1);

    tdebug("tdebug: %s %s\n", argv[1], argv[2]);

    sleep(1);
}

Re: variadic arguments not thread safe on amd64?

[Andrew Pinski]

On Mon, Apr 27, 2009 at 8:37 PM, Matt Provost <mprovost@termcap.net> wrote:
> void tdebug(const char *format, ...) {
>    va_list ap;
>    pthread_mutex_lock(&m);
>    mylog.format = format;
>    va_start(ap, format);
>    va_copy(mylog.ap, ap);
>    pthread_cond_signal(&c);
>    pthread_mutex_unlock(&m);

You are missing two va_end's here

-- Pinski

Re: variadic arguments not thread safe on amd64?

[Matt Provost]

On Mon, Apr 27, 2009 at 08:49:27PM -0700, Andrew Pinski wrote:
> On Mon, Apr 27, 2009 at 8:37 PM, Matt Provost <mprovost@termcap.net> wrote:
> > void tdebug(const char *format, ...) {
> > ?? ??va_list ap;
> > ?? ??pthread_mutex_lock(&m);
> > ?? ??mylog.format = format;
> > ?? ??va_start(ap, format);
> > ?? ??va_copy(mylog.ap, ap);
> > ?? ??pthread_cond_signal(&c);
> > ?? ??pthread_mutex_unlock(&m);
> 
> You are missing two va_end's here
> 

Yes I had a question about va_end in this situation. Putting one that
clears 'ap' seems fine but doesn't change anything. But if you va_end
the copy that you put in the struct, then what happens when the other
thread goes to use it? Or should the va_end for that be in the tprintlog
function after it's done with it?

In any case none of those combinations seem to affect the output.

Thanks,
Matt

Re: variadic arguments not thread safe on amd64?

[H.J. Lu]

On Mon, Apr 27, 2009 at 9:00 PM, Matt Provost <mprovost@termcap.net> wrote:
> On Mon, Apr 27, 2009 at 08:49:27PM -0700, Andrew Pinski wrote:
>> On Mon, Apr 27, 2009 at 8:37 PM, Matt Provost <mprovost@termcap.net> wrote:
>> > void tdebug(const char *format, ...) {
>> > ?? ??va_list ap;
>> > ?? ??pthread_mutex_lock(&m);
>> > ?? ??mylog.format = format;
>> > ?? ??va_start(ap, format);
>> > ?? ??va_copy(mylog.ap, ap);
>> > ?? ??pthread_cond_signal(&c);
>> > ?? ??pthread_mutex_unlock(&m);
>>
>> You are missing two va_end's here
>>
>
> Yes I had a question about va_end in this situation. Putting one that
> clears 'ap' seems fine but doesn't change anything. But if you va_end
> the copy that you put in the struct, then what happens when the other
> thread goes to use it? Or should the va_end for that be in the tprintlog
> function after it's done with it?
>
> In any case none of those combinations seem to affect the output.
>

va_list points to stack memory. Are you passing pointer to stack memory
from one thread to another? How is this behavior defined?


-- 
H.J.

Re: variadic arguments not thread safe on amd64?

[Ian Lance Taylor]

Matt Provost <mprovost@termcap.net> writes:

> Is there a portable way to pass a va_list between threads?

This question would be more appropriate for the gcc-help@gcc.gnu.org
mailing list.

The answer is no: you can not pass va_list values between threads.  If
you think about it you will realize that the va_list value can not hold
the complete set of variable arguments, as there is no way to know how
many there are.  Therefore, if you pass enough variable arguments to a
function, it is guaranteed that some will be on the stack.  You can not
reliably pass a reference to the stack of one thread to another thread,
because the first thread might exit before the second thread runs.

Ian