Unexpected behavior of va_list when used in multiple functions

Question

In my project I'm making a C class hierarchy similar to, but not the same as Axel-Tobias Schreiner Object-oriented Programming in ansi C see https://www.cs.rit.edu/~ats/books/ooc.pdf. for example.

I'm initializing my object a little bit different than Axel is. I'm running into troubles when I'm passing va_list objects between multiple initializing functions. Say I've an object two, that derives from object one. Then while initializing a two object, I'll need to initialize the one part first, and subsequently initialize the two part. So I'm having 1 init function that calls the public initializer function with arguments that initialize the one part of the two object and arguments that only initializes the two part that extends one object.

The library I'm creating is quite large, but I've extracted a mini project from it that demonstrates the same issues:

CMakeLists.txt:

cmake_minimum_required(VERSION 3.5)

project (var_args
        VERSION     "0.0"
        LANGUAGES   C CXX
        )

set(HEADERS "init.h")
set(SOURCES init.c program.c)

add_executable(program ${SOURCES} ${HEADERS})

if (NOT MSVC)
    target_compile_options(program PRIVATE -W -Wall -Wextra -pedantic)
else()
    target_compile_options(program PRIVATE /W4)
endif()

init.h:

typedef struct _one {
    int a;
    const char* msg;
} one_t;

/* this struct "derives" from struct _one */
typedef struct _two {
    one_t   parent;
    double  pi;
    double  e;
}two_t;

enum status_t {
    STATUS_SUCCES,
    STATUS_INVALID_ARGUMENT,
    STATUS_ERROR
};

enum init_one_flags {
    INIT_ONE_A,         // 2nd argument should be of type int
    INIT_ONE_MSG,       // 3rd argument should be const char*
    INIT_ONE_FINISHED,  // takes no arugment, but rather tell init1 should be finished.
    INIT_ONE_SENTINAL   // Keep at the last position.
};

enum init_two_flags {
    INIT_TWO_PI = INIT_ONE_SENTINAL,    // 2nd arugument should be of type double.
    INIT_TWO_E,                         // 2nd argument shoudl be double.
    INIT_TWO_FINISHED,                  // takes no arugment, but rather tell init2 should be finished.
    INIT_TWO_SENTINAL,                  // for init3...
};

#ifdef __cplusplus
extern "C" {
#endif

int init_two(two_t* two, ...);

//void init_one(one_t* one, ...);

#ifdef __cplusplus
}
#endif

init.c:

#include <stdarg.h>
#include "init.h"

static int priv_init1(one_t* one, va_list list)
{
    // use default values;
    int a = 0;
    const char* msg = "";

    int selector, ret = STATUS_SUCCES;

    while ((selector = va_arg(list, int)) != INIT_ONE_FINISHED) {
        switch (selector) {
        case INIT_ONE_A:
            a = va_arg(list, int);
            break;
        case INIT_ONE_MSG:
            msg = va_arg(list, const char*);
            break;
        default:
            // unknown argument
            return STATUS_INVALID_ARGUMENT;
        }
    }

    one->a = a;
    one->msg = msg;
    return ret;
}

static int priv_init2(two_t* two, va_list list)
{
    double pi = 3.1415, e=2.7128;
    int selector, ret = STATUS_SUCCES;

    ret = priv_init1((one_t*)two, list);
    if (ret)
        return ret;

    while ((selector = va_arg(list, int)) != INIT_TWO_FINISHED) {
        switch (selector) {
        case INIT_TWO_PI:
            pi = va_arg(list, double);
            break;
        case INIT_TWO_E:
            pi = va_arg(list, double);
            break;
        default:
            return STATUS_INVALID_ARGUMENT;
        }
    }

    two->pi = pi;
    two->e = e;

    return STATUS_SUCCES;
}

int init_two(two_t* two, ...)
{
    int ret;
    va_list list;
    va_start(list, two);
    ret = priv_init2(two, list);
    va_end(list);

    return ret;
}

