Asan interception library patch

The idea of this patch is as follows.
Consider that a user program defines own function read(), and Asan intercepts read() as well. Currently we get 'duplicate symbol' linker error in this case. With this patch it's possible to successfully link and detect the situation in run-time.
For example, here is how Tsanv2 output looks on a hypothetical program that refines own read() and write():
$ tests/tsan_test 
ThreadSanitizer: failed to intercept 'read' function
ThreadSanitizer: failed to intercept 'write' function
[==========] Running 58 tests from 11 test cases.
[----------] Global test environment set-up.
...

The trick is to make the interceptors weak symbols + a bit of compiler magic to detect interception failure.

P.s. the patch changes names of interceptors on Linux from e.g. read() to interception_wrap_read().

[dvyukov, 2012-04-12 14:16:49]

The idea of this patch is as follows.
Consider that a user program defines own function read(), and Asan intercepts
read() as well. Currently we get 'duplicate symbol' linker error in this case.
With this patch it's possible to successfully link and detect the situation in
run-time.
For example, here is how Tsanv2 output looks on a hypothetical program that
refines own read() and write():
$ tests/tsan_test 
ThreadSanitizer: failed to intercept 'read' function
ThreadSanitizer: failed to intercept 'write' function
[==========] Running 58 tests from 11 test cases.
[----------] Global test environment set-up.
...

The trick is to make the interceptors weak symbols + a bit of compiler magic to
detect interception failure.

P.s. the patch changes names of interceptors on Linux from e.g. read() to
interception_wrap_read().

[samsonov, 2012-04-12 14:21:26]

http://codereview.appspot.com/6010049/diff/1/interception.h
File interception.h (right):

http://codereview.appspot.com/6010049/diff/1/interception.h#newcode78
interception.h:78: # define DECLARE_WRAPPER(ret_type, convention, func, ...)
Naming was not self-explanatory already and now it's even worse :( As I get it,
you use DECLARE_WRAPPER(...func...) to define "func" that is:
1) alias to an interceptor if instrumented code doesn't define its own "func"
2) breaks linkage if instrumented code does define its own "func"
Why not call this macro smth like "ADD_GUARD_AGAINST_CUSTOM_DEFINITION" or smth
like that.

http://codereview.appspot.com/6010049/diff/1/interception.h#newcode91
interception.h:91: # define WRAP(x) interception_wrap_##x
This fails on our output tests: the top stack frame of allocation now says
"interception_wrap_malloc" instead of "malloc". Can we use some other naming?

http://codereview.appspot.com/6010049/diff/1/interception.h#newcode96
interception.h:96: __attribute__((weak, alias("interception_wrap_"#func), \
alias(WRAPPER_NAME(func)) ?

http://codereview.appspot.com/6010049/diff/1/interception.h#newcode97
interception.h:97: visibility("default")))
You can use INTERCEPTOR_ATTRIBUTE instead of visibility("default")

http://codereview.appspot.com/6010049/diff/1/interception_linux.h
File interception_linux.h (right):

http://codereview.appspot.com/6010049/diff/1/interception_linux.h#newcode32
interception_linux.h:32: (void*)&func, (void*)&WRAP(func))
I think that
&& (void*)&func == (void*)&WRAP(func)
would be better.
Anyway, it's worth explaining this magic in the comment here or in
interception.h

[samsonov, 2012-04-12 14:22:45]

On 2012/04/12 14:21:26, samsonov wrote:
> http://codereview.appspot.com/6010049/diff/1/interception.h
> File interception.h (right):
> 
> http://codereview.appspot.com/6010049/diff/1/interception.h#newcode78
> interception.h:78: # define DECLARE_WRAPPER(ret_type, convention, func, ...)
> Naming was not self-explanatory already and now it's even worse :( As I get
it,
> you use DECLARE_WRAPPER(...func...) to define "func" that is:
> 1) alias to an interceptor if instrumented code doesn't define its own "func"
> 2) breaks linkage if instrumented code does define its own "func"
> Why not call this macro smth like "ADD_GUARD_AGAINST_CUSTOM_DEFINITION" or
smth
> like that.

