[5/6] Disassembly unit test: memory error

Message ID 1484560977-8693-6-git-send-email-yao.qi@linaro.org
State New, archived
Headers

Commit Message

Yao Qi Jan. 16, 2017, 10:02 a.m. UTC
  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(+)
  

Comments

Luis Machado Jan. 17, 2017, 2:37 p.m. UTC | #1
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.
  
Pedro Alves Jan. 20, 2017, 12:08 a.m. UTC | #2
On 01/16/2017 10:02 AM, Yao Qi wrote:
> +  bool see_memory_error = false;

seen_memory_error or saw_memory_error.  "see" reads like an order.

Thanks,
Pedro Alves
  
Yao Qi Jan. 24, 2017, 3:32 p.m. UTC | #3
On 17-01-17 08:37:48, Luis Machado wrote:
> >+
> >+    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?

-1 is what target_read_code returned on error, and -1 is expected by
disassembler in opcodes.  We need to define the error number in
include/dis-asm.h.  I'll do it later.

> 
> >+      return -1;
> >+    }
> >+  };
> >+
  

Patch

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)
+{
+  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.  */
+      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.   */
+  SELF_CHECK (see_memory_error);
+
+}
+
 } // 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
 }