Issue 21974 - ImportC: support __builtin_va_list, __builtin_va_start, __builtin_va_arg, __builtin_va_end
Summary: ImportC: support __builtin_va_list, __builtin_va_start, __builtin_va_arg, __b...
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P1 normal
Assignee: No Owner
URL:
Keywords: ImportC, rejects-valid
: 22307 (view as issue list)
Depends on:
Blocks:
 
Reported: 2021-05-25 17:29 UTC by Iain Buclaw
Modified: 2022-01-22 01:30 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Iain Buclaw 2021-05-25 17:29:56 UTC
On C compilers, the library never defines the va_list type itself, the compiler always provides the full definition.

In D, we already generate the va_list type internally (Target.tvalist), this can be exposed to importC as a `__builtin_valist` keyword to support stdio.h and stdarg.h, which have these definitions:
---
typedef __builtin_va_list va_list;
typedef __builtin_va_list __gnuc_va_list;
---
Comment 1 Iain Buclaw 2021-05-25 17:39:51 UTC
On a note of hindsight, the original location of tvalist being in Type was in-fact a good thing, and moving the type to Target will result in a bit of manoeuvring to keep the dmd.cparse happily isolated.
Comment 2 Iain Buclaw 2021-07-05 18:15:47 UTC
Current workaround is to add `typedef` declarations in the "wrapper" sources.
Comment 3 Walter Bright 2021-07-16 08:47:36 UTC
What is the typedef you've added to the wrapper sources?
Comment 4 Walter Bright 2021-07-16 09:14:14 UTC
As you say, it's problematic to add the __builtin_va_list to cparse, as cparse does not have access to the types.

A simple approach is to have the semantic routines start by adding the declaration for it, rather than cparse.
Comment 5 Iain Buclaw 2021-07-16 09:40:35 UTC
(In reply to Walter Bright from comment #3)
> What is the typedef you've added to the wrapper sources?
For testing purposes.
---
/* Provide user declarations of the built-in va_list type.  */
#if defined(__i386__)
typedef char* __builtin_va_list;
#elif defined(__x86_64__)
typedef struct
{
    unsigned gp_offset;
    unsigned fp_offset;
    void *overflow_arg_area;
    void *reg_save_area;
} __builtin_va_list[1];
#else
#error "unsupported"
#endif
---

It's not the only `__builtin_` that I've written something to provide for.
e.g:
---
#define __builtin_bswap16(x) \
    ((u_int16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8)))
#define __builtin_bswap32(x) \
    ((((x) & 0xff000000u) >> 24) | (((x) & 0x00ff0000u) >> 8) \
     | (((x) & 0x0000ff00u) << 8) | (((x) & 0x000000ffu) << 24))
#define __builtin_bswap64(x) \
    ((((x) & 0xff00000000000000ull) >> 56) \
     | (((x) & 0x00ff000000000000ull) >> 40) \
     | (((x) & 0x0000ff0000000000ull) >> 24) \
     | (((x) & 0x000000ff00000000ull) >> 8) \
     | (((x) & 0x00000000ff000000ull) << 8) \
     | (((x) & 0x0000000000ff0000ull) << 24) \
     | (((x) & 0x000000000000ff00ull) << 40) \
     | (((x) & 0x00000000000000ffull) << 56))
---

I'm currently on the fence really whether we should open the gates for handling `__builtin_` symbols.
Comment 6 Walter Bright 2021-07-17 06:47:46 UTC
Those last three look like they were added before gcc could handle inline functions.
Comment 7 Walter Bright 2021-07-17 07:09:57 UTC
One theory is if a declaration can be supplied by the library or some source file, it should *not* be built in to the compiler. This:

1. reduces the complexity of the compiler
2. makes it editable by the user
3. means updates can be done without altering the compiler
4. makes it unnecessary to get it to match every version of every C compiler out there

We do this in D by automatically importing object.d.

I already believe we'll have to supply such a file anyway simply to deal with all the 400 predefines that gcc generates.
Comment 8 Iain Buclaw 2021-07-18 20:42:26 UTC
(In reply to Walter Bright from comment #6)
> Those last three look like they were added before gcc could handle inline
> functions.
More like, it existed before gcc recognized bswap as an intrinsic.

The original code from bits/byteswap.h:
---
static __inline __uint32_t
__bswap_32 (__uint32_t __bsx)
{
#if __GNUC_PREREQ (4, 3)
  return __builtin_bswap32 (__bsx);
#else
  return __bswap_constant_32 (__bsx);
#endif
}
---
Comment 9 Walter Bright 2021-09-28 06:19:46 UTC
These also need to be added per https://issues.dlang.org/show_bug.cgi?id=22307 :

__builtin_va_start
__builtin_va_arg
__builtin_va_end
Comment 10 Walter Bright 2021-09-28 06:20:56 UTC
*** Issue 22307 has been marked as a duplicate of this issue. ***
Comment 11 Walter Bright 2021-09-28 06:24:05 UTC
The following code:

#include <stdarg.h>

#define  MAXARGS     31

int execl(const char *file, const char *args, ...)
{
    va_list ap;
    char *array[MAXARGS +1];
    int argno = 0;


    va_start(ap, args);
    while (args != 0 && argno < MAXARGS)
    {
        array[argno++] = args;
        args = va_arg(ap, const char *);
    }
    array[argno] = (char *) 0;
    va_end(ap);
    return execv(file, array);
}


when compiled with gcc -E test.c produces:

# 1 "test.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "test.c"
# 1 "/usr/lib/gcc/x86_64-linux-gnu/4.8/include/stdarg.h" 1 3 4
# 40 "/usr/lib/gcc/x86_64-linux-gnu/4.8/include/stdarg.h" 3 4
typedef __builtin_va_list __gnuc_va_list;
# 98 "/usr/lib/gcc/x86_64-linux-gnu/4.8/include/stdarg.h" 3 4
typedef __gnuc_va_list va_list;
# 2 "test.c" 2



int execl(const char *file, const char *args, ...)
{
    va_list ap;
    char *array[31 +1];
    int argno = 0;


    __builtin_va_start(ap,args);
    while (args != 0 && argno < 31)
    {
        array[argno++] = args;
        args = __builtin_va_arg(ap,const char *);
    }
    array[argno] = (char *) 0;
    __builtin_va_end(ap);
    return execv(file, array);
}
Comment 12 dave287091 2021-09-28 06:30:49 UTC
There’s also __builtin_va_copy for va_copy.
Comment 13 Walter Bright 2021-09-28 07:48:24 UTC
https://github.com/dlang/dmd/pull/13107 solves the specific __builtin_va_list problem, but not the general problem.

The general problem could be addressed by importing core.stdc.stdarg into the C semantic routines and thereby hijacking the D implementation for use with the C code.
Comment 14 Walter Bright 2021-10-20 09:09:50 UTC
https://github.com/dlang/dmd/pull/13205

adds support for __builtin_va_start and __builtin_va_end
Comment 15 Walter Bright 2022-01-22 01:30:34 UTC
Fixed by https://github.com/dlang/dmd/pull/13532