[Sanitizer] Properly use dllimport / dllexport.

[mpividori]

Summary

I update the headers, so we can change the dllexports to dllimport when defining SANITIZER_IMPORT_INTERFACE , I need these changes for futures diffs fixing Sanitizer Coverage for Windows.

[zturner]

lgtm, this is a standard pattern on Windows.

[rnk]

You don't think you should flip it around so consumers of sanitiers_internal_defs.h don't need to define stuff? Instead we'd add SANITIZER_EXPORT_INTERFACE to defines when building sanitizer_common.

[mpividori]

@rnk I define it that way, because that interface is internal. We only use SANITIZER_INTERFACE_ATTRIBUTE internally, we don't include that macro in the compiler-rt/include directory.
And we only need to change it to "dllimports" for the internal library: clang-rt_asan_dynamic_runtime_thunk-arch.lib
But now that I think, it would be better to take your approach, and also include SANITIZER_INTERFACE_ATTRIBUTE in the headers provided in the compiler-rt/include directory. So clients, take advantage of the "dllimport" optimization.
Would you agree?

[rnk]

In D29052#654145, @mpividori wrote:

@rnk I define it that way, because that interface is internal. We only use SANITIZER_INTERFACE_ATTRIBUTE internally, we don't include that macro in the compiler-rt/include directory.
And we only need to change it to "dllimports" for the internal library: clang-rt_asan_dynamic_runtime_thunk-arch.lib

Usually there are more library users than there are libraries, so it's better to define a special macro when compiling the library rather than when consuming the library.

But now that I think, it would be better to take your approach, and also include SANITIZER_INTERFACE_ATTRIBUTE in the headers provided in the compiler-rt/include directory. So clients, take advantage of the "dllimport" optimization.
Would you agree?

I agree, that would be better.

[mpividori]

In D29052#654165, @rnk wrote:

In D29052#654145, @mpividori wrote:

@rnk I define it that way, because that interface is internal. We only use SANITIZER_INTERFACE_ATTRIBUTE internally, we don't include that macro in the compiler-rt/include directory.
And we only need to change it to "dllimports" for the internal library: clang-rt_asan_dynamic_runtime_thunk-arch.lib

Usually there are more library users than there are libraries, so it's better to define a special macro when compiling the library rather than when consuming the library.

But now that I think, it would be better to take your approach, and also include SANITIZER_INTERFACE_ATTRIBUTE in the headers provided in the compiler-rt/include directory. So clients, take advantage of the "dllimport" optimization.
Would you agree?

I agree, that would be better.

@rnk Thinking again on this, I think this is appropriate, by default use dllexport, because SANITIZER_INTERFACE_ATTRIBUTE is used on internal headers, not in headers provided to users: which should go in the compiler-rt/include dir.
Internally, we always need to use dllexport, except for some special cases, the dll_thunks and dynamic_runtime_thunks. So, I think is appropriate to use dllexport by default, internally, otherwise, I should add a flag like: BUILDING_LIBRARY, and always include that flag, for all sanitizers, except when building the dll_thunks and dynamic_runtime_thunks, which I think will be confusing.
We always need to dllexport the interface, when the sanitizer is a dll, and when it is a static library (because we use interception).

So, I propose to continue with this approach. In the directory: compiler-rt/include , we can do something different because that headers are independent.
Would you agree?

[rnk]

Sure, sounds reasonable. In this case we just happen to have fewer library consumers of the exported APIs. The only library consumers are the static thunk libraries.

compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h

 #include "sanitizer_platform.h"
 
 #ifndef SANITIZER_DEBUG
 # define SANITIZER_DEBUG 0
 #endif
 
 // Only use SANITIZER_*ATTRIBUTE* before the function return type!
 #if SANITIZER_WINDOWS
+#if SANITIZER_IMPORT_INTERFACE
+# define SANITIZER_INTERFACE_ATTRIBUTE __declspec(dllimport)
+#else
 # define SANITIZER_INTERFACE_ATTRIBUTE __declspec(dllexport)
+#endif
 # define SANITIZER_WEAK_ATTRIBUTE
 #elif SANITIZER_GO
 # define SANITIZER_INTERFACE_ATTRIBUTE
 # define SANITIZER_WEAK_ATTRIBUTE
 #else
 # define SANITIZER_INTERFACE_ATTRIBUTE __attribute__((visibility("default")))
 # define SANITIZER_WEAK_ATTRIBUTE  __attribute__((weak))
 #endif