Type Casts for varargs Arguments

[obgm]

As mentioned in PR #217, I see potential issues with type casts for arguments that are passed to __VA_ARGS__ functions such as printf. The following example illustrates how the length argument of dtls_debug_hexdump is passed to printf in the RIOTOS-version of tinydtls:

#include <stdio.h>
#define log_write(level, ...) printf(__VA_ARGS__)

enum { LOG_DEBUG };

#define LOG(level, ...) do {                            \
        log_write((level), __VA_ARGS__); } while (0U)

#define LOG_DEBUG(...) LOG(LOG_DEBUG, __VA_ARGS__)

#define dtls_debug_hexdump(name, buf, length) { LOG_DEBUG("(%zu bytes)", (size_t)(length));}

int main(void) {
  unsigned short sz = 1 << 15;
  dtls_debug_hexdump("", "", sz);
}

What happens in a function using __VA_ARGS__ is that the passed arguments are retrieved using va_args that is described for C99 in clause 7.15.1.1 of ISO/IEC 9899:1999 (and pretty much the same for C11):

7.15.1.1 The va_arg macro
Synopsis
  #include <stdarg.h>
  type va_arg(va_list ap, type);

Description
  [...]
  The parameter type shall be a type name specified such that the type of a pointer to an object that has the specified type can be obtained simply by postfixing a * to type.

The problem now is that, e.g., the newlib implementation for printf selects a field from a union with a suitable type based on sizeof(size_t)—from (unsigned) short to unsigned long (or even unsigned long long if supported).

So, in the example above, the following lines

  unsigned short sz = 1 << 15;
  dtls_debug_hexdump("", "", sz);

could pretty much cause this:

#define u_quad_t unsigned long
 u_quad_t _uquad = va_arg (..., u_quad_t);

which, from the description of va_args is semantically equivalent with

  unsigned long _uquad = *(unsigned long *)(&sz);

Depending on the architecture, this could lead to runtime errors caused by, e.g. accessing illegal addresses or invalid alignment. Unfortunately, it is difficult to construct this situation deliberately, especially on desktop systems rather than embedded platforms.

[boaks]

There is a huge difference in

int a = 1;
f1(size_t)a);

and

int* a = 1;
f2(size_t*)a);

The first just copies the value of a as size_t "on the callstack", the second a pointer to a wrong type. We use the first (cast the value) and no pointer.

So, if we call the variadic function (f(...)) with: (f(...) must be f(type r, ...) but for simplicity, the named parameter is left out):

unsigned short sz = 1 << 15;
f(sz);

sz will be promoted to unsigned int (see GCC- A.2.2.4 Calling Variadic Functions). So that actual argument will fill the space of an "unsigned int".

if now that argument is accessed via:

u_quad_t _uquad = va_arg (..., u_quad_t);

then the pointer to "unsigned int" is casted into an pointer of "unsigned long" and that may be an violation, because the expected type differs from the type of the actual provided (and promoted) value.

But if that value is casted when passed in,

unsigned short sz = 1 << 15;
f((unsigned long)sz);

Then the actual argument will fill the space of an "unsigned long" and the later access works.

Summary:

I still see, that we need to ensure, that the passed argument is of the expected type. And the very best way to do this is the cast exactly where it is passed in, therefore

LOG_DBG("%s (%zu bytes):", name, (size_t)(length))  

"%zu" expects size_t and (size_t)(length) copies the length as size_t on the callstack. Perfect solution.

[boaks]

A.2.2.5 Argument Access Macros

States explicit for va_arg:

The type of the value returned by va_arg is type as specified in the call. type must be a self-promoting type (not char or short int > or float) that matches the type of the actual argument.

If printf reads %zu with va_arg(list, size_t), then the actual argument must be passed in as size_t.

The main question seems to be, if the arguments passed in to the optional arguments are passed in "by value" or something else may happen. I only see the "by value".

[boaks]

Do variadic functions need to have a different calling convention to regular functions?

None of the comments points to something, that indicates, the arguments are not passed "by value".

[boaks]

I also don't see, that the preprocessing stage will cause issues with the variadic macro and passing in a casted argument.

On the preprocessor macro expansion level, there is no va_arg, even if that is a macro on it's own. For the preprocessor macro expansion, __VA_ARGS__ is replaced literal by the arguments passed to the macro.

So, also here, I don't see, that anything else happens then passing the arguments "by value". In the macro expansion there is nothing as va_arg, which then uses the type and assumptions about the pointer to that type. va_arg is only used in the implementation of the variadic function, not the preprocessor macro.

[boaks]

I think, we can close this one, or?