program.c:

#include <stdio.h>
#include "init.h"

int main() {
    int ret;
    two_t two;

    ret = init_two(
        &two,
        INIT_ONE_A,         1,
        INIT_ONE_MSG,       "Hello, world",
        INIT_ONE_FINISHED,
        INIT_TWO_PI,        2 * 3.1415,
        INIT_TWO_FINISHED
    );

    if (ret) {
        fprintf(stderr, "unable to init two...\n");
        printf("a=%d\tmsg=%s\tpi=%lf\te%lf\n",
            two.parent.a,
            two.parent.msg,
            two.pi,
            two.e
        );
        return 1;
    }
    else {
        printf("a=%d\tmsg=%s\tpi=%lf\te%lf\n",
            two.parent.a,
            two.parent.msg,
            two.pi,
            two.e
        );
        return 0;
    }
}

Now the problem I'm running into is that the behavior of this code is exactly what I expect on Linux with gcc or clang in debug and release builds. Unfortunately the code fails on Windows with visual studio 17.

So the output of the program should be someting like:

a=1 msg=Hello, world pi=6.283000 e2.712800

And that is exactly what I obtain on Linux with gcc (5.4.0-6)

On windows I get:

a=1 msg=Hello, world pi=jiberish here e2=jiberish here.

And the function init_two does return that the function was successful on Linux and that is is not successful on windows. Also I can see that the part of one_t part of two is initialized successfully, whereas the two_t part is not. I would be very grateful if someone would point out the issue what is going wrong. Is the va_list is passed by reference on Linux, whereas the va_list is passed by value on windows? Are the enum values promoted to int on Linux, whereas they are passed as char on windows? Finally, I tagged the question as C and C++ because I know the code I demonstrate is C, but I would like it to work with a C++ compiler as well.

Answer 1

The implementation of va_list can differ greatly across compilers.

It's possible, for example, that gcc implements it as a pointer so passing it to another function by value still modifies the underlying structure so that changes in the called function are visible in the calling function, while MSVC implements it as a struct so changes in the calling function are not visible in the caller.

You can get around this by passing a pointer to your initial va_list instance to all the functions that need it. Then the internal state will be consistent in all functions.

// use pointer to va_list
static int priv_init1(one_t* one, va_list *list)
{
    // use default values;
    int a = 0;
    const char* msg = "";

    int selector, ret = STATUS_SUCCES;

    while ((selector = va_arg(*list, int)) != INIT_ONE_FINISHED) {
        switch (selector) {
        case INIT_ONE_A:
            a = va_arg(*list, int);
            break;
        case INIT_ONE_MSG:
            msg = va_arg(*list, const char*);
            break;
        default:
            // unknown argument
            return STATUS_INVALID_ARGUMENT;
        }
    }

    one->a = a;
    one->msg = msg;

    return ret;
}

// use pointer to va_list
static int priv_init2(two_t* two, va_list *list)
{
    double pi = 3.1415, e=2.7128;
    int selector, ret = STATUS_SUCCES;

    ret = priv_init1((one_t*)two, list);
    if (ret)
        return ret;

    while ((selector = va_arg(*list, int)) != INIT_TWO_FINISHED) {
        switch (selector) {
        case INIT_TWO_PI:
            pi = va_arg(*list, double);
            break;
        case INIT_TWO_E:
            pi = va_arg(*list, double);
            break;
        default:
            return STATUS_INVALID_ARGUMENT;
        }
    }

    two->pi = pi;
    two->e = e;

    return STATUS_SUCCES;
}

int init_two(two_t* two, ...)
{
    int ret;
    va_list list;
    va_start(list, two);
    ret = priv_init2(two, &list);    // pass pointer
    va_end(list);

    return ret;
}

This usage is explicitly mentioned in section 7.16p3 of the C standard:

The type declared is

va_list

