i#140 client profiling: add WHERE_CLEAN_CALLEE

Commit log for first patchset:
---------------
i#140 client profiling: add WHERE_CLEAN_CALLEE

Adds a new where_am_i_t WHERE_CLEAN_CALLEE which is
set and cleared by the clean call instrumentation.
Adds several tests for profiling with clients.
---------------

[Byron, 2015-12-18 01:28:24]

Commit log for latest patchset:
---------------
i#140 client profiling: add WHERE_CLEAN_CALLEE

Adds a new where_am_i_t WHERE_CLEAN_CALLEE which is
set and cleared by the clean call instrumentation.
Adds several tests for profiling with clients.

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

[Byron, 2015-12-18 01:33:52]

https://codereview.appspot.com/283860043/diff/20001/core/arch/arch.h
File core/arch/arch.h (right):

https://codereview.appspot.com/283860043/diff/20001/core/arch/arch.h#newcode168
core/arch/arch.h:168: #define WHEREAMI_PROT_OFFSET  
((PROT_OFFS)+offsetof(dcontext_t, whereami))
I'm not sure how to name these. The clean call instrumentation gets a plain
dcontext_t (and I'm assuming that's normal since it came from
insert_get_mcontext_base(), which is used in similar instrumentation).

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c
File core/arch/mangle_shared.c (right):

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:362: /* SCRATCH_REG0 is dead here: callee clobbers xax
*/
I can't be 100% certain about this, it's just what I saw in all the cases I
tried. The suite passes this way, with 3 separate -prof_pcs test targets.

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:398: /* SCRATCH_REG0 is dead here: restore of the app
stack will clobber xax */
This seems to be more certain, since it's DR instrumentation clobbering xax.

https://codereview.appspot.com/283860043/diff/20001/suite/tests/common/protec...
File suite/tests/common/protect-dstack.c (right):

https://codereview.appspot.com/283860043/diff/20001/suite/tests/common/protec...
suite/tests/common/protect-dstack.c:86: # define
OWNING_THREAD_OFFSET_IN_DCONTEXT IF_X64_ELSE(0x220,0x1a8)
Moving dcontext.whereami changed the offset of owning_thread. But could these be
changed to use offsetof?

[bruening, 2015-12-18 18:36:11]

The two offsets should be collapsible into one.

The commit msg should point out this only happens for prof_pcs.

https://codereview.appspot.com/283860043/diff/20001/core/arch/arch.h
File core/arch/arch.h (right):

https://codereview.appspot.com/283860043/diff/20001/core/arch/arch.h#newcode80
core/arch/arch.h:80: * (see the instr_shared.c routines for dcontext instr
building)
Please see this comment above: this is why a single offset works, if the
routines like opnd_create_dcontext_field_via_reg_sz() are used.

https://codereview.appspot.com/283860043/diff/20001/core/arch/arch.h#newcode169
core/arch/arch.h:169: #define WHEREAMI_OFFSET        (offsetof(dcontext_t,
whereami))
See above and later comment: there should just be one offset define, so:

1) s/WHEREAMI_PROT_OFFSET/WHEREAMI_OFFSET/
2) delete the line above this comment

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c
File core/arch/mangle_shared.c (left):

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:318: 
?

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c
File core/arch/mangle_shared.c (right):

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:362: /* SCRATCH_REG0 is dead here: callee clobbers xax
*/
By "clobbers", do you mean that xax is scratch in all known calling conventions?

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:366: OPND_CREATE_MEMPTR(SCRATCH_REG0,
WHEREAMI_OFFSET),
Size is incorrect: you mean 32 not PTR

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:366: OPND_CREATE_MEMPTR(SCRATCH_REG0,
WHEREAMI_OFFSET),
Best to use opnd_create_dcontext_field_via_reg_sz() and the conventions used
elsewhere, to handle SELFPROT_DCONTEXT: this is why a single offset works, b/c
there's some magic inside dcontext_opnd_common() that handles unprotected vs
normal fields and relies on the offsets having certain properties (see comment
in arch.h at top of the offset defs).

