i#1734 Dr. Fuzz: add drfuzz repeat test

Commit log for first patchset:
---------------
i#1734 Dr. Fuzz: add drfuzz repeat test

- add drfuzz test to repeatedly execute a function with different args
---------------

[zhaoqin, 2015-07-31 20:33:21]

Commit log for latest patchset:
---------------
i#1734 Dr. Fuzz: add drfuzz repeat test

- add drfuzz test to repeatedly execute a function with different args

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

[Byron, 2015-08-01 03:01:22]

A couple of nits

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_ap...
File tests/framework/drfuzz_app_repeat.c (right):

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_ap...
tests/framework/drfuzz_app_repeat.c:43: /* repeatme should be re-executed 5
times with arg 1-5*/
Format: missing space at end

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_cl...
File tests/framework/drfuzz_client_repeat.c (right):

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_cl...
tests/framework/drfuzz_client_repeat.c:53: void *arg_value;
Seems better to make this a ptr_uint_t and cast to (void *). The compiler will
zero-extend properly. If the constant is a concern, you can make a macro:

#ifdef X64
# define CONST(x) xLL
#else
# define CONST(x) xL
#endif

[zhaoqin, 2015-08-02 02:59:59]

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_ap...
File tests/framework/drfuzz_app_repeat.c (right):

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_ap...
tests/framework/drfuzz_app_repeat.c:43: /* repeatme should be re-executed 5
times with arg 1-5*/
On 2015/08/01 03:01:22, Byron wrote:
> Format: missing space at end

Done.

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_cl...
File tests/framework/drfuzz_client_repeat.c (right):

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_cl...
tests/framework/drfuzz_client_repeat.c:53: void *arg_value;
On 2015/08/01 03:01:22, Byron wrote:
> Seems better to make this a ptr_uint_t and cast to (void *). The compiler will
> zero-extend properly. If the constant is a concern, you can make a macro:
> 
> #ifdef X64
> # define CONST(x) xLL
> #else
> # define CONST(x) xL
> #endif

do you think if it makes more sense set arg_value in drfuzz_get_arg to be reg_t
*, since it should be a register sized value anyway?

[Byron, 2015-08-03 18:39:08]

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_cl...
File tests/framework/drfuzz_client_repeat.c (right):

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_cl...
tests/framework/drfuzz_client_repeat.c:42: void *arg_value;
Same as below for this: please change to ptr_uint_t to reduce casting.

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_cl...
tests/framework/drfuzz_client_repeat.c:53: void *arg_value;
On 2015/08/02 02:59:59, zhaoqin wrote:
> On 2015/08/01 03:01:22, Byron wrote:
> > Seems better to make this a ptr_uint_t and cast to (void *). The compiler
will
> > zero-extend properly. If the constant is a concern, you can make a macro:
> > 
> > #ifdef X64
> > # define CONST(x) xLL
> > #else
> > # define CONST(x) xL
> > #endif
> 
> do you think if it makes more sense set arg_value in drfuzz_get_arg to be
reg_t
> *, since it should be a register sized value anyway?

Since Derek prefers the more typical (void *), I think we should go with that.
As discussed in the meeting, a constant macro isn't necessary. So I'd still like
the variable declared as ptr_uint_t since (a) that's how this function is using
the variable, and (b) it requires fewer casts.

[zhaoqin, 2015-08-03 20:24:27]

Commit log for latest patchset:
---------------
i#1734 Dr. Fuzz: add drfuzz repeat test

- add drfuzz test to repeatedly execute a function with different args

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

[zhaoqin, 2015-08-03 20:24:29]

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_cl...
File tests/framework/drfuzz_client_repeat.c (right):

https://codereview.appspot.com/251590043/diff/20001/tests/framework/drfuzz_cl...
tests/framework/drfuzz_client_repeat.c:53: void *arg_value;
On 2015/08/03 18:39:08, Byron wrote:
> On 2015/08/02 02:59:59, zhaoqin wrote:
> > On 2015/08/01 03:01:22, Byron wrote:
> > > Seems better to make this a ptr_uint_t and cast to (void *). The compiler
> will
> > > zero-extend properly. If the constant is a concern, you can make a macro:
> > > 
> > > #ifdef X64
> > > # define CONST(x) xLL
> > > #else
> > > # define CONST(x) xL
> > > #endif
> > 
> > do you think if it makes more sense set arg_value in drfuzz_get_arg to be
> reg_t
> > *, since it should be a register sized value anyway?
> 
> Since Derek prefers the more typical (void *), I think we should go with that.
> As discussed in the meeting, a constant macro isn't necessary. So I'd still
like
> the variable declared as ptr_uint_t since (a) that's how this function is
using
> the variable, and (b) it requires fewer casts.

Done.

[Byron, 2015-08-03 20:31:49]

LGTM

[zhaoqin, 2015-08-03 21:12:06]

Commit log for latest patchset:
---------------
i#1734 Dr. Fuzz: add drfuzz repeat test

- add drfuzz test to repeatedly execute a function with different args

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

[bruening, 2015-08-04 05:40:39]

LGTM w/ comments

https://codereview.appspot.com/251590043/diff/60001/tests/framework/CMakeList...
File tests/framework/CMakeLists.txt (right):

https://codereview.appspot.com/251590043/diff/60001/tests/framework/CMakeList...
tests/framework/CMakeLists.txt:94: add_drmf_test(drfuzz_test_empty
drfuzz_app_empty.c drfuzz_client_empty.c
Not sure what value this test is adding now: I thought you were going to put the
repeat logic into this test.

How did we end up with this test yet Umbra does not have a single test?

https://codereview.appspot.com/251590043/diff/60001/tests/framework/drfuzz_cl...
File tests/framework/drfuzz_client_repeat.c (right):

https://codereview.appspot.com/251590043/diff/60001/tests/framework/drfuzz_cl...
tests/framework/drfuzz_client_repeat.c:71: void dr_init(client_id_t id)
Please use the new routine instead of the legacy dr_init (yes, DR is now updated
and it is available).

https://codereview.appspot.com/251590043/diff/60001/tests/framework/drfuzz_cl...
tests/framework/drfuzz_client_repeat.c:74: generic_func_t repeatme;
Using this "generic_func_t" type is kind of confusing, b/c it has the wrong
signature for repeatme(): repeatme() takes an arg!

[Byron, 2015-08-04 05:55:06]

https://codereview.appspot.com/251590043/diff/60001/tests/framework/drfuzz_cl...
File tests/framework/drfuzz_client_repeat.c (right):

https://codereview.appspot.com/251590043/diff/60001/tests/framework/drfuzz_cl...
tests/framework/drfuzz_client_repeat.c:74: generic_func_t repeatme;
On 2015/08/04 05:40:39, bruening wrote:
> Using this "generic_func_t" type is kind of confusing, b/c it has the wrong
> signature for repeatme(): repeatme() takes an arg!

We've generally been referring to fuzz targets opaquely, only mentioning args by
number in the API. Should we change that?

[zhaoqin, 2015-08-06 15:14:05]

Committed as
https://github.com/DynamoRIO/drmemory/commit/700cd29f185ebe216a484070449632ce...

Final commit log: 
---------------
i#1734 Dr. Fuzz: add drfuzz repeat test

- add drfuzz test to repeatedly execute a function with different args
- replace dr_init with dr_client_main for all clients in tests/framework

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