i#2006 generalize drcachesim: add drmodtrack_dump_buf

Commit log for first patchset:
---------------
i#2006 generalize drcachesim: add drmodtrack_dump_buf

Adds drmodtrack_dump_ex to dump module info string to the  provided buffer.
---------------

[zhaoqin, 2016-10-19 02:59:48]

Commit log for latest patchset:
---------------
i#2006 generalize drcachesim: add drmodtrack_dump_ex

Adds drmodtrack_dump_ex to dump module info using provided function.

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

[bruening, 2016-10-19 21:37:36]

https://codereview.appspot.com/316770043/diff/20001/ext/drcovlib/drcovlib.h
File ext/drcovlib/drcovlib.h (right):

https://codereview.appspot.com/316770043/diff/20001/ext/drcovlib/drcovlib.h#n...
ext/drcovlib/drcovlib.h:239: * function \p write_file_func.
WDYT of writing to a buffer instead, which would be more general and flexible.

https://codereview.appspot.com/316770043/diff/20001/ext/drcovlib/drcovlib.h#n...
ext/drcovlib/drcovlib.h:240: * The information can be read back in using \p
drmodtrack_offline_read().
It can?  That takes in a file_t.

[zhaoqin, 2016-10-20 01:01:20]

https://codereview.appspot.com/316770043/diff/20001/ext/drcovlib/drcovlib.h
File ext/drcovlib/drcovlib.h (right):

https://codereview.appspot.com/316770043/diff/20001/ext/drcovlib/drcovlib.h#n...
ext/drcovlib/drcovlib.h:239: * function \p write_file_func.
On 2016/10/19 21:37:36, bruening wrote:
> WDYT of writing to a buffer instead, which would be more general and flexible.

then how do we decide the buffer size.
There could be dozens of modules and each has a path.
What if the buf size is not enough?

https://codereview.appspot.com/316770043/diff/20001/ext/drcovlib/drcovlib.h#n...
ext/drcovlib/drcovlib.h:240: * The information can be read back in using \p
drmodtrack_offline_read().
On 2016/10/19 21:37:36, bruening wrote:
> It can?  That takes in a file_t.

Yes, so long it can read the file.
They would be in different process, so file_t would be good in our case.

[bruening, 2016-10-20 02:47:16]

https://codereview.appspot.com/316770043/diff/20001/ext/drcovlib/drcovlib.h
File ext/drcovlib/drcovlib.h (right):

https://codereview.appspot.com/316770043/diff/20001/ext/drcovlib/drcovlib.h#n...
ext/drcovlib/drcovlib.h:239: * function \p write_file_func.
On 2016/10/20 01:01:20, zhaoqin wrote:
> On 2016/10/19 21:37:36, bruening wrote:
> > WDYT of writing to a buffer instead, which would be more general and
flexible.
> 
> then how do we decide the buffer size.
> There could be dozens of modules and each has a path.
> What if the buf size is not enough?

In the past when we generalized interfaces to allow specifying where to write
things, we ended up having to extend them yet again for situations where a
direct file write didn't work (e.g., disassembly).  I'm trying to avoid
repeating that mistake here.  Since the module list is relatively small, it
seems like the most flexible interface.

For the size, the typical method (used by system calls, etc.) is to return the
size needed when the buffer is too small.

[zhaoqin, 2016-10-20 17:56:03]

Commit log for latest patchset:
---------------
i#2006 generalize drcachesim: add drmodtrack_dump_buf

Adds drmodtrack_dump_buf to dump module info string into a provided
buffer.

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

[bruening, 2016-10-21 02:10:43]

LGTM w/ a few comments

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/drcovlib.h
File ext/drcovlib/drcovlib.h (right):

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/drcovlib.h#n...
ext/drcovlib/drcovlib.h:241: * If the buffer is too small, returns
DRCOVLIB_ERROR_BUF_TOO_SMALL.
Hmm, ok, so the caller may have to try multiple times: I thought it was going to
return the required size, but I guess that is not simple in the current code
without having a big enough internal buffer or sthg.

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/modules.c
File ext/drcovlib/modules.c (right):

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/modules.c#ne...
ext/drcovlib/modules.c:322: len = dr_snprintf(buf, size, "%3u, " PFX ", " PFX ",
" PFX "",
OK, adding spaces in case this gets converted to C++ I guess: may as well in new
code

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/modules.c#ne...
ext/drcovlib/modules.c:336: size -= len;
Hmm, I guess print_to_buffer is inside DR and drfront_bufprint is not accessible
here.

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/modules.c#ne...
ext/drcovlib/modules.c:410: size_t size = 1000;
I'd make this bigger so there's no retry for all but extreme cases: I'd go to 8K
or sthg.  Plus, you know the the # of modules, may as well scale it by that:
maybe as much as 512 * num-of-modules?

[zhaoqin, 2016-10-21 17:56:57]

Commit log for latest patchset:
---------------
i#2006 generalize drcachesim: add drmodtrack_dump_buf

Adds drmodtrack_dump_buf to dump module info string into a provided
buffer.

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

[zhaoqin, 2016-10-21 18:11:27]

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/drcovlib.h
File ext/drcovlib/drcovlib.h (right):

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/drcovlib.h#n...
ext/drcovlib/drcovlib.h:241: * If the buffer is too small, returns
DRCOVLIB_ERROR_BUF_TOO_SMALL.
On 2016/10/21 02:10:43, bruening wrote:
> Hmm, ok, so the caller may have to try multiple times: I thought it was going
to
> return the required size, but I guess that is not simple in the current code
> without having a big enough internal buffer or sthg.

Acknowledged.

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/modules.c
File ext/drcovlib/modules.c (right):

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/modules.c#ne...
ext/drcovlib/modules.c:322: len = dr_snprintf(buf, size, "%3u, " PFX ", " PFX ",
" PFX "",
On 2016/10/21 02:10:43, bruening wrote:
> OK, adding spaces in case this gets converted to C++ I guess: may as well in
new
> code

Acknowledged.

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/modules.c#ne...
ext/drcovlib/modules.c:336: size -= len;
On 2016/10/21 02:10:43, bruening wrote:
> Hmm, I guess print_to_buffer is inside DR and drfront_bufprint is not
accessible
> here.

Acknowledged.

https://codereview.appspot.com/316770043/diff/40001/ext/drcovlib/modules.c#ne...
ext/drcovlib/modules.c:410: size_t size = 1000;
On 2016/10/21 02:10:43, bruening wrote:
> I'd make this bigger so there's no retry for all but extreme cases: I'd go to
8K
> or sthg.  Plus, you know the the # of modules, may as well scale it by that:
> maybe as much as 512 * num-of-modules?

Done.

[zhaoqin, 2016-10-23 02:13:36]

Committed as
https://github.com/DynamoRIO/dynamorio/commit/b060a056ce600560ff1b2ba72337285...

Final commit log: 
---------------
i#2006 generalize drcachesim: add drmodtrack_dump_buf

Adds drmodtrack_dump_buf to dump module info string into a provided
buffer.

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