Ditto below.

https://codereview.appspot.com/283860043/diff/20001/suite/tests/CMakeLists.txt
File suite/tests/CMakeLists.txt (right):

https://codereview.appspot.com/283860043/diff/20001/suite/tests/CMakeLists.tx...
suite/tests/CMakeLists.txt:2164: endif (NOT X64)
Hmm, do all these tests noticeably increase the suite length?  Maybe it's better
to only run these in the long suite.

https://codereview.appspot.com/283860043/diff/20001/suite/tests/common/protec...
File suite/tests/common/protect-dstack.c (right):

https://codereview.appspot.com/283860043/diff/20001/suite/tests/common/protec...
suite/tests/common/protect-dstack.c:86: # define
OWNING_THREAD_OFFSET_IN_DCONTEXT IF_X64_ELSE(0x220,0x1a8)
On 2015/12/18 01:33:52, Byron wrote:
> Moving dcontext.whereami changed the offset of owning_thread. But could these
be
> changed to use offsetof?

Non-trivial to use DR headers here IIRC

[Byron, 2015-12-18 21:54:50]

Commit log for latest patchset:
---------------
i#140 client profiling: add WHERE_CLEAN_CALLEE

Adds a new where_am_i_t WHERE_CLEAN_CALLEE which is
set and cleared by the clean call instrumentation,
though only when option -prof_pcs is specified
(otherwise it isn't worth the performance cost).
Adds several tests for profiling with clients.

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

[Byron, 2015-12-18 21:55:17]

https://codereview.appspot.com/283860043/diff/20001/core/arch/arch.h
File core/arch/arch.h (right):

https://codereview.appspot.com/283860043/diff/20001/core/arch/arch.h#newcode80
core/arch/arch.h:80: * (see the instr_shared.c routines for dcontext instr
building)
On 2015/12/18 18:36:10, bruening wrote:
> Please see this comment above: this is why a single offset works, if the
> routines like opnd_create_dcontext_field_via_reg_sz() are used.

ok

https://codereview.appspot.com/283860043/diff/20001/core/arch/arch.h#newcode169
core/arch/arch.h:169: #define WHEREAMI_OFFSET        (offsetof(dcontext_t,
whereami))
On 2015/12/18 18:36:10, bruening wrote:
> See above and later comment: there should just be one offset define, so:
> 
> 1) s/WHEREAMI_PROT_OFFSET/WHEREAMI_OFFSET/
> 2) delete the line above this comment

Done.

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c
File core/arch/mangle_shared.c (right):

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:362: /* SCRATCH_REG0 is dead here: callee clobbers xax
*/
On 2015/12/18 18:36:10, bruening wrote:
> By "clobbers", do you mean that xax is scratch in all known calling
conventions?

Updated comment.

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:366: OPND_CREATE_MEMPTR(SCRATCH_REG0,
WHEREAMI_OFFSET),
On 2015/12/18 18:36:10, bruening wrote:
> Size is incorrect: you mean 32 not PTR

Done.

https://codereview.appspot.com/283860043/diff/20001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:366: OPND_CREATE_MEMPTR(SCRATCH_REG0,
WHEREAMI_OFFSET),
On 2015/12/18 18:36:10, bruening wrote:
> Best to use opnd_create_dcontext_field_via_reg_sz() and the conventions used
> elsewhere, to handle SELFPROT_DCONTEXT: this is why a single offset works, b/c
> there's some magic inside dcontext_opnd_common() that handles unprotected vs
> normal fields and relies on the offsets having certain properties (see comment
> in arch.h at top of the offset defs).
> 
> Ditto below.

Done.

https://codereview.appspot.com/283860043/diff/20001/suite/tests/CMakeLists.txt
File suite/tests/CMakeLists.txt (right):

https://codereview.appspot.com/283860043/diff/20001/suite/tests/CMakeLists.tx...
suite/tests/CMakeLists.txt:2164: endif (NOT X64)
On 2015/12/18 18:36:10, bruening wrote:
> Hmm, do all these tests noticeably increase the suite length?  Maybe it's
better
> to only run these in the long suite.

