i#1734 Dr. Fuzz: fixed 3 bugs in drfuzz

Commit log for first patchset:
---------------
i#1734 Dr. Fuzz: fixed 3 bugs in drfuzz

- used fuzz_target_t instead of pass_target_t to compute argsize,
  because pass_target_t is not fully initialized in create_pass_target().
- fixed wrong function call in registering thread_init function
- copy original_args to current_args for the first entry
---------------

[Byron, 2015-07-30 19:59:51]

LGTM

I already fixed all 3 of these in my working branch after merging yesterday. It
probably is not worth the effort to have two people fixing the same bugs.

[zhaoqin, 2015-07-30 20:00:48]

On 2015/07/30 19:59:51, Byron wrote:
> LGTM
> 
> I already fixed all 3 of these in my working branch after merging yesterday.
It
> probably is not worth the effort to have two people fixing the same bugs.

I am adding the test, and saw the crash, need to fix before adding test.

[zhaoqin, 2015-07-30 20:02:32]

Commit log for latest patchset:
---------------
i#1734 Dr. Fuzz: fixed 3 bugs in drfuzz

- used fuzz_target_t instead of pass_target_t to compute argsize,
  because pass_target_t is not fully initialized in create_pass_target().
- fixed wrong function call in registering thread_init function
- copy original_args to current_args for the first entry

Review-URL: https://codereview.appspot.com/259830043
---------------

[zhaoqin, 2015-07-30 20:03:29]

On 2015/07/30 20:02:32, zhaoqin wrote:
> Commit log for latest patchset:
> ---------------
> i#1734 Dr. Fuzz: fixed 3 bugs in drfuzz
> 
> - used fuzz_target_t instead of pass_target_t to compute argsize,
>   because pass_target_t is not fully initialized in create_pass_target().
> - fixed wrong function call in registering thread_init function
> - copy original_args to current_args for the first entry
> 
> Review-URL: https://codereview.appspot.com/259830043
> ---------------

Derek, can you take a look

[bruening, 2015-07-31 02:58:14]

On 2015/07/30 20:03:29, zhaoqin wrote:
> On 2015/07/30 20:02:32, zhaoqin wrote:
> > Commit log for latest patchset:
> > ---------------
> > i#1734 Dr. Fuzz: fixed 3 bugs in drfuzz
> > 
> > - used fuzz_target_t instead of pass_target_t to compute argsize,
> >   because pass_target_t is not fully initialized in create_pass_target().
> > - fixed wrong function call in registering thread_init function
> > - copy original_args to current_args for the first entry
> > 
> > Review-URL: https://codereview.appspot.com/259830043
> > ---------------
> 
> Derek, can you take a look

Is this related to i#1745?

[bruening, 2015-07-31 02:59:20]

On 2015/07/31 02:58:14, bruening wrote:
> On 2015/07/30 20:03:29, zhaoqin wrote:
> > On 2015/07/30 20:02:32, zhaoqin wrote:
> > > Commit log for latest patchset:
> > > ---------------
> > > i#1734 Dr. Fuzz: fixed 3 bugs in drfuzz
> > > 
> > > - used fuzz_target_t instead of pass_target_t to compute argsize,
> > >   because pass_target_t is not fully initialized in create_pass_target().
> > > - fixed wrong function call in registering thread_init function
> > > - copy original_args to current_args for the first entry

nit: inconsistent tense (I also find it weird to be past tense, esp in the 1st
line)

[zhaoqin, 2015-07-31 03:08:29]

On 2015/07/31 02:58:14, bruening wrote:
> On 2015/07/30 20:03:29, zhaoqin wrote:
> > On 2015/07/30 20:02:32, zhaoqin wrote:
> > > Commit log for latest patchset:
> > > ---------------
> > > i#1734 Dr. Fuzz: fixed 3 bugs in drfuzz
> > > 
> > > - used fuzz_target_t instead of pass_target_t to compute argsize,
> > >   because pass_target_t is not fully initialized in create_pass_target().
> > > - fixed wrong function call in registering thread_init function
> > > - copy original_args to current_args for the first entry
> > > 
> > > Review-URL: https://codereview.appspot.com/259830043
> > > ---------------
> > 
> > Derek, can you take a look
> 
> Is this related to i#1745?

might be, one bug is the thread_init and thread_exit are both registered as
thread exit event.
If the thread_exit is called first, I would expect some sigfault.
However, it does not happen on my machine.
Also, the callstack shows the drwrap_free_user_data, I do not think drfuzz add
any user_data yet.

[zhaoqin, 2015-07-31 03:09:48]

Commit log for latest patchset:
---------------
i#1734 Dr. Fuzz: fixes 3 bugs in drfuzz

- uses fuzz_target_t instead of pass_target_t to compute argsize,
  because pass_target_t is not fully initialized in create_pass_target().
- fixes wrong function call in registering thread_init function
- copies original_args to current_args for the first entry

Review-URL: https://codereview.appspot.com/259830043
---------------

[bruening, 2015-07-31 03:10:01]

