i#2006 generalize drcachesim: add extern "C" for drmemtrace_client_main

Commit log for first patchset:
---------------
i#2006 generalize drcachesim: add extern "C" for drmemtrace_client_main

Adds extern "C" for drmemtrace_client_main to make sure it is exported
without mangling.
---------------

[bruening, 2016-12-08 17:04:14]

"drcachesime"

[zhaoqin, 2016-12-08 18:00:10]

Commit log for latest patchset:
---------------
i#2006 generalize drcachesim: add extern "C" for drmemtrace_client_main

Adds extern "C" for drmemtrace_client_main to make sure it is exported
without mangling.

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

[bruening, 2016-12-08 18:24:08]

LGTM w/ comment

Would be nice to edit the Rietveld issue to fix the typo in the title (yes it
bothers me...in every email, in browser title, on top of page...)

https://codereview.appspot.com/318940043/diff/20001/clients/drcachesim/tracer...
File clients/drcachesim/tracer/tracer.cpp (right):

https://codereview.appspot.com/318940043/diff/20001/clients/drcachesim/tracer...
clients/drcachesim/tracer/tracer.cpp:810: extern "C" {
Nicer to remove from here and have a declaration up top?  Avoids cluttering the
definition here and having a trailing end many lines below (but then duplicates
signature).

[bruening, 2016-12-08 18:24:36]

Also, please add a test that will catch this.

[zhaoqin, 2016-12-08 19:46:43]

Commit log for latest patchset:
---------------
i#2006 generalize drcachesim: add extern "C" for drmemtrace_client_main

Adds extern "C" for drmemtrace_client_main to make sure it is exported
without mangling.

Adds burst_client to test app's dr_client_main calling drmemtrace_client_main.

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

[zhaoqin, 2016-12-08 19:47:53]

update the Rietveld issue too.

https://codereview.appspot.com/318940043/diff/20001/clients/drcachesim/tracer...
File clients/drcachesim/tracer/tracer.cpp (right):

https://codereview.appspot.com/318940043/diff/20001/clients/drcachesim/tracer...
clients/drcachesim/tracer/tracer.cpp:810: extern "C" {
On 2016/12/08 18:24:08, bruening wrote:
> Nicer to remove from here and have a declaration up top?  Avoids cluttering
the
> definition here and having a trailing end many lines below (but then
duplicates
> signature).

Done.

[zhaoqin, 2016-12-08 20:57:54]

Commit log for latest patchset:
---------------
i#2006 generalize drcachesim: add extern "C" for drmemtrace_client_main on Linux

Adds extern "C" for drmemtrace_client_main to make sure it is exported
without mangling on Linux

Adds burst_client to test app's dr_client_main calling drmemtrace_client_main.

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

[zhaoqin, 2016-12-08 21:01:03]

Commit log for latest patchset:
---------------
i#2006 generalize drcachesim: add extern "C" for drmemtrace_client_main on Linux

Adds extern "C" for drmemtrace_client_main to make sure it is exported
without mangling on Linux

Adds burst_client to test app's dr_client_main calling drmemtrace_client_main.

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

[zhaoqin, 2016-12-08 21:13:59]

Commit log for latest patchset:
---------------
i#2006 generalize drcachesim: add extern "C" for drmemtrace_client_main on Linux

Adds extern "C" for drmemtrace_client_main to make sure it is exported
without mangling on Linux

Adds burst_client to test app's dr_client_main calling drmemtrace_client_main.

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

[bruening, 2016-12-08 21:22:45]

Several comments but shouldn't need another review so LGTM w/ comments

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/CMakeL...
File clients/drcachesim/CMakeLists.txt (right):

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/CMakeL...
clients/drcachesim/CMakeLists.txt:262: if (UNIX)
Add a comment explaining why UNIX-only and whether temp (i#) or permanent

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/CMakeL...
clients/drcachesim/CMakeLists.txt:264:
target_compile_definitions(tool.drcacheoff.burst_client
This was added in cmake 2.8.10 -- I don't think it's safe to use it here.  Best
to use what our existing cmake code is using.

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/CMakeL...
clients/drcachesim/CMakeLists.txt:268:
add_win32_flags(tool.drcacheoff.burst_client)
Nop -- I guess if the UNIX above is removed in the future (see above comment),
maybe w/ a comment saying that's why it's here

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/tests/...
File clients/drcachesim/tests/burst_static.cpp (right):

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/tests/...
clients/drcachesim/tests/burst_static.cpp:103: /* FIXME-i#2006: the weak symbol
is not supported on Windows. */
Best to remove the "-": every other instance has a space.  This will just
complicate searching.

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/tests/...
clients/drcachesim/tests/burst_static.cpp:113: fprintf(stderr, "wrong
drmemtrace_client_main\n");
use std::cerr like the existing code

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/tests/...
clients/drcachesim/tests/burst_static.cpp:120: fprintf(stderr, "app
dr_client_main\n");
use std::cerr like the existing code

[zhaoqin, 2016-12-09 00:22:16]

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/CMakeL...
File clients/drcachesim/CMakeLists.txt (right):

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/CMakeL...
clients/drcachesim/CMakeLists.txt:262: if (UNIX)
On 2016/12/08 21:22:45, bruening wrote:
> Add a comment explaining why UNIX-only and whether temp (i#) or permanent

Add i#2099.

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/CMakeL...
clients/drcachesim/CMakeLists.txt:264:
target_compile_definitions(tool.drcacheoff.burst_client
On 2016/12/08 21:22:45, bruening wrote:
> This was added in cmake 2.8.10 -- I don't think it's safe to use it here. 
Best
> to use what our existing cmake code is using.

Done.

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/CMakeL...
clients/drcachesim/CMakeLists.txt:268:
add_win32_flags(tool.drcacheoff.burst_client)
On 2016/12/08 21:22:45, bruening wrote:
> Nop -- I guess if the UNIX above is removed in the future (see above comment),
> maybe w/ a comment saying that's why it's here

Done.

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/tests/...
File clients/drcachesim/tests/burst_static.cpp (right):

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/tests/...
clients/drcachesim/tests/burst_static.cpp:103: /* FIXME-i#2006: the weak symbol
is not supported on Windows. */
On 2016/12/08 21:22:45, bruening wrote:
> Best to remove the "-": every other instance has a space.  This will just
> complicate searching.

Done.

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/tests/...
clients/drcachesim/tests/burst_static.cpp:113: fprintf(stderr, "wrong
drmemtrace_client_main\n");
On 2016/12/08 21:22:45, bruening wrote:
> use std::cerr like the existing code

Done.

https://codereview.appspot.com/318940043/diff/80001/clients/drcachesim/tests/...
clients/drcachesim/tests/burst_static.cpp:120: fprintf(stderr, "app
dr_client_main\n");
On 2016/12/08 21:22:45, bruening wrote:
> use std::cerr like the existing code

Done.

[zhaoqin, 2016-12-09 00:22:47]

Commit log for latest patchset:
---------------
i#2006 generalize drcachesim: add extern "C" for drmemtrace_client_main on Linux

Adds extern "C" for drmemtrace_client_main to make sure it is exported
without mangling on Linux

Adds burst_client to test app's dr_client_main calling drmemtrace_client_main.

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

[zhaoqin, 2016-12-09 01:07:37]

Committed as
https://github.com/DynamoRIO/dynamorio/commit/12112de76cc0d8dbf487d913476e82c...

Final commit log: 
---------------
i#2006 generalize drcachesim: add extern "C" for drmemtrace_client_main on Linux

Adds extern "C" for drmemtrace_client_main to make sure it is exported
without mangling on Linux

Adds burst_client to test app's dr_client_main calling drmemtrace_client_main.

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