Another manifestation is that if I add "inline" like this:
extern "C" inline void _ReadWriteBarrier();
I get another warning too:
test.cpp(1,24) : warning: inline function '_ReadWriteBarrier' is not defined [-Wundefined-inline]

This warning is visible when doing "check-asan check-sa without the "-Wno-undefined-inline" flag.

This would require completely changing our approach to MS intrinsics, which we've been avoiding for some time. We'd have to take everything in intrin.h and make them compiler builtins instead of static, alwaysinline, nodebug wrappers around our gcc-style builtins.

So far, we've been recommending that people include intrin.h, which is the MSDN documented way to get at these intrinsics.

The approach that yields a warning is used in lib/sanitizer_common/sanitizer_atomic_msvc.h. This header is included in a few places in the ASan RTL and is transitively included into lib/asan/asan_malloc_win.cc which defines its own malloc, free, etc. Ditto with lib/asan/asan_interceptors.cc and longjmp, lib/asan/asan_dll_thunk etc.
Including <intrin.h> brings in the "real" declaration of those function and we end up having conflicting declarations/definitions here and there.

I think we have a convention to avoid systen includes as much as possible in the ASan RTL. [Looping in Kostya]

Is the code like I wrote initially valid?
If yes -- we can just live with the warning disabled in CMake for a while.

I'd disable the warning for now.

If we are not going to change our approach for MS intrinsics, perhaps we should just accept this pragma and don't do anything with it?

This seems to be a common compatibility issue, so I think we should handle the pragma.

In Clang, #pragma intrinsic should check if the arguments are all recognized builtins. If they are not, we should emit a diagnostic telling the user that this intrinsic is not recognized as a compiler builtin and they should try including <intrin.h> to access the non-builtin intrinsics.

I don't think we need support for the counterpart, #pragma function, which demotes a builtin to a normal function.

We already have a short list of MSVC intrinsics implemented as builtins in Builtins.def. I think we should try to add most of the commonly used families of intrinsics (_Interlocked*) to this list. Anything that compiles down to inline assembly should IMO stay part of the intrin.h header, but if it's sufficiently useful that it can be an LLVM intrinsic, then I think we should thread it through that way.

I hit this again in this way:

$ cat t.cpp
#include <windows.h>
int main() {
MemoryBarrier();
}

$ clang-cl t.cpp
t-6c0216.obj : error LNK2019: unresolved external symbol __faststorefence referenced in function main
t.exe : fatal error LNK1120: 1 unresolved externals

This made the clang-cl self-host not use atomics, which is silly. Force including intrin.h fixes it, but that's a hack.

We could handle this entirely in clang by emitting an inline asm call, but that's gross. We could add the LLVM intrinsic, or see if there's one we can reuse.

I hit this again in this way:

ReactOS build is still blocked by this. Tested r241339.

Some of the link errors we get with:

clang version 3.8.0 (trunk 251783)
Target: i686-pc-windows-msvc
Thread model: posix

are:

ldrinit.c.obj : error LNK2019: unresolved external symbol __mm_pause referenced in function _LdrpInitSecurityCookie@4
ldrinit.c.obj : error LNK2019: unresolved external symbol ___ull_rshift referenced in function _LdrpInitSecurityCookie@4
rtl.lib(actctx.c.obj) : error LNK2019: unresolved external symbol __byteswap_ushort referenced in function "?filt$0@0@check_actctx@@" (?filt$0@0@check_actctx@@)
rtl.lib(bitmap.c.obj) : error LNK2019: unresolved external symbol __BitScanReverse referenced in function _RtlFindMostSignificantBit@8
rtl.lib(bitmap.c.obj) : error LNK2019: unresolved external symbol __BitScanForward referenced in function _RtlFindLeastSignificantBit@8
rtl.lib(except.c.obj) : error LNK2019: unresolved external symbol __ReturnAddress referenced in function _RtlUnwind@16
rtl.lib(slist.c.obj) : error LNK2019: unresolved external symbol __InterlockedCompareExchange64 referenced in function @​RtlInterlockedPushListSList@16
rtl.lib(interlck.c.obj) : error LNK2001: unresolved external symbol __InterlockedCompareExchange64
rtl.lib(network.c.obj) : error LNK2019: unresolved external symbol __byteswap_ulong referenced in function _RtlIpv4StringToAddressW@16

Apparently Google's HighwayHash (https://github.com/google/highwayhash/) uses this:

#if MSC_VERSION
#include <intrin.h>
#pragma intrinsic(_ReadWriteBarrier)
#define COMPILER_FENCE _ReadWriteBarrier()
#elif GCC_VERSION
#define COMPILER_FENCE asm volatile("" : : : "memory")
#else
#define COMPILER_FENCE
#endif

Comment 10: That should work fine today.

Any time you explicitly include intrin.h, things should be fine. We'll ignore the pragma intrinsic line and use what's in intrin.h, and that works (hence the recommendation to build with /FIIntrin.h in clang-cl builds for now).

What doesn't work is to manually declare an intrinsic's prototype and then say pragma intrinsic, like so:

void __cpuid(int info[4], int fn);
#pragma intrinsic(__cpuid)

With cl, this will work: The first line provides a declaration, and the second tells the compiler to use an intrinsic. With clang-cl, __cpuid is implemented as an always-inline function in the intrin.h header shipping with clang-cl. Since that's not included here, the definition of __cpuid() isn't available and hence you'll get a link error for __cpuid if it's called.

In code you control, you can always explicitly #include <intrin.h> to fix this. However, some system headers do declare functions and then mark some of them as pragma intrinsic, and that's why you'll e.g. end up with undefined references to __stosb if you call memset(foo, 0, ...): The system header expands memset() to something that's an inline function that calls __stosb, and the system header expects that __stosb won't require intrin.h nor a definition for __stosb since it declares it as intrinsic.

Fixing that is what this bug is about. In practice, we should figure out which intrinsics are forward-declared and used by system headers in this way, and add support for those.

Albert implemented #pragma intrinsic and a lot of builtins for it. It may not cover every case, but the situation should be substantially improved now.

Yes, let's mark this done. If someone is missing anything from the current implementation, let's have targeted bugs for that.

mentioned in issue #7187