sccz80's varargs/stdarg support appears to be broken

[imneme]

Let's contrast zsdcc and sccz80 for code that uses a variable number of arguments. I get:

ubuntu@ubuntu:~/spectrum$ z88dk.zcc +cpm -compiler=sdcc vargs.c -O3 -o vargs.com && tnylpo vargs.com
a = 'Hello', b = 'World', 1, 2, 3, -1

ubuntu@ubuntu:~/spectrum$ z88dk.zcc +cpm vargs.c -O3 -o vargs.com && tnylpo vargs.com
vargs.c:7:28: warning: Converting integer type to pointer witout cast. From unsigned int  to char * b*  [-Wincompatible-pointer-types]
vargs.c:7:31: warning: Assigning 'vl', type: unsigned char * vl from char * b*  [-Wincompatible-pointer-types]
a = '', b = '', 1023, -254

I tried adding decorators like __smallc but it didn't seem to make a difference.

Tested with sccz80 17199-a5539b895-20210102.

#include <stdio.h>
#include <stdarg.h>

void vargs(char *a, char *b, ...)
{
    va_list vl;
    va_start(vl, b);
    printf("a = '%s', b = '%s'", a, b);
    while (1) {
        int varg = va_arg(vl, int);
        printf(", %d", varg);
        if (varg < 0)
            break;
    }
    printf("\n");
    va_end(vl);
}

int main()
{
    vargs("Hello", "World", 1, 2, 3, -1);
    return 0;
}

[suborb]

Please see https://github.com/z88dk/z88dk/issues/330#issuecomment-324791397

vaarg is difficult with l->r calling it seems.

[imneme]

Thanks, this is useful, although it #330 omits a rather key detail. The trick is to not only make sure __Z88DK_R2L_CALLING_CONVENTION is defined when including stdarg.h but also to declare varargs functions with __stdc.

Since normal stdarg.h behavior is hopelessly broken in left-to-right mode, why not have varadic functions default to right-to-left argument passing since that fixes the issue? Or, if you're really attached to the current breakage, how about warning people in the deficiencies section of the wiki?

[imneme]

An alternative to editing the code is to use right-to-left arg-passing globally. This works, allowing the above code to be run with:

ubuntu@ubuntu:~/spectrum$ z88dk.zcc +cpm vargs.c -set-r2l-by-default -D__Z88DK_R2L_CALLING_CONVENTION -O3 -o vargs.com && tnylpo vargs.com
a = 'Hello', b = 'World', 1, 2, 3, -1

But it would be nice if -set-r2l-by-default actually set __Z88DK_R2L_CALLING_CONVENTION.

[imneme]

Also, although the classic lib's stdarg.h has @aralbrec's __Z88DK_R2L_CALLING_CONVENTION fix, the version in newlib does not. Currently you can kludge around it with code like:

#define __SDCC
#undef __SCCZ80
#include <stdarg.h>

But obviously the right solution is the very modest fix of revising stdarg.h.

I've enclosed a patch for newlib's stdarg.h.

--- stdarg.h.orig   2021-01-03 15:40:20.227323981 -0800
+++ stdarg.h    2021-01-03 15:42:44.261635344 -0800
@@ -5,7 +5,7 @@
 #ifndef __STDARG_H__
 #define __STDARG_H__

-#ifdef __CLANG
+#if defined(__CLANG)

 typedef unsigned char * va_list;

@@ -14,9 +14,7 @@
 #define va_copy(dest, src)      { dest = src; }
 #define va_end(marker)          { marker = (va_list) 0; };

-#endif
-
-#ifdef __SDCC
+#elif defined(__SDCC) || defined(__Z88DK_R2L_CALLING_CONVENTION)

 // SDCC
 // r->l parameter passing means there are no issues
@@ -59,15 +57,15 @@

 #define va_ptr(marker, type)    *((type *)(marker - sizeof(type)))

-#endif
-
-#ifdef __SCCZ80
+#elif defined(__SCCZ80)

 // SCCZ80
 // l->r parameter passing means compiler must tell us how many params are on the stack

 // djm 28/2/2000

+#warning stdarg.h has many caveats when used in left-to-right mode.
+
 extern int __LIB__ getarg(void);

 #define va_list                 unsigned char *

[suborb]

Yes, something needs to be done with vaarg, switching vaarg to r->l is probably the easiest thing to do.

There's a few comments on the edge cases that it breaks here: https://github.com/z88dk/z88dk/issues/490#issuecomment-343118841 - mostly around function pointers.

Making any function that is prototyped with ellipses implicitly __stdc should be trivial, the printf/scanf families would need to be fixed up (not a huge job).

I suspect the major breakage would be around () function pointers - until a few years ago prototypes weren't supported on function pointers so there's a lot of code using that syntax around. Whether they then use variadic function pointers is doubtful of course. Given that stdarg comes up roughly once every 3 years(!) I suspect not.

There's probably going to be a "release snapshot" at the end of January/early February so I don't want to make a major switchover like that before then, but I'll cherry pick your suggestions above in the next few days.

[aralbrec]

I'll just add this is how it's handled in newlib now:

int m_printf(char *fmt, ...)
{
   va_list v;
   va_start(v, fmt);

   if (!flags.quiet)
   {
#ifdef __SCCZ80
      return vprintf(va_ptr(v,char *), v);
#else
      return vprintf(fmt, v);
#endif
   }

   return 0;
}

ie, special case code for sccz80. There is an additional restriction in that va_start has to be executed immediately inside the function; I am not even sure if it works if local variables are declared in the function. L->R convention for vararg is painful and the transition to R->L is not without problems as listed above but maybe sccz80 has enough tools now to make that possible.

Changing to R->L also involves updating all the vararg functions in the library like printf, scanf, etc. That might not be too terrible as the existing sdcc code might be applicable.