[PATCH] D137268: [clang][Headers] Do not define varargs macros for __need___va_list

[Adhemerval Zanella via Phabricator via cfe-commits]

zatrazz added a comment.

In D137268#4069935 <https://reviews.llvm.org/D137268#4069935>, @enh wrote:

> In D137268#4069856 <https://reviews.llvm.org/D137268#4069856>, @zatrazz wrote:
>
>> I think I have caught this because your standard conformance tests checks for __gnuc_va_list 
>> on wchar.h, wich is always defined on on GCC (git log shows it was changed to fix XPG7 
>> tests, but I am not sure exactly why the author has changed the va_list to __gnuc_va_list).
>
> bionic's tests check for `va_list`, because that's what POSIX says will be visible. ISO C doesn't say that, so i think the _intention_ for glibc -- musl seems to do this correctly -- is to say "if we're only compiling C source, you don't get `va_list`, but if we're compiling POSIX source, you do get `va_list`". so i think this `__gnuc_va_list` thing is their workaround to still export the _functions_ without exporting the _type_ for ISO C?

Indeed the __gnuc_va_list is used when building for c99/c11, since va_list should not be defined in such cases. It seems also that glibc stdio.h does handle it by defining __gnuc_va_list to va_list if POSIX2001 or POSIX2008 is used (-D_XOPEN_SOURCE=600 or -D_XOPEN_SOURCE=700), so I think we should do the same for wchar.h

> here's the bionic *POSIX* test: https://cs.android.com/android/platform/superproject/+/master:bionic/tests/headers/posix/wchar_h.c;l=38 (but note that you'll have to look at the corresponding Android.bp file to see us define `_POSIX_SOURCE`.) note that our tests pass against _bionic_ (and, i think, musl). it's just old-glibc-with-new-llvm they fail against.
>
>> And it seems that glibc seems broken also using GCC stdarg.h if I fix the test to check for
>> va_list instead. I will take a look at this.
>
> thanks!

Could you check if this fixes your issue? I am not sure if if applies cleans to the ancient glibc 2.17 you are using:

  diff --git a/conform/data/wchar.h-data b/conform/data/wchar.h-data
  index e414651a33..582a99c00b 100644
  --- a/conform/data/wchar.h-data
  +++ b/conform/data/wchar.h-data
  @@ -15,6 +15,9 @@ type size_t
   type locale_t
   # endif
   tag {struct tm}
  +#if !defined ISO && !defined ISO99 && !defined ISO11 && !defined POSIX && !defined UNIX98
  +type va_list
  +#endif
  
   function wint_t btowc (int)
   function int fwprintf (FILE*, const wchar_t*, ...)
  diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h
  index 69e920b8c2..ca145bb8d2 100644
  --- a/wcsmbs/wchar.h
  +++ b/wcsmbs/wchar.h
  @@ -37,6 +37,17 @@
   #define __need___va_list
   #include <stdarg.h>
  
  +#if defined __USE_XOPEN2K || defined __USE_XOPEN2K8
  +# ifdef __GNUC__
  +#  ifndef _VA_LIST_DEFINED
  +typedef __gnuc_va_list va_list;
  +#   define _VA_LIST_DEFINED
  +#  endif
  +# else
  +#  include <stdarg.h>
  +# endif
  +#endif
  +
   #include <bits/wchar.h>
   #include <bits/types/wint_t.h>
   #include <bits/types/mbstate_t.h>

In any case, I will open a bug report on glibc fix on our side.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137268/new/

https://reviews.llvm.org/D137268