[Compiler-RT] Fix profiler building with MinGW GCC

[mati865]

Summary

• ftruncate is not available in mingw-w64 unless _POSIX is defined
• __attribute__((visibility("hidden"))) does nothing on Windows (aside spamming warnings)
• __attribute__((weak)) is not supported on Windows like on UNIX. Clang before version 11 will create standard symbol (resulting in "multiple definition" errors in some cases) while GCC creates weak symbol that serious limitations/issues (often leading to "undefined reference" errors). MinGW compilers should use selectany here just like MSVC does.

[mstorsjo]

The patch looks mostly good to me.

Regarding __attribute__((weak)) it should work fine in Clang 11 now (I landed a bunch of fixes relating to that in March), but I also remember that it's occasionally problematic with GCC, and generally selectany is preferred, assuming that's the kind of weak semantics that is needed. (The weak attribute can either be used for multiple weak symbols in the same link, or one weak + one non-weak, or the case where there's an undefined reference to a symbol marked weak, and no actual definition of the symbol. Selectany works fine for the first of these cases, but not necessarily for the other ones.)

compiler-rt/lib/profile/InstrProfilingPort.h

20

Why are these lines moved around? As far as I see, their content is kept intact.

[mati865]

Regarding __attribute__((weak)) it should work fine in Clang 11 now

Oh that's neat.
Should I revert weak change then?

This patch allows Clang to link libprofiler built with GCC without errors but the resulting binary crashes:

Process 8324 launched: 'D:\Projekty\rust\build\x86_64-pc-windows-gnu\test\run-make-fulldeps\instrument-coverage\instrume
nt-coverage\testprog.exe' (x86_64)
Process 8324 stopped
* thread #1, stop reason = Exception 0xc0000005 encountered at address 0x1400ad4d0: Access violation reading location 0x
00000012
    frame #0: 0x00000001400ad4d0 testprog.exe`initializeValueProfRuntimeRecord(Data=0x00000001400e5030, SiteCountArray=0
x0000000000000000) at InstrProfilingValue.c:332:14
   329            Nodes ? RTRecord.NodesKind[I][J] : INSTR_PROF_NULLPTR;
   330        while (Site) {
   331          C++;
-> 332          Site = Site->Next;
   333        }
   334        if (C > UCHAR_MAX)
   335          C = UCHAR_MAX;
(lldb) bt
* thread #1, stop reason = Exception 0xc0000005 encountered at address 0x1400ad4d0: Access violation reading location 0x
00000012
  * frame #0: 0x00000001400ad4d0 testprog.exe`initializeValueProfRuntimeRecord(Data=0x00000001400e5030, SiteCountArray=0
x0000000000000000) at InstrProfilingValue.c:332:14
    frame #1: 0x00000001400af05e testprog.exe`writeOneValueProfData(VPDataReader=0x00000001400d8560, Data=0x00000001400e
5030, BufferIO=0x00000001400d8540) at InstrProfilingWriter.c:146:25
    frame #2: 0x00000001400ae2e4 testprog.exe`lprofWriteDataImpl at InstrProfilingWriter.c:227:9
    frame #3: 0x00000001400ae262 testprog.exe`lprofWriteDataImpl(Writer=0x000000000014fd70, DataBegin=<unavailable>, Dat
aEnd=0x00000001400e51c0, CountersBegin=0x00000001400e4008, CountersEnd=0x00000001400e4048, VPDataReader=0x00000001400d85
60, NamesBegin="", NamesEnd="", SkipNameDataWrite=0) at InstrProfilingWriter.c:299
    frame #4: 0x00000001400adfd0 testprog.exe`lprofWriteData(Writer=0x000000000014fd70, VPDataReader=0x00000001400d8560,
 SkipNameDataWrite=0) at InstrProfilingWriter.c:248:10
    frame #5: 0x00000001400ac373 testprog.exe`__llvm_profile_write_file at InstrProfilingFile.c:346:12
    frame #6: 0x00000001400ac223 testprog.exe`__llvm_profile_write_file at InstrProfilingFile.c:935

This test works just fine when building libprofiler with Clang with or without this patch.

Right now it doesn't matter much for Rust since it's still forced to use GCC for backward compatibility but PGO tests fail with lld-link: error: duplicate symbol: __llvm_profile_raw_version without this patch.

compiler-rt/lib/profile/InstrProfilingPort.h

20

To keep the same order of defines between _MSC_VER and __GNUC__. I can revert that.

[mati865]

I was using Clang 10 during tests. I can repeat it with Clang 11 tomorrow/next week if you want.

[mstorsjo]

In D86405#2232228, @mati865 wrote:

Regarding __attribute__((weak)) it should work fine in Clang 11 now

Oh that's neat.
Should I revert weak change then?

Only if weak works properly with GCC as well. IIRC the case where you can replace weak with selectany (the case with multiple equal definitions in multiple object files) does work with GCC though, so if that works with both GCC and Clang we could keep it weak.

This patch allows Clang to link libprofiler built with GCC without errors but the resulting binary crashes:

Process 8324 launched: 'D:\Projekty\rust\build\x86_64-pc-windows-gnu\test\run-make-fulldeps\instrument-coverage\instrume
nt-coverage\testprog.exe' (x86_64)
Process 8324 stopped
* thread #1, stop reason = Exception 0xc0000005 encountered at address 0x1400ad4d0: Access violation reading location 0x
00000012
    frame #0: 0x00000001400ad4d0 testprog.exe`initializeValueProfRuntimeRecord(Data=0x00000001400e5030, SiteCountArray=0