It actually just adds one or two tests: sample.inscount.prof-pcs.cleancall and
sample.inscount.prof-pcs.thread-private.cleancall.

https://codereview.appspot.com/283860043/diff/20001/suite/tests/common/protec...
File suite/tests/common/protect-dstack.c (right):

https://codereview.appspot.com/283860043/diff/20001/suite/tests/common/protec...
suite/tests/common/protect-dstack.c:86: # define
OWNING_THREAD_OFFSET_IN_DCONTEXT IF_X64_ELSE(0x220,0x1a8)
On 2015/12/18 18:36:11, bruening wrote:
> On 2015/12/18 01:33:52, Byron wrote:
> > Moving dcontext.whereami changed the offset of owning_thread. But could
these
> be
> > changed to use offsetof?
> 
> Non-trivial to use DR headers here IIRC

Acknowledged.

[Byron, 2015-12-18 21:58:31]

https://codereview.appspot.com/283860043/diff/40001/core/arch/mangle_shared.c
File core/arch/mangle_shared.c (right):

https://codereview.appspot.com/283860043/diff/40001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:317: #endif
Accidentally deleted a blank line here. Corrected in my local.

[bruening, 2015-12-19 05:22:05]

LGTM

https://codereview.appspot.com/283860043/diff/20001/suite/tests/CMakeLists.txt
File suite/tests/CMakeLists.txt (right):

https://codereview.appspot.com/283860043/diff/20001/suite/tests/CMakeLists.tx...
suite/tests/CMakeLists.txt:2164: endif (NOT X64)
On 2015/12/18 21:55:17, Byron wrote:
> On 2015/12/18 18:36:10, bruening wrote:
> > Hmm, do all these tests noticeably increase the suite length?  Maybe it's
> better
> > to only run these in the long suite.
> 
> It actually just adds one or two tests: sample.inscount.prof-pcs.cleancall and
> sample.inscount.prof-pcs.thread-private.cleancall.

Oh, it's inside the "inscount" if().  OK.  Note that you could (also) add a
cross-suite run that adds -prof_pcs and -thread_private to a whole bunch of
other tests by editing the outer master list up above -- will put comment there

https://codereview.appspot.com/283860043/diff/40001/suite/tests/CMakeLists.txt
File suite/tests/CMakeLists.txt (right):

https://codereview.appspot.com/283860043/diff/40001/suite/tests/CMakeLists.tx...
suite/tests/CMakeLists.txt:105: "SHORT::LIN::ONLY::client.events$::-code_api
-no_early_inject"
See comment below: this is the outer master list, so you can add cross-cutting
things here (for long suite by not using SHORT).

[Byron, 2015-12-20 18:25:30]

Commit log for latest patchset:
---------------
i#140 client profiling: add WHERE_CLEAN_CALLEE

