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

["C. Bergström"]

On 07/17/13 07:57 PM, Aaron Ballman wrote:
> 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).
While not "clean" the intention is clear and only cosmetic - it's been 
applied and if you feel strongly that it's tooo fugly - Feel free to 
adjust, but please do so with with leaving it disable for mingw64 - 
There was an irc discussion about this and visibility isn't supported on 
Win and weak is buggy to the point of not being usable.