x0000000000000000) at InstrProfilingValue.c:332:14
   329            Nodes ? RTRecord.NodesKind[I][J] : INSTR_PROF_NULLPTR;
   330        while (Site) {
   331          C++;
-> 332          Site = Site->Next;
   333        }
   334        if (C > UCHAR_MAX)
   335          C = UCHAR_MAX;
(lldb) bt
* thread #1, stop reason = Exception 0xc0000005 encountered at address 0x1400ad4d0: Access violation reading location 0x
00000012
  * frame #0: 0x00000001400ad4d0 testprog.exe`initializeValueProfRuntimeRecord(Data=0x00000001400e5030, SiteCountArray=0
x0000000000000000) at InstrProfilingValue.c:332:14
    frame #1: 0x00000001400af05e testprog.exe`writeOneValueProfData(VPDataReader=0x00000001400d8560, Data=0x00000001400e
5030, BufferIO=0x00000001400d8540) at InstrProfilingWriter.c:146:25
    frame #2: 0x00000001400ae2e4 testprog.exe`lprofWriteDataImpl at InstrProfilingWriter.c:227:9
    frame #3: 0x00000001400ae262 testprog.exe`lprofWriteDataImpl(Writer=0x000000000014fd70, DataBegin=<unavailable>, Dat
aEnd=0x00000001400e51c0, CountersBegin=0x00000001400e4008, CountersEnd=0x00000001400e4048, VPDataReader=0x00000001400d85
60, NamesBegin="", NamesEnd="", SkipNameDataWrite=0) at InstrProfilingWriter.c:299
    frame #4: 0x00000001400adfd0 testprog.exe`lprofWriteData(Writer=0x000000000014fd70, VPDataReader=0x00000001400d8560,
 SkipNameDataWrite=0) at InstrProfilingWriter.c:248:10
    frame #5: 0x00000001400ac373 testprog.exe`__llvm_profile_write_file at InstrProfilingFile.c:346:12
    frame #6: 0x00000001400ac223 testprog.exe`__llvm_profile_write_file at InstrProfilingFile.c:935

Hmm, weird. I have no good clues off-hand about what might be causing this...

compiler-rt/lib/profile/InstrProfilingPort.h

20

Yeah it's probably clearer wrt this patch to keep the original order here.

[mati865]

Only if weak works properly with GCC as well.

Weak here doesn't work properly with GCC 6 and GCC 10 and causes issues for Clang 10. I'll build Clang 11 and report back later today.

[mati865]

Updated diff to remove _MSC_VER change.

Tested with Clang 11 and __attribute__((weak)) works just as good as __attribute__((selectany)).
Should I make it use weak like that?

#if defined(clang) && __clang_major__ > 10
#define COMPILER_RT_WEAK __attribute__((weak))
#else
#define COMPILER_RT_WEAK __attribute__((selectany))
#endif

[mstorsjo]

LGTM

In D86405#2233301, @mati865 wrote:

Updated diff to remove _MSC_VER change.

Tested with Clang 11 and __attribute__((weak)) works just as good as __attribute__((selectany)).
Should I make it use weak like that?

#if defined(clang) && __clang_major__ > 10
#define COMPILER_RT_WEAK __attribute__((weak))
#else
#define COMPILER_RT_WEAK __attribute__((selectany))
#endif

No, there's no need for that - selectany is a much simpler mechanism anyway - was just curious whether the weak stuff worked as intended now or not.

compiler-rt/lib/profile/InstrProfilingPort.h

 #ifndef PROFILE_INSTRPROFILING_PORT_H_
 #define PROFILE_INSTRPROFILING_PORT_H_
 
 #ifdef _MSC_VER
 #define COMPILER_RT_ALIGNAS(x) __declspec(align(x))
 #define COMPILER_RT_VISIBILITY
 /* FIXME: selectany does not have the same semantics as weak. */
 #define COMPILER_RT_WEAK __declspec(selectany)
 /* Need to include <windows.h> */
 #define COMPILER_RT_ALLOCA _alloca
 /* Need to include <stdio.h> and <io.h> */
 #define COMPILER_RT_FTRUNCATE(f,l) _chsize(_fileno(f),l)
 #define COMPILER_RT_ALWAYS_INLINE __forceinline
 #define COMPILER_RT_CLEANUP(x)
 #elif __GNUC__
-#define COMPILER_RT_ALIGNAS(x) __attribute__((aligned(x)))
+#ifdef _WIN32
+#define COMPILER_RT_FTRUNCATE(f, l) _chsize(fileno(f), l)
+#define COMPILER_RT_VISIBILITY
+#define COMPILER_RT_WEAK __attribute__((selectany))
+#else
+#define COMPILER_RT_FTRUNCATE(f, l) ftruncate(fileno(f), l)
 #define COMPILER_RT_VISIBILITY __attribute__((visibility("hidden")))
 #define COMPILER_RT_WEAK __attribute__((weak))
+#endif
+#define COMPILER_RT_ALIGNAS(x) __attribute__((aligned(x)))
 #define COMPILER_RT_ALLOCA __builtin_alloca
-#define COMPILER_RT_FTRUNCATE(f,l) ftruncate(fileno(f),l)
 #define COMPILER_RT_ALWAYS_INLINE inline __attribute((always_inline))
 #define COMPILER_RT_CLEANUP(x) __attribute__((cleanup(x)))
 #endif
 
 #if defined(__APPLE__)
 #define COMPILER_RT_SEG "__DATA,"
 #else
 #define COMPILER_RT_SEG ""