Adds a new where_am_i_t WHERE_CLEAN_CALLEE which is
set and cleared by the clean call instrumentation,
though only when option -prof_pcs is specified
(otherwise it isn't worth the performance cost).
Adds several tests for profiling with clients.
Adds "-prof_pcs" and "-prof_pcs -thread_private"
runs to the nightly suite.

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

[Byron, 2015-12-20 19:32:02]

With the current patch set (#4), this assert is now failing in all the tests
that use -prof_pcs and clean calls together:

|728                 ASSERT(dcontext->last_exit == get_starting_linkstub() ||
|729                        /* The start/stop API will set this linkstub. */
|730                        IF_APP_EXPORTS(dcontext->last_exit ==
get_native_exec_linkstub() || )
|731                        /* new thread */
|732                        IF_WINDOWS_ELSE_0(dcontext->last_exit ==
get_asynch_linkstub()));  

I'm going to leave this CL open with this issue, since I may not be able to
resolve it until after the holidays.

https://codereview.appspot.com/283860043/diff/20001/suite/tests/CMakeLists.txt
File suite/tests/CMakeLists.txt (right):

https://codereview.appspot.com/283860043/diff/20001/suite/tests/CMakeLists.tx...
suite/tests/CMakeLists.txt:2164: endif (NOT X64)
On 2015/12/19 05:22:05, bruening wrote:
> On 2015/12/18 21:55:17, Byron wrote:
> > On 2015/12/18 18:36:10, bruening wrote:
> > > Hmm, do all these tests noticeably increase the suite length?  Maybe it's
> > better
> > > to only run these in the long suite.
> > 
> > It actually just adds one or two tests: sample.inscount.prof-pcs.cleancall
and
> > sample.inscount.prof-pcs.thread-private.cleancall.
> 
> Oh, it's inside the "inscount" if().  OK.  Note that you could (also) add a
> cross-suite run that adds -prof_pcs and -thread_private to a whole bunch of
> other tests by editing the outer master list up above -- will put comment
there

Done.

https://codereview.appspot.com/283860043/diff/40001/suite/tests/CMakeLists.txt
File suite/tests/CMakeLists.txt (right):

https://codereview.appspot.com/283860043/diff/40001/suite/tests/CMakeLists.tx...
suite/tests/CMakeLists.txt:105: "SHORT::LIN::ONLY::client.events$::-code_api
-no_early_inject"
On 2015/12/19 05:22:05, bruening wrote:
> See comment below: this is the outer master list, so you can add cross-cutting
> things here (for long suite by not using SHORT).

Done.

[Byron, 2016-01-09 01:15:38]

Commit log for latest patchset:
---------------
i#140 client profiling: add WHERE_CLEAN_CALLEE

Adds a new where_am_i_t WHERE_CLEAN_CALLEE which is
set and cleared by the clean call instrumentation,
though only when option -prof_pcs is specified
(otherwise it isn't worth the performance cost).
Adds several tests for profiling with clients.
Adds "-prof_pcs" and "-prof_pcs -thread_private"
runs to the nightly suite.

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

[Byron, 2016-01-09 01:22:02]

Fixing an issue that occurs in the new long suite tests that combine "-prof_pcs"
with clean calls.

https://codereview.appspot.com/283860043/diff/80001/core/arch/mangle_shared.c
File core/arch/mangle_shared.c (right):

https://codereview.appspot.com/283860043/diff/80001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:406: if (callee == return_to_native || callee ==
call_to_native)
If there may be other clean calls that return to native, we could add a flag arg
"returns_to_native" instead, though it will affect the internal API in several
places. In that case would it be better to add a "flags" arg that combines
"clean_call", "returns", and "returns_to_native"?

[bruening, 2016-01-11 21:43:22]

LGTM

https://codereview.appspot.com/283860043/diff/80001/core/arch/mangle_shared.c
File core/arch/mangle_shared.c (right):

https://codereview.appspot.com/283860043/diff/80001/core/arch/mangle_shared.c...
core/arch/mangle_shared.c:406: if (callee == return_to_native || callee ==
call_to_native)
On 2016/01/09 01:22:02, Byron wrote:
> If there may be other clean calls that return to native, we could add a flag
arg
> "returns_to_native" instead, though it will affect the internal API in several
> places. In that case would it be better to add a "flags" arg that combines
> "clean_call", "returns", and "returns_to_native"?

Reading and restoring the prior value would be too expensive.  Flags arg would
be ok.  If going with hardcoded exclusions, it feels like there should be
comments at their impl points or sthg, or some abstraction like a function that
does this check, instead of just a list here?

[Byron, 2016-01-14 23:15:45]

Commit log for latest patchset:
---------------
i#140 client profiling: add WHERE_CLEAN_CALLEE

Add a new where_am_i_t WHERE_CLEAN_CALLEE which is
set and cleared by the clean call instrumentation,
though only when option -prof_pcs is specified
(otherwise it isn't worth the performance cost).
Combine bool args to insert_meta_call into a new
flags arg (does not affect the public API). Add
several tests for profiling with clients. Add
"-prof_pcs" and "-prof_pcs -thread_private" runs
to the long suite.

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

[Byron, 2016-01-14 23:17:58]

Commit log for latest patchset:
---------------
i#140 client profiling: add WHERE_CLEAN_CALLEE

Add a new where_am_i_t WHERE_CLEAN_CALLEE which is
set and cleared by the clean call instrumentation,
though only when option -prof_pcs is specified
(otherwise it isn't worth the performance cost).
Combine bool args to insert_meta_call into a new
flags arg (does not affect the public API). Add
several tests for profiling with clients. Add
"-prof_pcs" and "-prof_pcs -thread_private" runs
to the long suite.

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

[Byron, 2016-01-14 23:24:33]

Several questions about adding flags for insert_meta_call:

https://codereview.appspot.com/283860043/diff/120001/core/arch/arch.h
File core/arch/arch.h (right):

https://codereview.appspot.com/283860043/diff/120001/core/arch/arch.h#newcode352
core/arch/arch.h:352: META_CALL_CLEAN_CALL        = 1,
or META_CALL_FLAGS_CLEAN_CALL?

https://codereview.appspot.com/283860043/diff/120001/core/arch/interp.c
File core/arch/interp.c (right):

https://codereview.appspot.com/283860043/diff/120001/core/arch/interp.c#newco...
core/arch/interp.c:4659: dr_insert_clean_call_ex(dcontext, bb->ilist, NULL,
Changing to _ex because the public API doesn't have the new returns_to_native
option (to avoid compatibility issues).

https://codereview.appspot.com/283860043/diff/120001/core/lib/instrument.c
File core/lib/instrument.c (right):

https://codereview.appspot.com/283860043/diff/120001/core/lib/instrument.c#ne...
core/lib/instrument.c:4967: insert_meta_call_vargs(dcontext, ilist, where, 0,
vmcode_get_start(), callee,
Should there be a META_CALL_NONE, to use instead of 0?

https://codereview.appspot.com/283860043/diff/120001/core/lib/instrument.c#ne...
core/lib/instrument.c:5187: dr_insert_clean_call(void *drcontext, instrlist_t
*ilist, instr_t *where,
Not adding the flag to the public API for compatibility reasons.

https://codereview.appspot.com/283860043/diff/120001/core/lib/instrument.h
File core/lib/instrument.h (right):

https://codereview.appspot.com/283860043/diff/120001/core/lib/instrument.h#ne...
core/lib/instrument.h:5021: bool returns_to_native, uint num_args, ...);
Using a bool instead of flags because 2 of the 3 flags are set within.

https://codereview.appspot.com/283860043/diff/120001/core/lib/instrument.h#ne...
core/lib/instrument.h:5038: bool returns_to_native, uint num_args, opnd_t
*args);
ditto

[bruening, 2016-01-15 15:38:50]

See final comment -- this CL is changing the public API.  Plus, we already have
a bitfield of flags for clean calls.  I would vote for using that for at least
some of these 3 options, maybe all.

https://codereview.appspot.com/283860043/diff/120001/core/arch/arch.h
File core/arch/arch.h (right):

https://codereview.appspot.com/283860043/diff/120001/core/arch/arch.h#newcode352
core/arch/arch.h:352: META_CALL_CLEAN_CALL        = 1,
On 2016/01/14 23:24:33, Byron wrote:
> or META_CALL_FLAGS_CLEAN_CALL?

META_CALL_CLEAN_CALL seems fine.  No need for FLAGS.  Could even be
META_CALL_CLEAN.

https://codereview.appspot.com/283860043/diff/120001/core/arch/arch.h#newcode354
core/arch/arch.h:354: META_CALL_RETURNS_TO_NATIVE = 4,
Best to use hex for bitfields

https://codereview.appspot.com/283860043/diff/120001/core/lib/instrument.h
File core/lib/instrument.h (right):

https://codereview.appspot.com/283860043/diff/120001/core/lib/instrument.h#ne...
core/lib/instrument.h:5021: bool returns_to_native, uint num_args, ...);
On 2016/01/14 23:24:33, Byron wrote:
> Using a bool instead of flags because 2 of the 3 flags are set within.

So this is changing the API.  But we already have a set of flags in the param
dr_cleancall_save_t save_flags -- why not simply add a
DR_CLEANCALL_RETURNS_TO_NATIVE to that enum and avoid having to add a bool?

In fact, maybe all 3 meta call flags could go into that enum, or at least 2 of
them and then the insert_meta_call_vargs takes dr_cleancall_save_t and a bool or
sthg.

To put all 3 in, I guess you'd have DR_CLEANCALL_NOT_CLEAN??

[Byron, 2016-01-15 23:46:55]

Commit log for latest patchset:
---------------
i#140 client profiling: add WHERE_CLEAN_CALLEE

Add a new where_am_i_t WHERE_CLEAN_CALLEE which is
set and cleared by the clean call instrumentation,
though only when option -prof_pcs is specified
(otherwise it isn't worth the performance cost).
Combine bool args to insert_meta_call into a new
flags arg (does not affect the public API). Add
several tests for profiling with clients. Add
"-prof_pcs" and "-prof_pcs -thread_private" runs
to the long suite.

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

[Byron, 2016-01-15 23:47:19]

https://codereview.appspot.com/283860043/diff/120001/core/arch/arch.h
File core/arch/arch.h (right):

https://codereview.appspot.com/283860043/diff/120001/core/arch/arch.h#newcode352
core/arch/arch.h:352: META_CALL_CLEAN_CALL        = 1,
On 2016/01/15 15:38:50, bruening wrote:
> On 2016/01/14 23:24:33, Byron wrote:
> > or META_CALL_FLAGS_CLEAN_CALL?
> 
> META_CALL_CLEAN_CALL seems fine.  No need for FLAGS.  Could even be
> META_CALL_CLEAN.

Done.

https://codereview.appspot.com/283860043/diff/120001/core/arch/arch.h#newcode354
core/arch/arch.h:354: META_CALL_RETURNS_TO_NATIVE = 4,
On 2016/01/15 15:38:50, bruening wrote:
> Best to use hex for bitfields

Done.

https://codereview.appspot.com/283860043/diff/120001/core/lib/instrument.h
File core/lib/instrument.h (right):

https://codereview.appspot.com/283860043/diff/120001/core/lib/instrument.h#ne...
core/lib/instrument.h:5021: bool returns_to_native, uint num_args, ...);
On 2016/01/15 15:38:50, bruening wrote:
> On 2016/01/14 23:24:33, Byron wrote:
> > Using a bool instead of flags because 2 of the 3 flags are set within.
> 
> So this is changing the API.  But we already have a set of flags in the param
> dr_cleancall_save_t save_flags -- why not simply add a
> DR_CLEANCALL_RETURNS_TO_NATIVE to that enum and avoid having to add a bool?
> 
> In fact, maybe all 3 meta call flags could go into that enum, or at least 2 of
> them and then the insert_meta_call_vargs takes dr_cleancall_save_t and a bool
or
> sthg.
> 
> To put all 3 in, I guess you'd have DR_CLEANCALL_NOT_CLEAN??

I think the most readable approach is to add DR_CLEANCALL_RETURNS_TO_NATIVE and
map it to the new meta enum. Otherwise it seems like the enum names are hard to
understand at the call site.

[bruening, 2016-01-16 03:10:50]

LGTM

[Byron, 2016-01-16 03:27:04]

Committed as
https://github.com/DynamoRIO/dynamorio/commit/6036c948e027b2f5665eb2295f45459...

Final commit log: 
---------------
i#140 client profiling: add WHERE_CLEAN_CALLEE

Add a new where_am_i_t WHERE_CLEAN_CALLEE which is
set and cleared by the clean call instrumentation,
though only when option -prof_pcs is specified
(otherwise it isn't worth the performance cost).
Combine bool args to insert_meta_call into a new
flags arg (does not affect the public API). Add
several tests for profiling with clients. Add
"-prof_pcs" and "-prof_pcs -thread_private" runs
to the long suite.

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

[Byron, 2016-01-16 03:58:44]

Forgot to update, so the actual commit is
https://github.com/DynamoRIO/dynamorio/commit/2675a1b95efb54584dd5aff7579d20e....