This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] Use interception to access to strong functions defined in the main executable.
ClosedPublic

Authored by mpividori on Jan 25 2017, 9:33 PM.

Details

Summary

In Windows, when sanitizers are implemented as a shared library (DLL), users can redefine and export a new definition for weak functions, in the main executable, for example:

extern "C" __declspec(dllexport) void __sanitizer_cov_trace_pc_guard(u32* guard) {
  // Different implementation provided by the client.
}

However, other client's dlls, will continue using the default implementation imported from the sanitizer dll.
This is different in linux, where all the shared libraries will consider the strong definition.

With the implementation in this diff, when the dll is initialized, it will check if the main executable exports the definition for some weak function (for example __sanitizer_cov_trace_pc_guard). If it finds that function, then it will override the function in the dll with that pointer. So, all the client's dlls with instrumentation that import __sanitizer_cov_trace_pc_guard__dll() from dll, will be using the function provided by the main executable.

In other words, when the main executable exports a strong definition for a weak function, we ensure all the dlls use that implementation instead of the default weak implementation.

So, the behavior is similar to linux. Now, every user that want to override a weak function, only has to define and export it. The same for Linux and Windows, and it will work fine. So, there is no difference on the user's side.


All the sanitizers will include a file sanitizer_win_weak_interception.cc that register sanitizer's weak functions to be intercepted in the binary section: WEAK

When the sanitizer dll is initialized, it will execute weak_intercept_init() implemented in: sanitizer_common/sanitizer_win_weak_interception.cc, which will consider all the CB registered in the section WEAK. So, for all the weak functions registered, we will check if a strong definition is provided in the main executable. If that is the case, we will override the dll implementation with the user provided implementation.

All the files sanitizer_win_weak_interception.cc are independent, so we don't need to include a specific list of sanitizers.
Now, we include asan_win_weak_interception.cc ubsan_win_weak_interception.cc sanitizer_coverage_win_weak_interception.cc and sanitizer_win_weak_interception.cc, in asan dll, so when it is initialized, it will consider all the weak functions from asan, ubsan and sanitizer coverage.


After this diff, sanitizer coverage is fixed for MD on Windows. In particular libFuzzer can provide custom implementation for all sanitizer coverage's weak functions, and they will be considered by asan dll.

Diff Detail

Repository
rL LLVM

Event Timeline

mpividori created this revision.Jan 25 2017, 9:33 PM
zturner added a subscriber: zturner.Feb 2 2017, 1:41 PM
zturner added inline comments.
lib/sanitizer_common/sanitizer_win_weak_interception.cc
34–35 ↗(On Diff #85863)

I don't have a strong opinion here, but if the override fails, should it abort, or should it just behave as if the function was not found?

mpividori added inline comments.Feb 2 2017, 1:46 PM
lib/sanitizer_common/sanitizer_win_weak_interception.cc
34–35 ↗(On Diff #85863)

I think it is appropriate to fail.
If the user has provided a strong definition (that means we can find it with InternalGetProcAddress()) then we should use it. We can not ignore it because it would be incorrect. If we can not override it, then we should fail because otherwise the program would behave in an unexpected way.
Is the same that we do on dll_thunks in https://reviews.llvm.org/D29154. If it fails to override the function, it aborts.

alekseyshl accepted this revision.Feb 2 2017, 2:34 PM

LGTM

lib/sanitizer_common/sanitizer_win_weak_interception.cc
33 ↗(On Diff #85863)

Four spaces indent.

71 ↗(On Diff #85863)

Four spaces indent.

This revision is now accepted and ready to land.Feb 2 2017, 2:34 PM

Ok. @zturner @alekseyshl thanks for your time.

This revision was automatically updated to reflect the committed changes.