Bug 5494 – [patch,enh] Issues with std.stdarg

Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P3
Component
phobos
Product
D
Version
D2
Platform
Other
OS
Windows
Creation time
2011-01-26T23:56:00Z
Last change time
2015-01-26T12:39:30Z
Keywords
patch
Assigned to
nobody
Creator
sandford

Comments

Comment #0 by sandford — 2011-01-26T23:56:47Z
Currently, std.stdarg doesn't appear to be 64-bit safe (see Issue 4310) is extremely limited in functionality, and although mentioned in passing in the D-style Variadic Functions documentation, it isn't listed as a Phobos module. With regard to functionality, std.stdarg contains a single function ("va_arg") which interprets _argptr as a pointer to type T, returns a T and advances _argptr by T.sizeof adjusted to 32-bit alignment (see Issue 4310). This is unsafe, as _argptr may not actually be T* and therefore will be mis-aligned post va_arg. Given that D provides type safe Variadic Functions via the _arguments TypeInfo[], a better solution whould be to use the correct TypeInfo.tsize when advancing _argptr. Further more, there are use cases (std.boxer/std.variant, etc) which work with void*/TypeInfo directly and must currently maintain correct stack alignment manually. As a solution to these issues, I'm proposing the VariadicArguments range listed below. Note that switching the assert in get to enforce should allow all methods to be @trusted or @safe. /** Iterates a set of D-style variadic function arguments in a safe manner. * Example: * -------- void foo(...) { auto va_args = VariadicArguments(_arguments,_argptr) ; assert(va_args.length == 6); assert(va_args.get!uint == 1); va_args.popFront; assert(va_args.get!int == 2); va_args.popFront; assert(va_args.get!long == 3); va_args.popFront; assert(va_args.get!float == 4); va_args.popFront; assert(va_args.get!double == 5); va_args.popFront; assert(va_args.get!real == 6); va_args.popFront; } foo(1u,2,3L,4f,5.0,6.0L); * -------- */ struct VariadicArguments { private TypeInfo[] args; private void* ptr; this(TypeInfo[] arguments, void* argptr) { args = arguments; ptr = argptr; } /// Returns: true if there are no more arguments bool empty() const nothrow { return args.empty; } /// Returns: a copy of this. VariadicArguments save() nothrow { return this; } const(VariadicArguments) save() const nothrow { return this; } ///ditto /// Advances to the next argument void popFront() { ptr = ptr + ((args.front.tsize + size_t.sizeof - 1) & ~(size_t.sizeof - 1)); args.popFront; } /// Returns: a tuple of an untyped pointer to the current argument and it's TypeInfo. Tuple!(void*,TypeInfo) front() { return tuple(ptr,args.front); } /// Returns: the number of arguments size_t length() const nothrow { return args.length; } /// Returns: the current argument interpreted as type T T get(T)() nothrow { assert(typeid(T).toString() == args.front.toString(), "VariadicArguments.get: mis-matching types: "~typeid(T).toString~" vs. "~args.front.toString); return *cast(T*)ptr; } } unittest { void foo(...) { auto va_args = VariadicArguments(_arguments,_argptr) ; assert(va_args.length == 6); assert(va_args.get!uint == 1); va_args.popFront; assert(va_args.get!int == 2); va_args.popFront; assert(va_args.get!long == 3); va_args.popFront; assert(va_args.get!float == 4); va_args.popFront; assert(va_args.get!double == 5); va_args.popFront; assert(va_args.get!real == 6); va_args.popFront; } foo(1u,2,3L,4f,5.0,6.0L); }
Comment #1 by braddr — 2011-02-07T00:22:39Z
I think this one can be closed with the next release. druntime's vararg support has been updated to support the x86-64 c abi.
Comment #2 by sandford — 2011-02-07T09:22:50Z
Well, I think Issue 4310 can be closed, so long as std.stdarg gets depreciated in favor of core.stdarg. There is an API mismatch between the 32-bit and 64-bit core.stdarg, which should be fixed (I'd suggest making core.stdarg simply forward to core.stdc.stdarg, which is correct). Also, I think the idea of a range which wraps _arguments, _argptr and provides a simple and safe way to use variadic arguments would be a useful enhancement to dRuntime.
Comment #3 by andy — 2015-01-25T01:07:36Z
I'm gonna guess we can close this: Original report is for std.stdarg which is gone. std.c.stdarg forwards to core.stdc.stdarg Last comment: I'd suggest making core.stdarg simply forward to core.stdc.stdarg, which is correct Which seems to have happened. However there is no bug code so I cannot test core.stdc.stdarg I'd vote close this, and if its still a problem then a new report is filed with current names and some test code.