i#1834 memval Part 1: drx_terminate_basic_block

Commit log for first patchset:
---------------
i#1834 memval Part 1: drx_terminate_basic_block

Adds a drx_terminate_basic_block() method which constructs a zero-cost
label at the end of any basic block which is already non-terminated.
This facilitates the ability to add instrumentation after the current
instruction even if the basic block is not terminated with a special
instruction.
---------------

[toshi, 2017-01-18 20:37:45]

I got the timeout error that Branden mentioned not too long ago, so the
interactive diff isn't available but the static inline diff is. Should I resend
the patch set?

[bruening, 2017-01-18 22:46:09]

On 2017/01/18 20:37:45, toshi wrote:
< Adds a drx_terminate_basic_block() method which constructs a zero-cost
> label at the end of any basic block which is already non-terminated.
> This facilitates the ability to add instrumentation after the current
> instruction even if the basic block is not terminated with a special
> instruction.

Hmm, I don't think the term "terminated" is clear, nor is "special instruction".
 Maybe "branch-terminated".

To me "drx_terminate_basic_block" sounds like it's going to truncate the block
early and delete the trailing instrs, cutting it in half or something.

What do you think of these alternative names:

+ drx_tail_pad_block
+ drx_append_label

> I got the timeout error that Branden mentioned not too long ago, so the
> interactive diff isn't available but the static inline diff is. Should I
resend
> the patch set?

Yes please.  Also if you are interested in trying the new github review system
and rebase pull requests I wouldn't mind trying them out, given the auth and
other issues Rietveld has made us deal with and given that many new contributors
are more used to pull requests.

[toshi, 2017-01-19 03:00:14]

On 2017/01/18 22:46:09, bruening wrote:
> On 2017/01/18 20:37:45, toshi wrote:
> < Adds a drx_terminate_basic_block() method which constructs a zero-cost
> > label at the end of any basic block which is already non-terminated.
> > This facilitates the ability to add instrumentation after the current
> > instruction even if the basic block is not terminated with a special
> > instruction.
> 
> Hmm, I don't think the term "terminated" is clear, nor is "special
instruction".
>  Maybe "branch-terminated".
> 
> To me "drx_terminate_basic_block" sounds like it's going to truncate the block
> early and delete the trailing instrs, cutting it in half or something.
> 
> What do you think of these alternative names:
> 
> + drx_tail_pad_block
> + drx_append_label
> 
> > I got the timeout error that Branden mentioned not too long ago, so the
> > interactive diff isn't available but the static inline diff is. Should I
> resend
> > the patch set?
> 
> Yes please.  Also if you are interested in trying the new github review system
> and rebase pull requests I wouldn't mind trying them out, given the auth and
> other issues Rietveld has made us deal with and given that many new
contributors
> are more used to pull requests.

Will do, thanks!

[bruening, 2017-01-19 20:14:23]

On 2017/01/19 03:00:14, toshi wrote:
> On 2017/01/18 22:46:09, bruening wrote:
> > On 2017/01/18 20:37:45, toshi wrote:
> > < Adds a drx_terminate_basic_block() method which constructs a zero-cost
> > > label at the end of any basic block which is already non-terminated.
> > > This facilitates the ability to add instrumentation after the current
> > > instruction even if the basic block is not terminated with a special
> > > instruction.
> > 
> > Hmm, I don't think the term "terminated" is clear, nor is "special
> instruction".
> >  Maybe "branch-terminated".
> > 
> > To me "drx_terminate_basic_block" sounds like it's going to truncate the
block
> > early and delete the trailing instrs, cutting it in half or something.
> > 
> > What do you think of these alternative names:
> > 
> > + drx_tail_pad_block
> > + drx_append_label
> > 
> > > I got the timeout error that Branden mentioned not too long ago, so the
> > > interactive diff isn't available but the static inline diff is. Should I
> > resend
> > > the patch set?
> > 
> > Yes please.  Also if you are interested in trying the new github review
system
> > and rebase pull requests I wouldn't mind trying them out, given the auth and
> > other issues Rietveld has made us deal with and given that many new
> contributors
> > are more used to pull requests.
> 
> Will do, thanks!

https://github.com/DynamoRIO/dynamorio/pull/2143