add 'current' option to drcpusim, and allow use of drcpusim on all target architectures

Commit log for first patchset:
---------------
add 'current' option to drcpusim, and allow use of drcpusim on all target architectures
---------------

[bruening, 2016-02-23 03:16:13]

https://codereview.appspot.com/285570043/diff/1/clients/drcpusim/drcpusim.cpp
File clients/drcpusim/drcpusim.cpp (right):

https://codereview.appspot.com/285570043/diff/1/clients/drcpusim/drcpusim.cpp...
clients/drcpusim/drcpusim.cpp:728: return true;
This doesn't seem quite right: I assume "current" means the physical CPU being
used.  Thus, this needs to identify that CPU and pick the proper opcode set to
check.  I could target a binary that contains illegal opcodes for the current
physical CPU and the code as you have it would say that everything is legal, yet
the binary will hit an illegal instruction fault.

Perhaps an alternative model to proactive checking for "current" would be to
wait for an illegal instr fault, though it seems more in line with the rest of
the code to just point at the existing model.

[al.grant, 2016-02-23 10:13:23]

On 2016/02/23 03:16:13, bruening wrote:
> This doesn't seem quite right: I assume "current" means the physical CPU being
> used.  Thus, this needs to identify that CPU and pick the proper opcode set to
> check.  I could target a binary that contains illegal opcodes for the current
> physical CPU and the code as you have it would say that everything is legal,
yet
> the binary will hit an illegal instruction fault.
>
> Perhaps an alternative model to proactive checking for "current" would be to
> wait for an illegal instr fault, though it seems more in line with the rest of
> the code to just point at the existing model.

My idea was basically to have a baseline that other architectures can start
from, to check that drcpusim's basic mechanism (intercepting instructions)
is working, and not because "current" is useful in itself.  As you say, if an
unsupported instruction isn't faulted by drcpusim it will be faulted when
run in the code cache.  So if the user is using drcpusim to validate that
they aren't shipping code that will fail on some target system, it will
still fail, it just won't be picked up by drcpusim.  So it's not as if the
mode is giving misleading results.

Would you be happier if the mode was called "passthrough" or "null" or 
something else that made it clear that it wasn't likely to be particularly
useful in itself?

Of course, I will be following this up with patches to add checks for
ARM architecture variants.

[bruening, 2016-02-23 15:46:08]

On 2016/02/23 10:13:23, al.grant wrote:
> On 2016/02/23 03:16:13, bruening wrote:
> > This doesn't seem quite right: I assume "current" means the physical CPU
being
> > used.  Thus, this needs to identify that CPU and pick the proper opcode set
to
> > check.  I could target a binary that contains illegal opcodes for the
current
> > physical CPU and the code as you have it would say that everything is legal,
> yet
> > the binary will hit an illegal instruction fault.
> >
> > Perhaps an alternative model to proactive checking for "current" would be to
> > wait for an illegal instr fault, though it seems more in line with the rest
of
> > the code to just point at the existing model.
> 
> My idea was basically to have a baseline that other architectures can start
> from, to check that drcpusim's basic mechanism (intercepting instructions)
> is working, and not because "current" is useful in itself.  As you say, if an
> unsupported instruction isn't faulted by drcpusim it will be faulted when
> run in the code cache.  So if the user is using drcpusim to validate that
> they aren't shipping code that will fail on some target system, it will
> still fail, it just won't be picked up by drcpusim.  So it's not as if the
> mode is giving misleading results.

I would argue that is giving misleading results for a number of reasons:

1) It is failing in its base claim to report an error on an illegal instr.  A
user testing it on -current might reasonably conclude it's just broken.

2) Not all illegal instrs we might check for produce a fault: they can be
undefined and nothing visible happens on this particular model, making the error
reporting useful regardless of faults.

3) If I run it with -current on processor type A, and then run it with -cpu A on
processor type B, on the same binary, the results will differ, which is
incorrect.

4) Users may be testing their handling of our error reports.

> Would you be happier if the mode was called "passthrough" or "null" or 
> something else that made it clear that it wasn't likely to be particularly
> useful in itself?

I don't understand how it's useful to an end user: maybe there would be an
internal undocumented option.