[llvm-commits] [ASan] Intercept CreateThread on Windows (issue 5699064)

[timurrrr at google.com]

Done, PTAL


http://codereview.appspot.com/5699064/diff/4001/lib/asan/asan_interceptors_win.h
File lib/asan/asan_interceptors_win.h (right):

http://codereview.appspot.com/5699064/diff/4001/lib/asan/asan_interceptors_win.h#newcode21
lib/asan/asan_interceptors_win.h:21:
On 2012/02/24 14:35:53, samsonov wrote:
> iwyu "interception/interception.h" ? Or it's 2-step reverse include
guard?

On the second thought, I've decided to move the below code into
asan_interceptors.cc and avoid creating a new file for now.

http://codereview.appspot.com/5699064/diff/4001/lib/asan/asan_internal.h
File lib/asan/asan_internal.h (right):

http://codereview.appspot.com/5699064/diff/4001/lib/asan/asan_internal.h#newcode263
lib/asan/asan_internal.h:263: # define THREADPROC
On 2012/02/24 14:36:40, ramosian.glider wrote:
> THREAD_CALLING_CONV or something more readable.

Done.

http://codereview.appspot.com/5699064/diff/4001/lib/asan/asan_internal.h#newcode281
lib/asan/asan_internal.h:281: typedef ThreadReturnType (THREADPROC
*ThreadProc)(void* param);
On 2012/02/24 14:36:40, ramosian.glider wrote:
> I think "thread_return_t" is more consistent with the rest of the
code.

Done.

http://codereview.appspot.com/5699064/diff/4001/lib/asan/asan_thread.h
File lib/asan/asan_thread.h (right):

http://codereview.appspot.com/5699064/diff/4001/lib/asan/asan_thread.h#newcode68
lib/asan/asan_thread.h:68: static AsanThread *Create(int parent_tid,
ThreadProc start_routine,
On 2012/02/24 14:36:40, ramosian.glider wrote:
> thread_callback_t, worker_t?

Done.

http://codereview.appspot.com/5699064/diff/4001/lib/asan/asan_thread.h#newcode73
lib/asan/asan_thread.h:73: ThreadReturnType ThreadStart();
On 2012/02/24 14:35:53, samsonov wrote:
> ThreadStartRoutineReturnType?
thread_return_t

http://codereview.appspot.com/5699064/diff/4001/lib/asan/interception/interception.h
File lib/asan/interception/interception.h (right):

http://codereview.appspot.com/5699064/diff/4001/lib/asan/interception/interception.h#newcode108
lib/asan/interception/interception.h:108: // without defining
INTERCEPTOR(..., foo, ...). For example, if you override
On 2012/02/24 14:36:40, ramosian.glider wrote:
> Please move this comment below or rewrite it.

Done.

http://codereview.appspot.com/5699064/diff/4001/lib/asan/interception/interception.h#newcode109
lib/asan/interception/interception.h:109: // foo with an interceptor for
other function.
On 2012/02/24 14:35:53, samsonov wrote:
> Move this comment before DEFINE_REAL?

Done.

http://codereview.appspot.com/5699064/diff/4001/lib/asan/interception/interception.h#newcode117
lib/asan/interception/interception.h:117: DEFINE_REAL_EX(ret_type, ,
func, __VA_ARGS__);
On 2012/02/24 14:36:40, ramosian.glider wrote:
> I think it's better to put something here as a missing argument.
> (), maybe?

Done.

http://codereview.appspot.com/5699064/diff/4001/lib/asan/interception/interception.h#newcode126
lib/asan/interception/interception.h:126: INTERCEPTOR_EX(ret_type, ,
func, __VA_ARGS__)
On 2012/02/24 14:35:53, samsonov wrote:
> ...ret_type, /*no convention*/, func, ... ?

Done.

http://codereview.appspot.com/5699064/diff/4001/lib/asan/interception/interception.h#newcode128
lib/asan/interception/interception.h:128: #define
INTERCEPTOR_WINAPI(ret_type, func, ...) \
On 2012/02/24 14:35:53, samsonov wrote:
> Do we want to have similar macro for other OS?
We might, but highly unlikely we'll need to.

> Maybe, it's better to move this
> into interception_win.h and add comment here?

http://codereview.appspot.com/5699064/