6758 – std.c.stdarg problems with 8 or more integer arguments on x86_64

When there are 8 or more integer (not float or double) arguments to a vararg function as well as string arguments va_arg() appears to get some wrong argument details.
This problem is present on both the D1 and D2 compilers.
Code works fine with 32 bit compilation. Only a problem with 64 bit compilation.
Example code attached. This might also seg fault in some circumstances.
This displays (see incorrect information for arguments 9 and 10 in the first section):
9 variable arguments
argument types [int, int, int, int, int, int, int, immutable(char)[], immutable(char)[]]
2) int arg = 2
3) int arg = 3
4) int arg = 4
5) int arg = 5
6) int arg = 6
7) int arg = 7
8) int arg = 8
9) string arg = '', length 0
10) string arg = '?', length 4764008
9 variable arguments
argument types [double, int, int, int, int, int, int, immutable(char)[], immutable(char)[]]
2) double arg = 2.000000
3) int arg = 3
4) int arg = 4
5) int arg = 5
6) int arg = 6
7) int arg = 7
8) int arg = 8
9) string arg = '9', length 1
10) string arg = '10', length 2
9 variable arguments
argument types [int, int, int, int, int, int, immutable(char)[], immutable(char)[], immutable(char)[]]
2) int arg = 2
3) int arg = 3
4) int arg = 4
5) int arg = 5
6) int arg = 6
7) int arg = 7
8) string arg = '8', length 1
9) string arg = '9', length 1
10) string arg = '10', length 2
9 variable arguments
argument types [immutable(char)[], int, int, int, int, int, int, immutable(char)[], immutable(char)[]]
2) string arg = '2', length 1
3) int arg = 3
4) int arg = 4
5) int arg = 5
6) int arg = 6
7) int arg = 7
8) int arg = 8
9) string arg = '9', length 1
10) string arg = '10', length 2

Created attachment 1030 [details] 
Example code for this problem

The quantity 8 (number of int or uint arguments) is probably not particularly significant. I have also seen seg faults on code with as few as 6 integer arguments mixed with strings and floats. 8 seems to be where the problem happens in the example code.

Doing a dump of the __va_argsave structure this looks like an alignment
issue. The compiler is aligning string arguments on 16 byte multiples,
so there is an 8 byte gap between the 8 (int) and "9" (string) arguments
of this example.
The stdarg module is not taking this alignment gap into account.
9 variable arguments
argument types [int, int, int, int, int, int, int, immutable(char)[], immutable(char)[]]
								Argument value:
regs:
00007FFF922CA110: AA 7F 46 00 00 00 00 00 01 00 00 00 00 00 00 00
00007FFF922CA120: 02 00 00 00 00 00 00 00 03 00 00 00 00 00 00 00 2 3
00007FFF922CA130: 04 00 00 00 00 00 00 00 05 00 00 00 00 00 00 00 4 5
fpregs:
00007FFF922CA140: 50 A4 2C 92 FF 7F 00 00 9D 94 41 00 00 00 00 00
00007FFF922CA150: 14 80 46 00 00 00 00 00 FF FF FF 7F FE FF FF FF
00007FFF922CA160: FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF 
00007FFF922CA170: FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF 
00007FFF922CA180: FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF 
00007FFF922CA190: FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF 
00007FFF922CA1A0: FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF 
00007FFF922CA1B0: FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF 
va:
00007FFF922CA1C0: 10 00 00 00 30 00 00 00 60 A2 2C 92 FF 7F 00 00
00007FFF922CA1D0: 10 A1 2C 92 FF 7F 00 00 04 00 00 80 00 00 00 00
00007FFF922CA1E0: 09 00 00 00 00 00 00 00 78 22 6A 00 00 00 00 00
00007FFF922CA1F0: C0 A1 2C 92 FF 7F 00 00 00 00 00 00 00 00 00 00
00007FFF922CA200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00007FFF922CA210: 00 00 00 00 3D 00 00 00 00 00 00 00 00 00 FC 7F
00007FFF922CA220: 28 05 00 2C 3D 00 00 00 40 0B 00 2C 3D 00 00 00
00007FFF922CA230: 68 3C C0 2A 3D 00 00 00 80 2A 6A 00 00 00 00 00
00007FFF922CA240: 01 00 00 00 3D 00 00 00 00 00 00 00 00 00 00 00
00007FFF922CA250: A0 A2 2C 92 FF 7F 00 00 E2 9B 44 00 00 00 00 00
00007FFF922CA260: 06 00 00 00 00 00 00 00 07 00 00 00 00 00 00 00 6 7
00007FFF922CA270: 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 8	(gap)
00007FFF922CA280: 01 00 00 00 00 00 00 00 A8 37 49 00 00 00 00 00 "9"
00007FFF922CA290: 02 00 00 00 00 00 00 00 C0 37 49 00 00 00 00 00 "10"
2) int arg = 2
3) int arg = 3
4) int arg = 4
5) int arg = 5
6) int arg = 6
7) int arg = 7
8) int arg = 8
9) string arg = '', length 0
10) string arg = '?', length 4798376
 0x4937A8 = 4798376

