[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.

lib/sanitizer_common/sanitizer_internal_defs.h

 
 // 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

lib/sanitizer_common/sanitizer_win_defs.h

 // Name for exported implementation of weak function.
 # define WEAK_EXPORT_NAME(Name) Name##__dll
 
+#if SANITIZER_IMPORT_INTERFACE
+# define WIN_INTERFACE_ATTRIBUTE __declspec(dllimport)
+#else
+# define WIN_INTERFACE_ATTRIBUTE __declspec(dllexport)
+#endif
+
 // Use this macro when you need to define and export a weak function from a
 // library. For example:
 //   WIN_WEAK_EXPORT_DEF(bool, compare, int a, int b) { return a > b; }

 //   WIN_WEAK_EXPORT_DECL(bool, compare, int a, int b);
 # define WIN_WEAK_EXPORT_DECL(ReturnType, Name, ...)                           \
   extern "C" ReturnType Name(__VA_ARGS__);                                     \
-  extern "C" __declspec(dllexport)                                             \
+  extern "C" WIN_INTERFACE_ATTRIBUTE                                           \
   ReturnType WEAK_EXPORT_NAME(Name)(__VA_ARGS__)
 
 // Use this macro when you need to import a weak function from a library. It