[cfe-dev] [compiler-rt] 2 nit Win32 patches

[Aaron Ballman]

On Tue, Jul 16, 2013 at 5:10 PM, "C. Bergström"
<cbergstrom at pathscale.com> wrote:
> Hi
>
> If someone can give review and commit these 2 compiler-rt patches that would
> be great.
>
> Patch 0 : compiler-rt-chkstk.patch - This allows building with recent
> mingw64 on Windows. We're hoping to get the build working and then possibly
> open a discussion on the benefits, if any, for more than a stub function.
>
> Patch 1 : compiler-rt-hidden.patch - Per recommendation from a Windows dev
> it's not supported and we should disable it.
>
> diff --git a/lib/eprintf.c b/lib/eprintf.c
> index b07d624..3626dbf 100644
> --- a/lib/eprintf.c
> +++ b/lib/eprintf.c
> @@ -22,7 +22,9 @@
>   * It should never be exported from a dylib, so it is marked
>   * visibility hidden.
>   */
> +#ifndef _WIN32

What problem is this trying to solve with MinGW?  I would imagine this
would be problematic with MSVC since it doesn't understand
__attribute__, but MinGW uses gcc under the hood.

>  __attribute__((visibility("hidden")))
> +#endif
>  void __eprintf(const char* format, const char* assertion_expression,
>   const char* line, const char* file)
>  {
> diff --git a/lib/int_util.c b/lib/int_util.c
> index 871d191..6d8922a 100644
> --- a/lib/int_util.c
> +++ b/lib/int_util.c
> @@ -24,7 +24,9 @@
>  #ifdef KERNEL_USE
>
>  extern void panic(const char *, ...) __attribute__((noreturn));
> +#ifndef _WIN32
>  __attribute__((visibility("hidden")))
> +#endif
>  void compilerrt_abort_impl(const char *file, int line, const char *function) {
>    panic("%s:%d: abort in %s", file, line, function);
>  }
> @@ -35,8 +37,10 @@ void compilerrt_abort_impl(const char *file, int line, const char *function) {
>  extern void __assert_rtn(const char *func, const char *file,
>                       int line, const char * message) __attribute__((noreturn));
>
> +#ifndef _WIN32
>  __attribute__((weak))
>  __attribute__((visibility("hidden")))
> +#endif
>  void compilerrt_abort_impl(const char *file, int line, const char *function) {
>    __assert_rtn(function, file, line, "libcompiler_rt abort");
>  }
> @@ -47,8 +51,10 @@ void compilerrt_abort_impl(const char *file, int line, const char *function) {
>  /* Get the system definition of abort() */
>  #include <stdlib.h>
>
> +#ifndef _WIN32
>  __attribute__((weak))
>  __attribute__((visibility("hidden")))
> +#endif
>  void compilerrt_abort_impl(const char *file, int line, const char *function) {
>    abort();
>  }

I think a more clean approach would be to define a macro for the
__attribute__'s being #ifdef'ed out, and then use the macros.  This is
the approach taken elsewhere in llvm (see Compiler.h).

~Aaron