I mean, "breaks in runtime"

> 
> http://codereview.appspot.com/6010049/diff/1/interception.h#newcode91
> interception.h:91: # define WRAP(x) interception_wrap_##x
> This fails on our output tests: the top stack frame of allocation now says
> "interception_wrap_malloc" instead of "malloc". Can we use some other naming?
> 
> http://codereview.appspot.com/6010049/diff/1/interception.h#newcode96
> interception.h:96: __attribute__((weak, alias("interception_wrap_"#func), \
> alias(WRAPPER_NAME(func)) ?
> 
> http://codereview.appspot.com/6010049/diff/1/interception.h#newcode97
> interception.h:97: visibility("default")))
> You can use INTERCEPTOR_ATTRIBUTE instead of visibility("default")
> 
> http://codereview.appspot.com/6010049/diff/1/interception_linux.h
> File interception_linux.h (right):
> 
> http://codereview.appspot.com/6010049/diff/1/interception_linux.h#newcode32
> interception_linux.h:32: (void*)&func, (void*)&WRAP(func))
> I think that
> && (void*)&func == (void*)&WRAP(func)
> would be better.
> Anyway, it's worth explaining this magic in the comment here or in
> interception.h

[kcc1, 2012-04-12 20:39:59]

Please hold on with this patch.
I'd really like to understand the consequences but have not time until late
next week (traveling, etc).
It would help if you could make a test for this new functionality.

--kcc

On Thu, Apr 12, 2012 at 7:22 AM, <samsonov@google.com> wrote:

> On 2012/04/12 14:21:26, samsonov wrote:
>
>>
http://codereview.appspot.com/**6010049/diff/1/interception.h<http://coderevi...
>> File interception.h (right):
>>
>
> 
http://codereview.appspot.com/**6010049/diff/1/interception.h#**newcode78<htt...
>> interception.h:78: # define DECLARE_WRAPPER(ret_type, convention,
>>
> func, ...)
>
>> Naming was not self-explanatory already and now it's even worse :( As
>>
> I get it,
>
>> you use DECLARE_WRAPPER(...func...) to define "func" that is:
>> 1) alias to an interceptor if instrumented code doesn't define its own
>>
> "func"
>
>> 2) breaks linkage if instrumented code does define its own "func"
>> Why not call this macro smth like
>>
> "ADD_GUARD_AGAINST_CUSTOM_**DEFINITION" or smth
>
>> like that.
>>
>
> I mean, "breaks in runtime"
>
>
>
> 
http://codereview.appspot.com/**6010049/diff/1/interception.h#**newcode91<htt...
>> interception.h:91: # define WRAP(x) interception_wrap_##x
>> This fails on our output tests: the top stack frame of allocation now
>>
> says
>
>> "interception_wrap_malloc" instead of "malloc". Can we use some other
>>
> naming?
>
> 
http://codereview.appspot.com/**6010049/diff/1/interception.h#**newcode96<htt...
>> interception.h:96: __attribute__((weak,
>>
> alias("interception_wrap_"#**func), \
>
>> alias(WRAPPER_NAME(func)) ?
>>
>
> 
http://codereview.appspot.com/**6010049/diff/1/interception.h#**newcode97<htt...
>> interception.h:97: visibility("default")))
>> You can use INTERCEPTOR_ATTRIBUTE instead of visibility("default")
>>
>
> 
http://codereview.appspot.com/**6010049/diff/1/interception_**linux.h<http://...
>> File interception_linux.h (right):
>>
>
>
> http://codereview.appspot.com/**6010049/diff/1/interception_**
>
linux.h#newcode32<http://codereview.appspot.com/6010049/diff/1/interception_linux.h#newcode32>
>
>> interception_linux.h:32: (void*)&func, (void*)&WRAP(func))
>> I think that
>> && (void*)&func == (void*)&WRAP(func)
>> would be better.
>> Anyway, it's worth explaining this magic in the comment here or in
>> interception.h
>>
>
>
>
>
http://codereview.appspot.com/**6010049/<http://codereview.appspot.com/6010049/>
>