Re: [PATCH 5/6] Disassembly unit test: memory error

Luis Machado
2017-01-17T14:38:07Z

On 01/16/2017 04:02 AM, Yao Qi wrote:
> This patch adds a unit test about memory error occurs on reading
> memory, and check MEMORY_ERROR exception is always thrown.
>
> v2:
> - use null_stream,
>
> gdb:
>
> 2017-01-10 Yao Qi <yao.qi@linaro.org>
>
> * disasm-selftests.c (gdb_disassembler_memory_error_test): New function.
> (_initialize_disasm_test): Register
> gdb_disassembler_memory_error_test.
> ---
> gdb/disasm-selftests.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
> index 46a0a21..ad2b4eb 100644
> --- a/gdb/disasm-selftests.c
> +++ b/gdb/disasm-selftests.c
> @@ -163,6 +163,48 @@ gdb_disassembler_print_one_insn_test (struct gdbarch *gdbarch)
> SELF_CHECK (di.print_insn (0) == len);
> }
>
> +/* Test disassembly on memory error. */
> +
> +static void
> +gdb_disassembler_memory_error_test (struct gdbarch *gdbarch)

Same comment as the other patch. Do we need to repeat the context of the
test in the function name?

> +{
> + class gdb_disassembler_test : public gdb_disassembler
> + {
> + public:
> + gdb_disassembler_test (struct gdbarch *gdbarch)
> + : gdb_disassembler (gdbarch, null_stream (),
> + gdb_disassembler_test::read_memory)
> + {
> + }
> +
> + static int read_memory (bfd_vma memaddr, gdb_byte *myaddr,
> + unsigned int len,
> + struct disassemble_info *info)
> + {
> + /* Always get an error. */

/* Always return an error. */

Also, should we return an explicit error like TARGET_XFER_E_IO or
MEMORY_ERROR?

> + return -1;
> + }
> + };
> +
> + gdb_disassembler_test di (gdbarch);
> + bool see_memory_error = false;
> +
> + TRY
> + {
> + di.print_insn (0);
> + }
> + CATCH (ex, RETURN_MASK_ERROR)
> + {
> + if (ex.error == MEMORY_ERROR)
> + see_memory_error = true;
> + }
> + END_CATCH
> +
> + /* Expect MEMORY_ERROR. */

Too many spaces after period.

> + SELF_CHECK (see_memory_error);
> +

Spurious newline?

> +}
> +
> } // namespace selftests
> #endif /* GDB_SELF_TEST */
>
> @@ -174,5 +216,6 @@ _initialize_disasm_test (void)
> {
> #if GDB_SELF_TEST
> register_self_test (selftests::gdb_disassembler_print_one_insn_test);
> + register_self_test (selftests::gdb_disassembler_memory_error_test);
> #endif
> }
>

Otherwise OK.