[sanitizer] Make __sanitizer::CheckFailed not public

[cryptoad]

Summary

As far as I can tell, there is no reason why __sanitizer::CheckFailed should
be exported. Looking back in time, it was added with the FIXME with the
following by @timurrrr:

[*San/RTL] Fix minor breakage
Grumbling: this hasn't been caught by running 'make check-{a,l,t}san check-sanitizer'

I can't find any detail about the breakage, all tests seem to work for me, so
maybe Windows (@rnk?) or something I have no setup for.

The reason to make it private (past the FIXME) is that Scudo defines its own
(without callback) and I am trying to make the .so be loadable with the UBsan
one (that has its own public CheckFailed) with as little drama as possible.

[rnk]

I ran the tests with your patch and everything passes, but of course, Timur mentioned that nothing in our test suite finds this issue. He probably encountered it in Chrome and it was hard to reduce into something we could add to the test suite.

I'd say, commit it, and we'll see if the CrWinASan bots come back green after the fact. They probably will. The need for this may have gone away as we restructured the various static and dynamic asan libraries.

[eugenis]

LGTM from Linux PoV

[mstorsjo]

Sorry to say, but this broke asan on windows for me (on i386 only though).

For me, __asan::register_dso_globals() in asan_globals_win.cc.obj in clang_rt.asan_dynamic_runtime_thunk-i386 refers to this symbol, which previously used to be exported from clang_rt.asan_dynamic-i386. So even though it in principle shouldn't be part of the public interface, the way the static/dynamic parts of asan for windows is structured, this needs to be exported there.

It doesn't seem to trigger any issue on x86_64 though; I think the compiler can assume that the CheckFailed call in asan_globals_win.cc.obj can't be triggered there and doesn't emit any call.

Should we revert this again, or readd the attribute for windows builds only?

[cryptoad]

In D52279#1246222, @mstorsjo wrote:

Should we revert this again, or readd the attribute for windows builds only?

I am not familiar at all with the logic there, I can re-add it for Windows unless someone else has a better idea (maybe __i386__ only too)?

[cryptoad]

Or maybe replace the CHECK there with a Trap?

[mstorsjo]

In D52279#1246672, @cryptoad wrote:

Or maybe replace the CHECK there with a Trap?

Oh, indeed - we call __debugbreak() there instead, for the dll thunk case - we can use that for the dynamic runtime thunk case as well. I'll send a patch with that.

lib/sanitizer_common/sanitizer_internal_defs.h

 typedef void* thread_return_t;
 # define THREAD_CALLING_CONV
 #endif  // _WIN32
 typedef thread_return_t (THREAD_CALLING_CONV *thread_callback_t)(void* arg);
 
 // NOTE: Functions below must be defined in each run-time.
 void NORETURN Die();
 
-// FIXME: No, this shouldn't be in the sanitizer interface.
-SANITIZER_INTERFACE_ATTRIBUTE
 void NORETURN CheckFailed(const char *file, int line, const char *cond,
                           u64 v1, u64 v2);
 
 // Check macro
 #define RAW_CHECK_MSG(expr, msg) do { \
   if (UNLIKELY(!(expr))) { \
     RawWrite(msg); \
     Die(); \