Page MenuHomePhabricator

[compiler-rt][asan] Fix incorrect macro preventing ICF with MSVC
ClosedPublic

Authored by etienneb on Feb 21 2017, 11:18 AM.

Details

Summary

The DLL thunks are stubs added to an instrumented DLL to redirect ASAN API calls
to the real ones in the main executable. These thunks must contain dummy
code before __asan_init got called. Unfortunately, MSVC linker is doing ICF and is
merging functions with the same body.

In our case, this two ASAN thunks were incorrectly merged:

asan_interface.inc:16
INTERFACE_FUNCTION(__asan_before_dynamic_init)
sanitizer_common_interface.inc:16
INTERFACE_FUNCTION(__sanitizer_verify_contiguous_container)

The same thunk got patched twice. After the second patching, calls to
__asan_before_dynamic_init are redirected to __sanitizer_verify_contiguous_container
and trigger a DCHECK on incorrect operands/

The problem was caused by the macro that is only using LINE to prevent
collapsing code.

#define INTERCEPT_SANITIZER_FUNCTION(name)
  extern "C" __declspec(noinline) void name() {
  volatile int prevent_icf = (__LINE__ << 8); (void)prevent_icf;

The current patch is adding COUNTER which is safer than LINE.

Also, to precent ICF (guarantee that code is different), we are using a unique attribute:

  • the name of the function

Event Timeline

etienneb created this revision.Feb 21 2017, 11:18 AM
etienneb edited the summary of this revision. (Show Details)Feb 21 2017, 11:21 AM
etienneb edited the summary of this revision. (Show Details)Feb 21 2017, 11:25 AM
etienneb edited the summary of this revision. (Show Details)
rnk accepted this revision.Feb 21 2017, 11:28 AM

lgtm

My best other idea is to try to make the functions self-recursive, but I don't think that's enough.

This revision is now accepted and ready to land.Feb 21 2017, 11:28 AM

My other idea was to provide raw bytes and emit them into the code section.

char think[] = { 0x90, 0x90, 0x90, 0x90, 0x90, 0x90, 0xCC, 0xCC, 0xCC 0xCC, ... 0xCC };

Now, we rely on the compiler to produce a function that we are able to patch instead of hardcoding a function we can patch.
But still, we need to prevent ICF.

rnk added a comment.Feb 21 2017, 11:55 AM

Nice, counter macro is better.

If we want raw bytes I'd use an assembler file.

etienneb closed this revision.Feb 21 2017, 12:16 PM