which is a complete object type suitable for holding information needed by the macros va_start , va_arg , va_end , and va_copy . If access to the varying arguments is desired, the called function shall declare an object (generally referred to as ap in this subclause) having type va_list. The object ap may be passed as an argument to another function; if that function invokes the va_arg macro with parameter ap , the value of ap in the calling function is indeterminate and shall be passed to the va_end macro prior to any further reference to ap. ²⁵³⁾

253) It is permitted to create a pointer to a va_list and pass that pointer to another function, in which case the original function may make further use of the original list after the other function returns.

Answer 2

This should work with any compiler:

static int priv_init1(one_t* one, va_list* list)
{
    // use default values;
    int a = 0;
    const char* msg = "";

    int selector, ret = STATUS_SUCCES;

    while ((selector = va_arg(*list, int)) != INIT_ONE_FINISHED) {
        switch (selector) {
        case INIT_ONE_A:
            a = va_arg(*list, int);
            break;
        case INIT_ONE_MSG:
            msg = va_arg(*list, const char*);
            break;
        default:
            // unknown argument
            return STATUS_INVALID_ARGUMENT;
        }
    }

    one->a = a;
    one->msg = msg;
    return ret;
}

static int priv_init2(two_t* two, va_list list)
{
    double pi = 3.1415, e=2.7128;
    int selector, ret = STATUS_SUCCES;

    ret = priv_init1((one_t*)two, &list);
    if (ret)
        return ret;

    while ((selector = va_arg(list, int)) != INIT_TWO_FINISHED) {
        switch (selector) {
        case INIT_TWO_PI:
            pi = va_arg(list, double);
            break;
        case INIT_TWO_E:
            pi = va_arg(list, double);
            break;
        default:
            return STATUS_INVALID_ARGUMENT;
        }
    }

    two->pi = pi;
    two->e = e;

    return STATUS_SUCCES;
}

Anyway your design that va_list is increased by two diffrent functions looks very fragile. I wouldn't do that.

Also you API design requires specific and complex sequnce of arguments which compilers do not recognize. This is so bug prone that I would not pass this in code review.

Since this looks like XY problem. I recommend creating another question about your X problem (how to design API which does what you need).

Answer 3

As I understand it, the issue revolves around the fact that your implementation of inheritance requires the initialization function for a subclass to call the initialization function(s) of its superclass(es). This is combined with the fact that the user-facing initialization functions are declared as varargs functions. To make this work, the varargs functions serve as front ends for internal functions that accept the variable arguments via single parameters of type va_list.

This can be made to work in standard C, but you must observe some restrictions. Among those:

• Your varargs functions must each initialize their va_list via the va_start macro, exactly once.

• Your functions may pass objects of type va_list to other functions, but the caller may not afterward use that object unless they can be confident that the called function did not use either va_arg nor va_end on it, or unless they themselves first use va_end and then va_start on it.

• Every use of va_start or va_copy must be paired with a corresponding use of va_end on the same va_list

• Of course, it is up to you to ensure that the correct data types are specified to each use of va_arg.

You violate the second requirement by passing a va_list from one internal initialization function to another, and then afterward using it.

You should be able to work around this by passing pointers to the va_list and accessing it indirectly, so that all the functions involved use the same object. This will enable the state of the va_list to be shared, so that, for example, the initialization function for the topmost class can effectively consume the arguments intended for it, without its subclasses needing to know their number or types. That would take a form something like this:

static int priv_init1(one_t* one, va_list *list) {
    while ((selector = va_arg((*list), int)) != INIT_ONE_FINISHED) {
        // ...
    }

    // ...
    return ret;
}

static int priv_init2(two_t* two, va_list *list) {
    int ret = priv_init1((one_t*)two, list);

    // ...
    while ((selector = va_arg((*list), int)) != INIT_TWO_FINISHED) {
        // ...
    }

    // ...    
    return STATUS_SUCCES;
}

int init_two(two_t* two, ...) {
    int ret;
    va_list list;

    va_start(list, two);
    ret = priv_init2(two, &list);
    va_end(list);

    return ret;
}