Changing every .alignof to .sizeof in the stdarg module appears to be sufficient to get this code to work in both D1 and D2. Whether this change is strictly correct for all cases I do not know.
D2
druntime/import/core/stdc
diff stdarg.di.original stdarg.di
91c91
< auto p = cast(size_t)ap.stack_args + T.alignof - 1 & ~(T.alignof - 1);
---
> auto p = cast(size_t)ap.stack_args + T.sizeof - 1 & ~(T.sizeof - 1);
122c122
< auto p = cast(size_t)ap.stack_args + T.alignof - 1 & ~(T.alignof - 1);
---
> auto p = cast(size_t)ap.stack_args + T.sizeof - 1 & ~(T.sizeof - 1);
D1
src/phobos/std/c
diff stdarg.d.original stdarg.d
123c123
< auto p = (cast(size_t)ap.stack_args + T.alignof - 1) & ~(T.alignof - 1);
---
> auto p = (cast(size_t)ap.stack_args + T.sizeof - 1) & ~(T.sizeof - 1);
152c152
< auto p = (cast(size_t)ap.stack_args + T.alignof - 1) & ~(T.alignof - 1);
---
> auto p = (cast(size_t)ap.stack_args + T.sizeof - 1) & ~(T.sizeof - 1);

As suspected that change would cause other argument types to no longer work, e.g. passing structures by value. It looks like a proper fix will require dynamic arrays to be treated separately from other argument types with additional static if statement(s). Can somebody suggest the best static if test to check if a type in a template is a dynamic array of any other type ?

Perhaps somebody can tell me if D-style variadic functions really should be using the __va_argsave structure on x86_64 code or should that only be applicable to C-style variadic functions (as the documentation still implies at the moment).
The compiler appears to be using the __va_argsave method of argument passing for both C and D variadic functions on x86_64 code. So it's only possible to use _argptr for retrieving arguments (as the documentation says) on 32 bit code.
I have managed to patch stdarg.d (D1) and stdarg.di (D2) to work with most D argument types but cfloat still gives some problems.
If this is a compiler issue (wrong argument passing method being used for D variadic functions) then I'll stop expending any more time on it.

This might be the same as issue 6983, but I'm not certain.

> Perhaps somebody can tell me if D-style variadic functions really should be
using the __va_argsave structure on x86_64 code or should that only be
applicable to C-style variadic functions (as the documentation still implies at
the moment).
Yes, they should. The documentation is completely wrong -- that's bug 7893.
Here's what's happening. In 32 bit code, the glue layer sends dynamic arrays { size_t length, void * ptr} into the backend as if they were integers, of type ulong. That's basically a hack.
In 64 bit, the hack was extended: dynamic arrays are of type ucent.
Also delegates are of type cent. These are the only two cases where you can create a variable of type cent or ucent.
Then, they get aligned *as if they were of type of cent or ucent* -- ie, 16 bytes, whereas they should only be aligned to 8 bytes.
I believe that inside structs, dynamic arrays and delegates are aligned to only 8 bytes, so we have an inconsistency.
Normally, you don't see this bug because: (1) if the number of arguments is small, they get passed in registers, so the alignment is ignored. There are 6 registers available for passing integer arguments (RDI RSI RDX RCX R8 R9), so the simplest case where this happens is 5 integers + a dynamic array. This is why the problem only shows up when a large number of variadic arguments are used;
(2) the other code generation also uses the cent/ucent alignment.
Workaround is to special-case delegates and arrays in stdc.stdarg. But really the compiler should be fixed.

*** Issue 7891 has been marked as a duplicate of this issue. ***

The problem is worse than I thought.
cod1.c, line 2450, cdfunc()
puts an array parameter into registers, instead of the stack, if there are enough free registers.
Nothing that fits into a register ever has additional stack alignment. So effectively, it is aligned as size_t.
ie, parameter[i].numalign = 0.
But, once the registers are full, it becomes treated as a ucent, and gets extra stack alignment.
parameter[i].numalign = 8;
Then, in line 2528, if it was on the stack, the code to align the stack (SUB RSP, numalign) is generated.
But, in 2564, this doesn't get happen for register parameters. Instead, the registers are just saved without any padding.
So it is NOT sufficient to adjust stdc.stdarg, to treat delegates and d arrays as if their alignof was ucent.alignof. Rather, they should be treated as sizeof.alignof, unless there are no registers left, in which case it should be ucent.alignof.
Perhaps it would be enough to set cod1.c line 2452: 
 // Parameter i goes on the stack
 parameters[i].reg = -1; // -1 means no register
- unsigned alignsize = el_alignsize(ep);
+ unsigned alignsize = ty64reg(ty) ? 0 : el_alignsize(ep);
 parameters[i].numalign = 0;
 if (alignsize > stackalign)
 
so that if it fits into a register, it is never aligned.

Commit pushed to dmd-1.x at https://github.com/D-Programming-Language/dmd
https://github.com/D-Programming-Language/dmd/commit/71c0d29d66c3fe946e1a714eef8098e6ad3dd373
fix Issue 6758 - std.c.stdarg problems with 8 or more integer arguments on x86_64

Commit pushed to master at https://github.com/D-Programming-Language/dmd
https://github.com/D-Programming-Language/dmd/commit/6b5485754868204358b1160b1a9515953d0ed6bc
fix Issue 6758 - std.c.stdarg problems with 8 or more integer arguments on x86_64

Commit pushed to master at https://github.com/D-Programming-Language/dmd
https://github.com/D-Programming-Language/dmd/commit/f87c43317a97b75c69639499a6445b82415739a3
add test case for Issue 6758