[MIPS] Add support for debug stub test

[MIPS] Add support for debug stub test

Add MIPS specific parts for this debug stub test, including:

- definition of MIPS_REG_DEFS,
- c code that sets expected register values in untrusted code,
- python code that checks for expected values in registers,
- other definitions and conditions needed for the test.

BUG= run_debug_stub_test for MIPS
TEST= run run_debug_stub_test on MIPS-HW

Committed: http://src.chromium.org/viewvc/native_client?view=rev&revision=12570

[petarj, 2013-12-26 16:16:49]

PTAL.

[Mark Seaborn, 2013-12-26 17:01:13]

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
File tests/debug_stub/debug_stub_test.py (right):

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debug_stub_test.py:332: # We skip zero register because it can
not be set.
Nit: 'cannot' is one word

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debug_stub_test.py:341: self.assertEquals(registers['t1'],
0x99000AA0)
Nit: use lower-case hex to match code above

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debug_stub_test.py:358: # Skip k0 and k1 registers, since they
can be changed by kernel.
Do you mean "can only"?

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debug_stub_test.py:446: if ARCH == 'arm' or ARCH == 'mips32':
To reduce duplication, can you factor out this check into:
def SingleSteppingWorks():
  return ARCH in ('x86-32', 'x86-64')

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
File tests/debug_stub/debugger_test.c (right):

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debugger_test.c:43: ".word 0x0000000d\n"
Can you comment the instruction that this encodes?

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debugger_test.c:133: /* Skip k0 and k1 registers, since they
can be changed by kernel. */
"can only"?

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debugger_test.c:194: #elif defined(__arm__)
"|| defined(__mips__)" for brevity?

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debugger_test.c:232: ".word 0x0000000d\n"
ditto

[petarj, 2013-12-26 18:36:24]

Second patch uploaded.

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
File tests/debug_stub/debug_stub_test.py (right):

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debug_stub_test.py:332: # We skip zero register because it can
not be set.
On 2013/12/26 17:01:13, Mark Seaborn wrote:
> Nit: 'cannot' is one word

'can not' is also correct and acceptable from the grammar point of view.
I will change it, though.

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debug_stub_test.py:341: self.assertEquals(registers['t1'],
0x99000AA0)
On 2013/12/26 17:01:13, Mark Seaborn wrote:
> Nit: use lower-case hex to match code above

Done.

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debug_stub_test.py:358: # Skip k0 and k1 registers, since they
can be changed by kernel.
On 2013/12/26 17:01:13, Mark Seaborn wrote:
> Do you mean "can only"?

No, application can change it too, but it makes no sense since it can not
rely on the values.

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debug_stub_test.py:446: if ARCH == 'arm' or ARCH == 'mips32':
On 2013/12/26 17:01:13, Mark Seaborn wrote:
> To reduce duplication, can you factor out this check into:
> def SingleSteppingWorks():
>   return ARCH in ('x86-32', 'x86-64')

Not applicable here, but done elsewhere.

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
File tests/debug_stub/debugger_test.c (right):

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debugger_test.c:43: ".word 0x0000000d\n"
On 2013/12/26 17:01:13, Mark Seaborn wrote:
> Can you comment the instruction that this encodes?

Done.

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debugger_test.c:133: /* Skip k0 and k1 registers, since they
can be changed by kernel. */
On 2013/12/26 17:01:13, Mark Seaborn wrote:
> "can only"?

See the comment elsewhere.

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debugger_test.c:194: #elif defined(__arm__)
On 2013/12/26 17:01:13, Mark Seaborn wrote:
> "|| defined(__mips__)" for brevity?

Done.

https://chromiumcodereview.appspot.com/107313005/diff/20001/tests/debug_stub/...
tests/debug_stub/debugger_test.c:232: ".word 0x0000000d\n"
On 2013/12/26 17:01:13, Mark Seaborn wrote:
> ditto

Done.

[Mark Seaborn, 2013-12-26 23:23:42]

LGTM, thanks

https://chromiumcodereview.appspot.com/107313005/diff/320001/tests/debug_stub...
File tests/debug_stub/debug_stub_test.py (right):

https://chromiumcodereview.appspot.com/107313005/diff/320001/tests/debug_stub...
tests/debug_stub/debug_stub_test.py:264: def SingleSteppingWorks(ARCH):
Local variables should be lower case (upper case is for global constants), so
either write "(arch)" here (and change function body) or "()" (and have function
body refer to the global).

[petarj, 2013-12-27 12:59:42]

https://chromiumcodereview.appspot.com/107313005/diff/320001/tests/debug_stub...
File tests/debug_stub/debug_stub_test.py (right):

https://chromiumcodereview.appspot.com/107313005/diff/320001/tests/debug_stub...
tests/debug_stub/debug_stub_test.py:264: def SingleSteppingWorks(ARCH):
On 2013/12/26 23:23:43, Mark Seaborn wrote:
> Local variables should be lower case (upper case is for global constants), so
> either write "(arch)" here (and change function body) or "()" (and have
function
> body refer to the global).

Done, thanks.

[commit-bot: I haz the power, 2013-12-27 13:00:02]

CQ is trying da patch. Follow status at
https://nativeclient-status.appspot.com/cq/petarj@mips.com/107313005/380001

[commit-bot: I haz the power, 2013-12-27 20:37:44]

Change committed as 12570