On 2015/07/31 03:08:29, zhaoqin wrote:
> On 2015/07/31 02:58:14, bruening wrote:
> > On 2015/07/30 20:03:29, zhaoqin wrote:
> > > On 2015/07/30 20:02:32, zhaoqin wrote:
> > > > Commit log for latest patchset:
> > > > ---------------
> > > > i#1734 Dr. Fuzz: fixed 3 bugs in drfuzz
> > > > 
> > > > - used fuzz_target_t instead of pass_target_t to compute argsize,
> > > >   because pass_target_t is not fully initialized in
create_pass_target().
> > > > - fixed wrong function call in registering thread_init function
> > > > - copy original_args to current_args for the first entry
> > > > 
> > > > Review-URL: https://codereview.appspot.com/259830043
> > > > ---------------
> > > 
> > > Derek, can you take a look
> > 
> > Is this related to i#1745?
> 
> might be, one bug is the thread_init and thread_exit are both registered as
> thread exit event.
> If the thread_exit is called first, I would expect some sigfault.
> However, it does not happen on my machine.
> Also, the callstack shows the drwrap_free_user_data, I do not think drfuzz add
> any user_data yet.

I don't think the crash has anything to do with actual user_data: note how pt is
NULL, and here is the line:

    if (pt->user_data[i] != NULL) {

[bruening, 2015-07-31 03:10:29]

https://codereview.appspot.com/259830043/diff/20001/drfuzz/drfuzz.c
File drfuzz/drfuzz.c (right):

https://codereview.appspot.com/259830043/diff/20001/drfuzz/drfuzz.c#newcode338
drfuzz/drfuzz.c:338: live->current_args[i] = live->original_args[i];
Is this a bug fix?  Can the client access current_args[] or sthg?  Can you
elaborate in the commit msg?

[bruening, 2015-07-31 03:10:43]

LGTM w/ comments

[zhaoqin, 2015-07-31 03:15:53]

Commit log for latest patchset:
---------------
i#1734 Dr. Fuzz: fixes 3 bugs in drfuzz

- uses fuzz_target_t instead of pass_target_t to compute argsize,
  because pass_target_t is not fully initialized in create_pass_target().
- fixes wrong function call in registering thread_init function
- copies original_args to current_args for the first iteration of fuzzing,
  otherwise the drfuzz_get_arg returns uninit data for getting current args.

Review-URL: https://codereview.appspot.com/259830043
---------------

[zhaoqin, 2015-07-31 03:18:00]

On 2015/07/31 03:10:01, bruening wrote:
> On 2015/07/31 03:08:29, zhaoqin wrote:
> > On 2015/07/31 02:58:14, bruening wrote:
> > > On 2015/07/30 20:03:29, zhaoqin wrote:
> > > > On 2015/07/30 20:02:32, zhaoqin wrote:
> > > > > Commit log for latest patchset:
> > > > > ---------------
> > > > > i#1734 Dr. Fuzz: fixed 3 bugs in drfuzz
> > > > > 
> > > > > - used fuzz_target_t instead of pass_target_t to compute argsize,
> > > > >   because pass_target_t is not fully initialized in
> create_pass_target().
> > > > > - fixed wrong function call in registering thread_init function
> > > > > - copy original_args to current_args for the first entry
> > > > > 
> > > > > Review-URL: https://codereview.appspot.com/259830043
> > > > > ---------------
> > > > 
> > > > Derek, can you take a look
> > > 
> > > Is this related to i#1745?
> > 
> > might be, one bug is the thread_init and thread_exit are both registered as
> > thread exit event.
> > If the thread_exit is called first, I would expect some sigfault.
> > However, it does not happen on my machine.
> > Also, the callstack shows the drwrap_free_user_data, I do not think drfuzz
add
> > any user_data yet.
> 
> I don't think the crash has anything to do with actual user_data: note how pt
is
> NULL, and here is the line:
> 
>     if (pt->user_data[i] != NULL) {

Not sure, in drfuzz_init:
drwrap_init();
...
tls_idx_fuzzer = drmgr_register_tls_field();

It should not affect drwrap's pt.

[zhaoqin, 2015-07-31 03:21:59]

On 2015/07/31 03:10:43, bruening wrote:
> LGTM w/ comments

what comment, the current_args = original_args?
I guess I just submit another review before seeing you msg?

[bruening, 2015-07-31 03:38:15]

On 2015/07/31 03:21:59, zhaoqin wrote:
> On 2015/07/31 03:10:43, bruening wrote:
> > LGTM w/ comments
> 
> what comment, the current_args = original_args?
> I guess I just submit another review before seeing you msg?

That and the commit msg nit

[zhaoqin, 2015-07-31 14:13:04]

Committed as
https://github.com/DynamoRIO/drmemory/commit/2a63da9885eecd1354b85495125861b2...

Final commit log: 
---------------
i#1734 Dr. Fuzz: fixes 3 bugs in drfuzz

- uses fuzz_target_t instead of pass_target_t to compute argsize,
  because pass_target_t is not fully initialized in create_pass_target().
- fixes wrong function call in registering thread_init function
- copies original_args to current_args for the first iteration of fuzzing,
  otherwise the drfuzz_get_arg returns uninit data for getting current args.

Review-URL: https://codereview.appspot.com/259830043
---------------