[PATCH 1/9] Clear GDB internal state after each unit test

[PATCH 1/9] Clear GDB internal state after each unit test

[Yao Qi]

GDB has some global variables, like sentinel_frame,
current_thread_arch, and etc, we need to reset them after each unit
test.

gdb:

2017-04-12  Yao Qi  <yao.qi@linaro.org>

	* selftest-arch.c (tests_with_arch): Call registers_changed
	and reinit_frame_cache.
	* selftest.c (run_self_tests): Likewise.
---
 gdb/selftest-arch.c | 4 ++++
 gdb/selftest.c      | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
index cbc8c50..c4fe60d 100644
--- a/gdb/selftest-arch.c
+++ b/gdb/selftest-arch.c
@@ -81,6 +81,10 @@ tests_with_arch ()
 				 _("Self test failed: arch %s: "), arches[i]);
 	    }
 	  END_CATCH
+
+	  /* Clear GDB internal state.  */
+	  registers_changed ();
+	  reinit_frame_cache ();
 	}
     }
 
diff --git a/gdb/selftest.c b/gdb/selftest.c
index adc7dda..14b76f6 100644
--- a/gdb/selftest.c
+++ b/gdb/selftest.c
@@ -53,6 +53,10 @@ run_self_tests (void)
 	  exception_fprintf (gdb_stderr, ex, _("Self test failed: "));
 	}
       END_CATCH
+
+      /* Clear GDB internal state.  */
+      registers_changed ();
+      reinit_frame_cache ();
     }
 
   printf_filtered (_("Ran %lu unit tests, %d failed\n"),
-- 
1.9.1

[PATCH OBV 3/9] Fix a typo in rx_fpsw_type

[Yao Qi]

gdb:

2017-04-12  Yao Qi  <yao.qi@linaro.org>

	* rx-tdep.c (rx_fpsw_type): Check tdep->rx_fpsw_type instead of
	tdep->rx_psw_type.
---
 gdb/rx-tdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/rx-tdep.c b/gdb/rx-tdep.c
index 1a3d103..83bb3ec 100644
--- a/gdb/rx-tdep.c
+++ b/gdb/rx-tdep.c
@@ -182,7 +182,7 @@ rx_fpsw_type (struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
-  if (tdep->rx_psw_type == NULL)
+  if (tdep->rx_fpsw_type == NULL)
     {
       tdep->rx_fpsw_type = arch_flags_type (gdbarch, "rx_fpsw_type", 4);
       append_flags_type_flag (tdep->rx_fpsw_type, 0, "RM0");
-- 
1.9.1

[PATCH 4/9] Use XCNEW gdbarch_tdep

[Yao Qi]

This patch uses XCNEW gdbarch_tdep instead of XNEW.

gdb:

2017-04-12  Yao Qi  <yao.qi@linaro.org>

	* alpha-tdep.c (alpha_gdbarch_init): Use XCNEW instead of XNEW.
	* avr-tdep.c (avr_gdbarch_init): Likewise.
	* bfin-tdep.c (bfin_gdbarch_init): Likewise.
	* cris-tdep.c (cris_gdbarch_init): Likewise.
	* ft32-tdep.c (ft32_gdbarch_init): Likewise.
	* lm32-tdep.c (lm32_gdbarch_init): Likewise.
	* m32r-tdep.c (m32r_gdbarch_init): Likewise.
	* m68hc11-tdep.c (m68hc11_gdbarch_init): Likewise.
	* mep-tdep.c (mep_gdbarch_init): Likewise.
	* microblaze-tdep.c (microblaze_gdbarch_init): Likewise.
	* mips-tdep.c (mips_gdbarch_init): Likewise.
	* mn10300-tdep.c (mn10300_gdbarch_init): Likewise.
	* moxie-tdep.c (moxie_gdbarch_init): Likewise.
	* msp430-tdep.c (msp430_gdbarch_init): Likewise.
	* sh64-tdep.c (sh64_gdbarch_init): Likewise.
	* v850-tdep.c (v850_gdbarch_init): Likewise.
---
 gdb/alpha-tdep.c      | 2 +-
 gdb/avr-tdep.c        | 2 +-
 gdb/bfin-tdep.c       | 2 +-
 gdb/cris-tdep.c       | 2 +-
 gdb/ft32-tdep.c       | 2 +-
 gdb/lm32-tdep.c       | 2 +-
 gdb/m32r-tdep.c       | 2 +-
 gdb/m68hc11-tdep.c    | 2 +-
 gdb/mep-tdep.c        | 2 +-
 gdb/microblaze-tdep.c | 2 +-
 gdb/mips-tdep.c       | 2 +-
 gdb/mn10300-tdep.c    | 2 +-
 gdb/moxie-tdep.c      | 2 +-
 gdb/msp430-tdep.c     | 2 +-
 gdb/sh64-tdep.c       | 2 +-
 gdb/v850-tdep.c       | 2 +-
 16 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index 4dd65c5..89f9676 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -1749,7 +1749,7 @@ alpha_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   if (arches != NULL)
     return arches->gdbarch;
 
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
   /* Lowest text address.  This is used by heuristic_proc_start()
diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index 0ae8a08..1c42f1b 100644
--- a/gdb/avr-tdep.c
+++ b/gdb/avr-tdep.c
@@ -1453,7 +1453,7 @@ avr_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     }
 
   /* None found, create a new architecture from the information provided.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
   
   tdep->call_length = call_length;
diff --git a/gdb/bfin-tdep.c b/gdb/bfin-tdep.c
index 80d83d4..7b922e4 100644
--- a/gdb/bfin-tdep.c
+++ b/gdb/bfin-tdep.c
@@ -811,7 +811,7 @@ bfin_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
       return arches->gdbarch;
     }
 
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
   tdep->bfin_abi = abi;
diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
index a46356f..e6fab60 100644
--- a/gdb/cris-tdep.c
+++ b/gdb/cris-tdep.c
@@ -4021,7 +4021,7 @@ cris_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     }
 
   /* No matching architecture was found.  Create a new one.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   info.byte_order = BFD_ENDIAN_LITTLE;
   gdbarch = gdbarch_alloc (&info, tdep);
 
diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c
index 5b28275..4cb8c25 100644
--- a/gdb/ft32-tdep.c
+++ b/gdb/ft32-tdep.c
@@ -597,7 +597,7 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     return arches->gdbarch;
 
   /* Allocate space for the new architecture.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
   /* Create a type for PC.  We can't use builtin types here, as they may not
diff --git a/gdb/lm32-tdep.c b/gdb/lm32-tdep.c
index bd3e182..54bff6e 100644
--- a/gdb/lm32-tdep.c
+++ b/gdb/lm32-tdep.c
@@ -521,7 +521,7 @@ lm32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     return arches->gdbarch;
 
   /* None found, create a new architecture from the information provided.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
   /* Type sizes.  */
diff --git a/gdb/m32r-tdep.c b/gdb/m32r-tdep.c
index 4701f7a..84ae4be 100644
--- a/gdb/m32r-tdep.c
+++ b/gdb/m32r-tdep.c
@@ -908,7 +908,7 @@ m32r_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     return arches->gdbarch;
 
   /* Allocate space for the new architecture.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
   set_gdbarch_read_pc (gdbarch, m32r_read_pc);
diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
index 893d9c2..307c9e9 100644
--- a/gdb/m68hc11-tdep.c
+++ b/gdb/m68hc11-tdep.c
@@ -1444,7 +1444,7 @@ m68hc11_gdbarch_init (struct gdbarch_info info,
     }
 
   /* Need a new architecture.  Fill in a target specific vector.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
   tdep->elf_flags = elf_flags;
 
diff --git a/gdb/mep-tdep.c b/gdb/mep-tdep.c
index 5a45d6e..7e9b855 100644
--- a/gdb/mep-tdep.c
+++ b/gdb/mep-tdep.c
@@ -2437,7 +2437,7 @@ mep_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     if (gdbarch_tdep (arches->gdbarch)->me_module == me_module)
       return arches->gdbarch;
 
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
   /* Get a CGEN CPU descriptor for this architecture.  */
diff --git a/gdb/microblaze-tdep.c b/gdb/microblaze-tdep.c
index 59f94c5..e5c2ceb 100644
--- a/gdb/microblaze-tdep.c
+++ b/gdb/microblaze-tdep.c
@@ -703,7 +703,7 @@ microblaze_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     }
 
   /* Allocate space for the new architecture.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
   set_gdbarch_long_double_bit (gdbarch, 128);
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 45d1d73..592d1aa 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -8488,7 +8488,7 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     }
 
   /* Need a new architecture.  Fill in a target specific vector.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
   tdep->elf_flags = elf_flags;
   tdep->mips64_transfers_32bit_regs_p = mips64_transfers_32bit_regs_p;
diff --git a/gdb/mn10300-tdep.c b/gdb/mn10300-tdep.c
index 0f5e10f..ce66b2b 100644
--- a/gdb/mn10300-tdep.c
+++ b/gdb/mn10300-tdep.c
@@ -1395,7 +1395,7 @@ mn10300_gdbarch_init (struct gdbarch_info info,
   if (arches != NULL)
     return arches->gdbarch;
 
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
   switch (info.bfd_arch_info->mach)
diff --git a/gdb/moxie-tdep.c b/gdb/moxie-tdep.c
index d0f4223..6cc4d38 100644
--- a/gdb/moxie-tdep.c
+++ b/gdb/moxie-tdep.c
@@ -1109,7 +1109,7 @@ moxie_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     return arches->gdbarch;
 
   /* Allocate space for the new architecture.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
   set_gdbarch_read_pc (gdbarch, moxie_read_pc);
diff --git a/gdb/msp430-tdep.c b/gdb/msp430-tdep.c
index d9eebf0..37a47d0 100644
--- a/gdb/msp430-tdep.c
+++ b/gdb/msp430-tdep.c
@@ -944,7 +944,7 @@ msp430_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* None found, create a new architecture from the information
      provided.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
   tdep->elf_flags = elf_flags;
   tdep->isa = isa;
diff --git a/gdb/sh64-tdep.c b/gdb/sh64-tdep.c
index 33986fd..3d29506 100644
--- a/gdb/sh64-tdep.c
+++ b/gdb/sh64-tdep.c
@@ -2371,7 +2371,7 @@ sh64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* None found, create a new architecture from the information
      provided.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
   /* Determine the ABI */
diff --git a/gdb/v850-tdep.c b/gdb/v850-tdep.c
index 668635e..18af250 100644
--- a/gdb/v850-tdep.c
+++ b/gdb/v850-tdep.c
@@ -1396,7 +1396,7 @@ v850_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
       return arches->gdbarch;
     }
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   tdep->e_flags = e_flags;
   tdep->e_machine = e_machine;
 
-- 
1.9.1

[PATCH 9/9] Add unit test to gdbarch methods register_to_value and value_to_register

[Yao Qi]

This patch adds one unit test for gdbarch methods register_to_value and
value_to_register.  The test pass different combinations of {regnu, type}
to gdbarch_register_to_value and gdbarch_value_to_register.  In order
to do the test, add a new function create_new_frame to create a fake
frame.  It can be improved after we converted frame_info to class.

This test is useful with ASAN.  Suppose I incorrectly modified
the size of buffer as below,

@@ -1228,7 +1228,7 @@ ia64_register_to_value (struct frame_info *frame, int regnum,
                        int *optimizedp, int *unavailablep)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  gdb_byte in[MAX_REGISTER_SIZE];
+  gdb_byte in[1];

   /* Convert to TYPE.  */
   if (!get_frame_register_bytes (frame, regnum, 0,

build GDB with "-fsanitize=address" and run unittest.exp, asan can detect
such error,

=================================================================
==6003==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff60a36520 at pc 0xbaa15d bp 0x7fff60a36250 sp 0x7fff60a36248
WRITE of size 16 at 0x7fff60a36520 thread T0
    #0 0xbaa15c in frame_register_unwind(frame_info*, int, int*, int*, lval_type*, unsigned long*, int*, unsigned char*) gdb/frame.c:1119
    #1 0xbaa43b in frame_register(frame_info*, int, int*, int*, lval_type*, unsigned long*, int*, unsigned char*) gdb/frame.c:1147
    #2 0xbab998 in get_frame_register_bytes(frame_info*, int, unsigned long, int, unsigned char*, int*, int*) gdb/frame.c:1424
    #3 0x6ff39e in ia64_register_to_value gdb/ia64-tdep.c:1236
    #4 0xbc9276 in gdbarch_register_to_value(gdbarch*, frame_info*, int, type*, unsigned char*, int*, int*) gdb/gdbarch.c:2565
    #5 0xbd8e6b in register_to_value_test gdb/git/gdb/gdbarch-selftests.c:83
    #6 0xd4e628 in tests_with_arch gdb/selftest-arch.c:75
    #7 0xd4d02c in run_self_tests() gdb/selftest.c:48
    #8 0xc8cc3d in maintenance_selftest gdb/maint.c:949

gdb:

2017-04-11  Yao Qi  <yao.qi@linaro.org>

	* gdbarch-selftests.c: New file.
	* frame.c [GDB_SELF_TESTS] (create_new_frame): New function.
	* frame.h [GDB_SELF_TESTS] (create_new_frame): Declare.
---
 gdb/Makefile.in         |   2 +
 gdb/findvar.c           |   1 -
 gdb/frame.c             |  18 +++++++
 gdb/frame.h             |   4 ++
 gdb/gdbarch-selftests.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 gdb/gdbarch-selftests.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 23e4bed..a926d9a 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1087,6 +1087,7 @@ SFILES = \
 	gdb_obstack.c \
 	gdb_usleep.c \
 	gdbarch.c \
+	gdbarch-selftests.c \
 	gdbtypes.c \
 	gnu-v2-abi.c \
 	gnu-v3-abi.c \
@@ -1697,6 +1698,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	gdb_usleep.o \
 	gdb_vecs.o \
 	gdbarch.o \
+	gdbarch-selftests.o \
 	gdbtypes.o \
 	gnu-v2-abi.o \
 	gnu-v3-abi.o \
diff --git a/gdb/findvar.c b/gdb/findvar.c
index ed4d5c1..84f4f47 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -1004,4 +1004,3 @@ address_from_register (int regnum, struct frame_info *frame)
 
   return result;
 }
-
diff --git a/gdb/frame.c b/gdb/frame.c
index d4fc456..0dc8726 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1704,6 +1704,24 @@ select_frame (struct frame_info *fi)
     }
 }
 
+#if GDB_SELF_TEST
+struct frame_info *
+create_new_frame (struct gdbarch *gdbarch)
+{
+  struct frame_info *this_frame = XCNEW (struct frame_info);
+  struct regcache *regcache = regcache_xmalloc (gdbarch, NULL);
+
+  sentinel_frame = create_sentinel_frame (NULL, regcache);
+  sentinel_frame->prev = this_frame;
+  sentinel_frame->prev_p = 1;;
+  this_frame->prev_arch.p = 1;
+  this_frame->prev_arch.arch = gdbarch;
+  this_frame->next = sentinel_frame;
+
+  return this_frame;
+}
+#endif
+
 /* Create an arbitrary (i.e. address specified by user) or innermost frame.
    Always returns a non-NULL value.  */
 
diff --git a/gdb/frame.h b/gdb/frame.h
index 1d0644f..78727b2 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -833,6 +833,10 @@ extern struct frame_info *deprecated_safe_get_selected_frame (void);
 
 extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR pc);
 
+#if GDB_SELF_TEST
+extern struct frame_info *create_new_frame (struct gdbarch *gdbarch);
+#endif
+
 /* Return true if the frame unwinder for frame FI is UNWINDER; false
    otherwise.  */
 
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
new file mode 100644
index 0000000..df084ce
--- /dev/null
+++ b/gdb/gdbarch-selftests.c
@@ -0,0 +1,123 @@
+/* Self tests for gdbarch for GDB, the GNU debugger.
+
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#if GDB_SELF_TEST
+#include "selftest.h"
+#include "selftest-arch.h"
+#include "inferior.h"
+
+namespace selftests {
+
+/* Test gdbarch methods register_to_value and value_to_register.  */
+
+static void
+register_to_value_test (struct gdbarch *gdbarch)
+{
+  const struct builtin_type *builtin = builtin_type (gdbarch);
+  struct type *types[] =
+    {
+      builtin->builtin_void, builtin->builtin_char,
+      builtin->builtin_short, builtin->builtin_int,
+      builtin->builtin_long, builtin->builtin_signed_char,
+      builtin->builtin_unsigned_short,
+      builtin->builtin_unsigned_int,
+      builtin->builtin_unsigned_long,
+      builtin->builtin_float,
+      builtin->builtin_double,
+      builtin->builtin_long_double,
+      builtin->builtin_complex,
+      builtin->builtin_double_complex,
+      builtin->builtin_string,
+      builtin->builtin_bool,
+      builtin->builtin_long_long,
+      builtin->builtin_unsigned_long_long,
+      builtin->builtin_int8,
+      builtin->builtin_uint8,
+      builtin->builtin_int16,
+      builtin->builtin_uint16,
+      builtin->builtin_int32,
+      builtin->builtin_uint32,
+      builtin->builtin_int64,
+      builtin->builtin_uint64,
+      builtin->builtin_int128,
+      builtin->builtin_uint128,
+      builtin->builtin_char16,
+      builtin->builtin_char32,
+    };
+
+  current_inferior()->gdbarch = gdbarch;
+
+  struct frame_info *frame = create_new_frame (gdbarch);
+  const int num_regs = (gdbarch_num_regs (gdbarch)
+			+ gdbarch_num_pseudo_regs (gdbarch));
+
+  /* Test gdbarch methods register_to_value and value_to_register with
+     different combinations of register numbers and types.  */
+  for (const auto &type : types)
+    {
+      for (auto regnum = 0; regnum < num_regs; regnum++)
+	{
+	  if (gdbarch_convert_register_p (gdbarch, regnum, type))
+	    {
+	      std::vector<gdb_byte> buf (TYPE_LENGTH (type));
+	      int optim, unavail, ok;
+
+	      ok = gdbarch_register_to_value (gdbarch, frame, regnum, type,
+					      buf.data (), &optim, &unavail);
+
+	      SELF_CHECK (ok);
+	      SELF_CHECK (!optim);
+	      SELF_CHECK (!unavail);
+
+	      bool saw_no_process_error = false;
+	      TRY
+		{
+		  gdbarch_value_to_register (gdbarch, frame, regnum, type,
+					     buf.data ());
+		}
+	      CATCH (ex, RETURN_MASK_ERROR)
+		{
+		  if (strcmp (ex.message,
+			      "You can't do that without a process to debug.")
+		      == 0)
+		    saw_no_process_error = true;
+		}
+
+	      /* GDB wants to write registers to target, so it is expected
+		 to see no process exception.  */
+	      SELF_CHECK (saw_no_process_error);
+	    }
+	}
+    }
+}
+
+} // namespace selftests
+#endif /* GDB_SELF_TEST */
+
+/* Suppress warning from -Wmissing-prototypes.  */
+extern initialize_file_ftype _initialize_gdbarch_selftests;
+
+void
+_initialize_gdbarch_selftests (void)
+{
+#if GDB_SELF_TEST
+  register_self_test_foreach_arch (selftests::register_to_value_test);
+#endif
+}
-- 
1.9.1

[PATCH 5/9] Restrict m68k_convert_register_p

[Yao Qi]

We need to convert register if the type is float.  Suppose we get a value
from float point register, but its type is integer, we don't have to convert.
This case may not exist in real code, but exist in my unit test case.

warning: Cannot convert floating-point register value to non-floating-point type.
Self test failed: arch m68k: self-test failed at gdb/git/gdb/findvar.c:1072

              ok = gdbarch_register_to_value (gdbarch, frame, regnum, type,
                                              buf.data (), &optim, &unavail);

1072:         SELF_CHECK (ok);

gdb:

2017-04-12  Yao Qi  <yao.qi@linaro.org>

	* m68k-tdep.c (m68k_convert_register_p): Check type's code is
	TYPE_CODE_FLT or not.
---
 gdb/m68k-tdep.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/gdb/m68k-tdep.c b/gdb/m68k-tdep.c
index 7c3bf4c..1486841 100644
--- a/gdb/m68k-tdep.c
+++ b/gdb/m68k-tdep.c
@@ -188,6 +188,8 @@ m68k_convert_register_p (struct gdbarch *gdbarch,
   if (!gdbarch_tdep (gdbarch)->fpregs_present)
     return 0;
   return (regnum >= M68K_FP0_REGNUM && regnum <= M68K_FP0_REGNUM + 7
+	  /* We only support floating-point values.  */
+	  && TYPE_CODE (type) == TYPE_CODE_FLT
 	  && type != register_type (gdbarch, M68K_FP0_REGNUM));
 }
 
@@ -203,17 +205,6 @@ m68k_register_to_value (struct frame_info *frame, int regnum,
   struct type *fpreg_type = register_type (get_frame_arch (frame),
 					   M68K_FP0_REGNUM);
 
-  /* We only support floating-point values.  */
-  if (TYPE_CODE (type) != TYPE_CODE_FLT)
-    {
-      warning (_("Cannot convert floating-point register value "
-	       "to non-floating-point type."));
-      *optimizedp = *unavailablep = 0;
-      return 0;
-    }
-
-  /* Convert to TYPE.  */
-
   /* Convert to TYPE.  */
   if (!get_frame_register_bytes (frame, regnum, 0, TYPE_LENGTH (type),
 				 from, optimizedp, unavailablep))
-- 
1.9.1

[PATCH OBV 2/9] XCNEW gdbarch_tdep in rl78 and rx

[Yao Qi]

"struct gdbarch_tdep" is XNEW'ed in rl78 and rx, so the memory is not
cleared.  As the result, tdep->rl78_psw_type may be never initialized
properly.

  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

  if (tdep->rl78_psw_type == NULL)
    {
      tdep->rl78_psw_type = arch_flags_type (gdbarch,
					     "builtin_type_rl78_psw", 1);

The bug is found by my unit test in the following patch.

gdb:

2017-04-12  Yao Qi  <yao.qi@linaro.org>

	* rl78-tdep.c (rl78_gdbarch_init): Use XCNEW instead of XNEW.
	* rx-tdep.c (rx_gdbarch_init): Likewise.
---
 gdb/rl78-tdep.c | 2 +-
 gdb/rx-tdep.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/rl78-tdep.c b/gdb/rl78-tdep.c
index 9f2193e..307a760 100644
--- a/gdb/rl78-tdep.c
+++ b/gdb/rl78-tdep.c
@@ -1412,7 +1412,7 @@ rl78_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* None found, create a new architecture from the information
      provided.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
   tdep->elf_flags = elf_flags;
 
diff --git a/gdb/rx-tdep.c b/gdb/rx-tdep.c
index 7b66d6d..1a3d103 100644
--- a/gdb/rx-tdep.c
+++ b/gdb/rx-tdep.c
@@ -1101,7 +1101,7 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* None found, create a new architecture from the information
      provided.  */
-  tdep = XNEW (struct gdbarch_tdep);
+  tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
   tdep->elf_flags = elf_flags;
 
-- 
1.9.1

[PATCH 0/9] Unit test to gdbarch methods register_to_value and value_to_register

[Yao Qi]

Alan recently has some patches removing/replacing MAX_REGISTER_SIZE, and
they touch some target dependent code that we can't test, which makes
me a little nervous.  So patch #9 adds
a unit test to gdbarch methods register_to_value and value_to_register.
Patches 1-8 are to fix problems found by this unit test.  So far, the
test to register_to_value doesn't check value contents got from register,
but the functions we are going to change in Alan's patches are covered
by this test at least.  I'll further improve this test to check value
contents by sub-class regcache which returns some unique contents.

Patch #2 and #3 are quite obvious.  Patch #5 - #8 are to restrict the
$ARCH_convert_register_p for some cases only exists in the unit test.

I recommend to review/read patch #9 first, and it helps to understand
what do the rest do.

I build GDB with all targets enabled, run unittest.exp on x86_64-linux,
and collect coverage data,
http://people.linaro.org/~yao.qi/gdb/coverage/unittest/gdb/ functions
$ARCH_register_to_value and $ARCH_value_to_register are covered.  Click
files ia64-tdep.c, alpha-tdep.c, and mips-tdep.c, search
"convert_register".

*** BLURB HERE ***

Yao Qi (9):
  Clear GDB internal state after each unit test
  XCNEW gdbarch_tdep in rl78 and rx
  Fix a typo in rx_fpsw_type
  Use XCNEW gdbarch_tdep
  Restrict m68k_convert_register_p
  Restrict ia64_convert_register_p
  Restrict alpha_convert_register_p
  Restrict i387_convert_register_p
  Add unit test to gdbarch methods register_to_value and
    value_to_register

 gdb/Makefile.in         |   2 +
 gdb/alpha-tdep.c        |  27 ++++-------
 gdb/avr-tdep.c          |   2 +-
 gdb/bfin-tdep.c         |   2 +-
 gdb/cris-tdep.c         |   2 +-
 gdb/findvar.c           |   1 -
 gdb/frame.c             |  16 +++++++
 gdb/frame.h             |   4 ++
 gdb/ft32-tdep.c         |   2 +-
 gdb/gdbarch-selftests.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/i387-tdep.c         |   5 +-
 gdb/ia64-tdep.c         |   1 +
 gdb/lm32-tdep.c         |   2 +-
 gdb/m32r-tdep.c         |   2 +-
 gdb/m68hc11-tdep.c      |   2 +-
 gdb/m68k-tdep.c         |  13 +----
 gdb/mep-tdep.c          |   2 +-
 gdb/microblaze-tdep.c   |   2 +-
 gdb/mips-tdep.c         |   2 +-
 gdb/mn10300-tdep.c      |   2 +-
 gdb/moxie-tdep.c        |   2 +-
 gdb/msp430-tdep.c       |   2 +-
 gdb/rl78-tdep.c         |   2 +-
 gdb/rx-tdep.c           |   4 +-
 gdb/selftest-arch.c     |   4 ++
 gdb/selftest.c          |   4 ++
 gdb/sh64-tdep.c         |   2 +-
 gdb/v850-tdep.c         |   2 +-
 28 files changed, 185 insertions(+), 51 deletions(-)
 create mode 100644 gdb/gdbarch-selftests.c

-- 
1.9.1

[PATCH 8/9] Restrict i387_convert_register_p

[Yao Qi]

gdb:

2017-04-12  Yao Qi  <yao.qi@linaro.org>

	* i387-tdep.c (i387_convert_register_p): Return false if type code
	is TYPE_CODE_FLT.

---
 gdb/i387-tdep.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index 9206109..925f4e5 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -341,8 +341,9 @@ i387_convert_register_p (struct gdbarch *gdbarch, int regnum,
   if (i386_fp_regnum_p (gdbarch, regnum))
     {
       /* Floating point registers must be converted unless we are
-	 accessing them in their hardware type.  */
-      if (type == i387_ext_type (gdbarch))
+	 accessing them in their hardware type or TYPE is not float.  */
+      if (type == i387_ext_type (gdbarch)
+	  || TYPE_CODE (type) != TYPE_CODE_FLT)
 	return 0;
       else
 	return 1;
-- 
1.9.1

[PATCH 7/9] Restrict alpha_convert_register_p

[Yao Qi]

This patch restricts alpha_convert_register_p from
"TYPE_LENGTH (type) != 8" to "TYPE_LENGTH (type) == 4", because,

 - we have check "TYPE_LENGTH (valtype) == 4" in alpha_register_to_value
   and alpha_value_to_register,
 - alpha lds and sts instruction access 4 bytes,
 - comments "It might need to convert the [float] register into the
   corresponding [integer] type (see Alpha)" and integer is 4-byte on
   alpha,

I think it is the right restrict condition to "TYPE_LENGTH (valtype) == 4".

gdb:

2017-04-12  Yao Qi  <yao.qi@linaro.org>

	* alpha-tdep.c (alpha_convert_register_p): Return true if type
	length is 4.
	(alpha_register_to_value): Remove type length check.
	(alpha_value_to_register): Likewise.
---
 gdb/alpha-tdep.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index 89f9676..d736cab 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -227,7 +227,7 @@ alpha_sts (struct gdbarch *gdbarch, void *out, const void *in)
 /* The alpha needs a conversion between register and memory format if the
    register is a floating point register and memory format is float, as the
    register format must be double or memory format is an integer with 4
-   bytes or less, as the representation of integers in floating point
+   bytes, as the representation of integers in floating point
    registers is different.  */
 
 static int
@@ -235,7 +235,7 @@ alpha_convert_register_p (struct gdbarch *gdbarch, int regno,
 			  struct type *type)
 {
   return (regno >= ALPHA_FP0_REGNUM && regno < ALPHA_FP0_REGNUM + 31
-	  && TYPE_LENGTH (type) != 8);
+	  && TYPE_LENGTH (type) == 4);
 }
 
 static int
@@ -252,14 +252,9 @@ alpha_register_to_value (struct frame_info *frame, int regnum,
 				 in, optimizedp, unavailablep))
     return 0;
 
-  if (TYPE_LENGTH (valtype) == 4)
-    {
-      alpha_sts (gdbarch, out, in);
-      *optimizedp = *unavailablep = 0;
-      return 1;
-    }
-
-  error (_("Cannot retrieve value from floating point register"));
+  alpha_sts (gdbarch, out, in);
+  *optimizedp = *unavailablep = 0;
+  return 1;
 }
 
 static void
@@ -268,14 +263,8 @@ alpha_value_to_register (struct frame_info *frame, int regnum,
 {
   gdb_byte out[MAX_REGISTER_SIZE];
 
-  switch (TYPE_LENGTH (valtype))
-    {
-    case 4:
-      alpha_lds (get_frame_arch (frame), out, in);
-      break;
-    default:
-      error (_("Cannot store value in floating point register"));
-    }
+  alpha_lds (get_frame_arch (frame), out, in);
+
   put_frame_register (frame, regnum, out);
 }
 
-- 
1.9.1

[PATCH 6/9] Restrict ia64_convert_register_p

[Yao Qi]

gdb:

2017-04-12  Yao Qi  <yao.qi@linaro.org>

	* ia64-tdep.c (ia64_convert_register_p): Check type's code is
	TYPE_CODE_FLT.
---
 gdb/ia64-tdep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 22e1588..8c48eef 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -1218,6 +1218,7 @@ static int
 ia64_convert_register_p (struct gdbarch *gdbarch, int regno, struct type *type)
 {
   return (regno >= IA64_FR0_REGNUM && regno <= IA64_FR127_REGNUM
+	  && TYPE_CODE (type) == TYPE_CODE_FLT
 	  && type != ia64_ext_type (gdbarch));
 }
 
-- 
1.9.1

Re: [PATCH 9/9] Add unit test to gdbarch methods register_to_value and value_to_register

[Simon Marchi]

On 2017-04-13 03:26, Yao Qi wrote:
> This patch adds one unit test for gdbarch methods register_to_value and
> value_to_register.  The test pass different combinations of {regnu, 
> type}
> to gdbarch_register_to_value and gdbarch_value_to_register.  In order
> to do the test, add a new function create_new_frame to create a fake
> frame.  It can be improved after we converted frame_info to class.
> 
> This test is useful with ASAN.  Suppose I incorrectly modified
> the size of buffer as below,
> 
> @@ -1228,7 +1228,7 @@ ia64_register_to_value (struct frame_info
> *frame, int regnum,
>                         int *optimizedp, int *unavailablep)
>  {
>    struct gdbarch *gdbarch = get_frame_arch (frame);
> -  gdb_byte in[MAX_REGISTER_SIZE];
> +  gdb_byte in[1];
> 
>    /* Convert to TYPE.  */
>    if (!get_frame_register_bytes (frame, regnum, 0,
> 
> build GDB with "-fsanitize=address" and run unittest.exp, asan can 
> detect
> such error,
> 
> =================================================================
> ==6003==ERROR: AddressSanitizer: stack-buffer-overflow on address
> 0x7fff60a36520 at pc 0xbaa15d bp 0x7fff60a36250 sp 0x7fff60a36248
> WRITE of size 16 at 0x7fff60a36520 thread T0
>     #0 0xbaa15c in frame_register_unwind(frame_info*, int, int*, int*,
> lval_type*, unsigned long*, int*, unsigned char*) gdb/frame.c:1119
>     #1 0xbaa43b in frame_register(frame_info*, int, int*, int*,
> lval_type*, unsigned long*, int*, unsigned char*) gdb/frame.c:1147
>     #2 0xbab998 in get_frame_register_bytes(frame_info*, int, unsigned
> long, int, unsigned char*, int*, int*) gdb/frame.c:1424
>     #3 0x6ff39e in ia64_register_to_value gdb/ia64-tdep.c:1236
>     #4 0xbc9276 in gdbarch_register_to_value(gdbarch*, frame_info*,
> int, type*, unsigned char*, int*, int*) gdb/gdbarch.c:2565
>     #5 0xbd8e6b in register_to_value_test 
> gdb/git/gdb/gdbarch-selftests.c:83
>     #6 0xd4e628 in tests_with_arch gdb/selftest-arch.c:75
>     #7 0xd4d02c in run_self_tests() gdb/selftest.c:48
>     #8 0xc8cc3d in maintenance_selftest gdb/maint.c:949
> 
> gdb:
> 
> 2017-04-11  Yao Qi  <yao.qi@linaro.org>
> 
> 	* gdbarch-selftests.c: New file.

Some unit tests are in the unittests directory.  Do we have a rule for 
what goes there versus in gdb/ directly?

> 	* frame.c [GDB_SELF_TESTS] (create_new_frame): New function.
> 	* frame.h [GDB_SELF_TESTS] (create_new_frame): Declare.
> ---
>  gdb/Makefile.in         |   2 +
>  gdb/findvar.c           |   1 -
>  gdb/frame.c             |  18 +++++++
>  gdb/frame.h             |   4 ++
>  gdb/gdbarch-selftests.c | 123 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 147 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/gdbarch-selftests.c
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 23e4bed..a926d9a 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1087,6 +1087,7 @@ SFILES = \
>  	gdb_obstack.c \
>  	gdb_usleep.c \
>  	gdbarch.c \
> +	gdbarch-selftests.c \
>  	gdbtypes.c \
>  	gnu-v2-abi.c \
>  	gnu-v3-abi.c \
> @@ -1697,6 +1698,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
>  	gdb_usleep.o \
>  	gdb_vecs.o \
>  	gdbarch.o \
> +	gdbarch-selftests.o \
>  	gdbtypes.o \
>  	gnu-v2-abi.o \
>  	gnu-v3-abi.o \
> diff --git a/gdb/findvar.c b/gdb/findvar.c
> index ed4d5c1..84f4f47 100644
> --- a/gdb/findvar.c
> +++ b/gdb/findvar.c
> @@ -1004,4 +1004,3 @@ address_from_register (int regnum, struct
> frame_info *frame)
> 
>    return result;
>  }
> -

Spurious change.

> diff --git a/gdb/frame.c b/gdb/frame.c
> index d4fc456..0dc8726 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -1704,6 +1704,24 @@ select_frame (struct frame_info *fi)
>      }
>  }
> 
> +#if GDB_SELF_TEST
> +struct frame_info *
> +create_new_frame (struct gdbarch *gdbarch)
> +{
> +  struct frame_info *this_frame = XCNEW (struct frame_info);
> +  struct regcache *regcache = regcache_xmalloc (gdbarch, NULL);
> +
> +  sentinel_frame = create_sentinel_frame (NULL, regcache);
> +  sentinel_frame->prev = this_frame;
> +  sentinel_frame->prev_p = 1;;
> +  this_frame->prev_arch.p = 1;
> +  this_frame->prev_arch.arch = gdbarch;
> +  this_frame->next = sentinel_frame;
> +
> +  return this_frame;
> +}
> +#endif
> +
>  /* Create an arbitrary (i.e. address specified by user) or innermost 
> frame.
>     Always returns a non-NULL value.  */
> 
> diff --git a/gdb/frame.h b/gdb/frame.h
> index 1d0644f..78727b2 100644
> --- a/gdb/frame.h
> +++ b/gdb/frame.h
> @@ -833,6 +833,10 @@ extern struct frame_info
> *deprecated_safe_get_selected_frame (void);
> 
>  extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR 
> pc);
> 
> +#if GDB_SELF_TEST
> +extern struct frame_info *create_new_frame (struct gdbarch *gdbarch);

The name create_new_frame suggests that it could be used to create a 
frame used in the standard GDB context.  Maybe create_test_frame would 
be more appropriate?

Even if this function is only used in tests, a comment explaining what 
it does would be nice, especially since it does more than creating a new 
frame (it sets up some global state).

> +#endif
> +
>  /* Return true if the frame unwinder for frame FI is UNWINDER; false
>     otherwise.  */
> 
> diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
> new file mode 100644
> index 0000000..df084ce
> --- /dev/null
> +++ b/gdb/gdbarch-selftests.c
> @@ -0,0 +1,123 @@
> +/* Self tests for gdbarch for GDB, the GNU debugger.
> +
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or 
> modify
> +   it under the terms of the GNU General Public License as published 
> by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see 
> <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#if GDB_SELF_TEST
> +#include "selftest.h"
> +#include "selftest-arch.h"
> +#include "inferior.h"
> +
> +namespace selftests {
> +
> +/* Test gdbarch methods register_to_value and value_to_register.  */
> +
> +static void
> +register_to_value_test (struct gdbarch *gdbarch)
> +{
> +  const struct builtin_type *builtin = builtin_type (gdbarch);
> +  struct type *types[] =
> +    {
> +      builtin->builtin_void, builtin->builtin_char,
> +      builtin->builtin_short, builtin->builtin_int,
> +      builtin->builtin_long, builtin->builtin_signed_char,

Can you put all of them on their own line?

> +      builtin->builtin_unsigned_short,
> +      builtin->builtin_unsigned_int,
> +      builtin->builtin_unsigned_long,
> +      builtin->builtin_float,
> +      builtin->builtin_double,
> +      builtin->builtin_long_double,
> +      builtin->builtin_complex,
> +      builtin->builtin_double_complex,
> +      builtin->builtin_string,
> +      builtin->builtin_bool,
> +      builtin->builtin_long_long,
> +      builtin->builtin_unsigned_long_long,
> +      builtin->builtin_int8,
> +      builtin->builtin_uint8,
> +      builtin->builtin_int16,
> +      builtin->builtin_uint16,
> +      builtin->builtin_int32,
> +      builtin->builtin_uint32,
> +      builtin->builtin_int64,
> +      builtin->builtin_uint64,
> +      builtin->builtin_int128,
> +      builtin->builtin_uint128,
> +      builtin->builtin_char16,
> +      builtin->builtin_char32,
> +    };
> +
> +  current_inferior()->gdbarch = gdbarch;
> +
> +  struct frame_info *frame = create_new_frame (gdbarch);

If we need to reuse this in other tests, it

> +  const int num_regs = (gdbarch_num_regs (gdbarch)
> +			+ gdbarch_num_pseudo_regs (gdbarch));
> +
> +  /* Test gdbarch methods register_to_value and value_to_register with
> +     different combinations of register numbers and types.  */
> +  for (const auto &type : types)
> +    {
> +      for (auto regnum = 0; regnum < num_regs; regnum++)
> +	{
> +	  if (gdbarch_convert_register_p (gdbarch, regnum, type))
> +	    {
> +	      std::vector<gdb_byte> buf (TYPE_LENGTH (type));
> +	      int optim, unavail, ok;
> +
> +	      ok = gdbarch_register_to_value (gdbarch, frame, regnum, type,
> +					      buf.data (), &optim, &unavail);
> +
> +	      SELF_CHECK (ok);
> +	      SELF_CHECK (!optim);
> +	      SELF_CHECK (!unavail);
> +
> +	      bool saw_no_process_error = false;
> +	      TRY
> +		{
> +		  gdbarch_value_to_register (gdbarch, frame, regnum, type,
> +					     buf.data ());
> +		}
> +	      CATCH (ex, RETURN_MASK_ERROR)
> +		{
> +		  if (strcmp (ex.message,
> +			      "You can't do that without a process to debug.")
> +		      == 0)
> +		    saw_no_process_error = true;
> +		}
> +
> +	      /* GDB wants to write registers to target, so it is expected
> +		 to see no process exception.  */
> +	      SELF_CHECK (saw_no_process_error);

When you subclass regcache to have a "mock" regcache, do you think we'll 
be able to test the success case of this?

> +	    }
> +	}
> +    }
> +}
> +
> +} // namespace selftests
> +#endif /* GDB_SELF_TEST */
> +
> +/* Suppress warning from -Wmissing-prototypes.  */
> +extern initialize_file_ftype _initialize_gdbarch_selftests;

I don't think we still need this anymore.

> +
> +void
> +_initialize_gdbarch_selftests (void)
> +{
> +#if GDB_SELF_TEST
> +  register_self_test_foreach_arch (selftests::register_to_value_test);
> +#endif
> +}

Thanks,

Simon

Re: [PATCH 5/9] Restrict m68k_convert_register_p

[Simon Marchi]

On 2017-04-13 03:26, Yao Qi wrote:
> @@ -203,17 +205,6 @@ m68k_register_to_value (struct frame_info *frame,
> int regnum,
>    struct type *fpreg_type = register_type (get_frame_arch (frame),
>  					   M68K_FP0_REGNUM);
> 
> -  /* We only support floating-point values.  */
> -  if (TYPE_CODE (type) != TYPE_CODE_FLT)
> -    {
> -      warning (_("Cannot convert floating-point register value "
> -	       "to non-floating-point type."));
> -      *optimizedp = *unavailablep = 0;
> -      return 0;
> -    }

Perhaps that can be replaced with a gdb_assert (TYPE_CODE (type) == 
TYPE_CODE_FLT) ?  Or maybe it would be more appropriate to put such 
asserts on convert_typed_floating ?

Likewise in the following patches, the if (TYPE_LENGTH (valtype) == 4) 
check in alpha could be replaced with gdb_assert (TYPE_LENGTH (valtype) 
== 4).

Simon

Re: [PATCH 5/9] Restrict m68k_convert_register_p

[Yao Qi]

Simon Marchi <simon.marchi@polymtl.ca> writes:

> Perhaps that can be replaced with a gdb_assert (TYPE_CODE (type) ==
> TYPE_CODE_FLT) ?  Or maybe it would be more appropriate to put such
> asserts on convert_typed_floating ?
>
> Likewise in the following patches, the if (TYPE_LENGTH (valtype) == 4)
> check in alpha could be replaced with gdb_assert (TYPE_LENGTH
> (valtype) == 4).

The assert was in my code, but removed before I post the patches,
because I am not confident putting such strong assert to the code that I
am very familiar with.  I am fine to add it back.

convert_typed_floating calls floatformat_from_type which asserts that
its type code is TYPE_CODE_FLT.

-- 
Yao (齐尧)
From dbc2fb1353e4690c357c7c4cb6a37c26db39d7e2 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Wed, 12 Apr 2017 14:34:27 +0100
Subject: [PATCH] Restrict m68k_convert_register_p

We need to convert register if the type is float.  Suppose we get a value
from float point register, but its type is integer, we don't have to convert.
This case may not exist in real code, but exist in my unit test case.

warning: Cannot convert floating-point register value to non-floating-point type.
Self test failed: arch m68k: self-test failed at gdb/git/gdb/findvar.c:1072

              ok = gdbarch_register_to_value (gdbarch, frame, regnum, type,
                                              buf.data (), &optim, &unavail);

1072:         SELF_CHECK (ok);

gdb:

2017-04-12  Yao Qi  <yao.qi@linaro.org>

	* m68k-tdep.c (m68k_convert_register_p): Check type's code is
	TYPE_CODE_FLT or not.

diff --git a/gdb/m68k-tdep.c b/gdb/m68k-tdep.c
index 7c3bf4c..8b6b568 100644
--- a/gdb/m68k-tdep.c
+++ b/gdb/m68k-tdep.c
@@ -188,6 +188,8 @@ m68k_convert_register_p (struct gdbarch *gdbarch,
   if (!gdbarch_tdep (gdbarch)->fpregs_present)
     return 0;
   return (regnum >= M68K_FP0_REGNUM && regnum <= M68K_FP0_REGNUM + 7
+	  /* We only support floating-point values.  */
+	  && TYPE_CODE (type) == TYPE_CODE_FLT
 	  && type != register_type (gdbarch, M68K_FP0_REGNUM));
 }
 
@@ -203,16 +205,7 @@ m68k_register_to_value (struct frame_info *frame, int regnum,
   struct type *fpreg_type = register_type (get_frame_arch (frame),
 					   M68K_FP0_REGNUM);
 
-  /* We only support floating-point values.  */
-  if (TYPE_CODE (type) != TYPE_CODE_FLT)
-    {
-      warning (_("Cannot convert floating-point register value "
-	       "to non-floating-point type."));
-      *optimizedp = *unavailablep = 0;
-      return 0;
-    }
-
-  /* Convert to TYPE.  */
+  gdb_assert (TYPE_CODE (type) == TYPE_CODE_FLT);
 
   /* Convert to TYPE.  */
   if (!get_frame_register_bytes (frame, regnum, 0, TYPE_LENGTH (type),

Re: [PATCH 9/9] Add unit test to gdbarch methods register_to_value and value_to_register

[Yao Qi]

Simon Marchi <simon.marchi@polymtl.ca> writes:

>> 2017-04-11  Yao Qi  <yao.qi@linaro.org>
>>
>> 	* gdbarch-selftests.c: New file.
>
> Some unit tests are in the unittests directory.  Do we have a rule for
> what goes there versus in gdb/ directly?
>

My understanding is that unit tests to code shared between GDB and
GDBserver go to gdb/unittests, and GDB code unit tests stay in gdb/.

>>
>> +#if GDB_SELF_TEST
>> +extern struct frame_info *create_new_frame (struct gdbarch *gdbarch);
>
> The name create_new_frame suggests that it could be used to create a
> frame used in the standard GDB context.  Maybe create_test_frame would
> be more appropriate?

OK.

>
> Even if this function is only used in tests, a comment explaining what
> it does would be nice, especially since it does more than creating a
> new frame (it sets up some global state).

I'll add comments, but to be clear, I don't expect this function being
used somewhere else.  It is added only for this specific test.

>> +
>> +/* Test gdbarch methods register_to_value and value_to_register.  */
>> +
>> +static void
>> +register_to_value_test (struct gdbarch *gdbarch)
>> +{
>> +  const struct builtin_type *builtin = builtin_type (gdbarch);
>> +  struct type *types[] =
>> +    {
>> +      builtin->builtin_void, builtin->builtin_char,
>> +      builtin->builtin_short, builtin->builtin_int,
>> +      builtin->builtin_long, builtin->builtin_signed_char,
>
> Can you put all of them on their own line?
>

Sure.

>> +      builtin->builtin_unsigned_short,
>> +      builtin->builtin_unsigned_int,
>> +      builtin->builtin_unsigned_long,
>> +      builtin->builtin_float,
>> +      builtin->builtin_double,
>> +      builtin->builtin_long_double,
>> +      builtin->builtin_complex,
>> +      builtin->builtin_double_complex,
>> +      builtin->builtin_string,
>> +      builtin->builtin_bool,
>> +      builtin->builtin_long_long,
>> +      builtin->builtin_unsigned_long_long,
>> +      builtin->builtin_int8,
>> +      builtin->builtin_uint8,
>> +      builtin->builtin_int16,
>> +      builtin->builtin_uint16,
>> +      builtin->builtin_int32,
>> +      builtin->builtin_uint32,
>> +      builtin->builtin_int64,
>> +      builtin->builtin_uint64,
>> +      builtin->builtin_int128,
>> +      builtin->builtin_uint128,
>> +      builtin->builtin_char16,
>> +      builtin->builtin_char32,
>> +    };
>> +
>> +  current_inferior()->gdbarch = gdbarch;
>> +
>> +  struct frame_info *frame = create_new_frame (gdbarch);
>
> If we need to reuse this in other tests, it
>

It is an incomplete sentence.

>> +  const int num_regs = (gdbarch_num_regs (gdbarch)
>> +			+ gdbarch_num_pseudo_regs (gdbarch));
>> +
>> +  /* Test gdbarch methods register_to_value and value_to_register with
>> +     different combinations of register numbers and types.  */
>> +  for (const auto &type : types)
>> +    {
>> +      for (auto regnum = 0; regnum < num_regs; regnum++)
>> +	{
>> +	  if (gdbarch_convert_register_p (gdbarch, regnum, type))
>> +	    {
>> +	      std::vector<gdb_byte> buf (TYPE_LENGTH (type));
>> +	      int optim, unavail, ok;
>> +
>> +	      ok = gdbarch_register_to_value (gdbarch, frame, regnum, type,
>> +					      buf.data (), &optim, &unavail);
>> +
>> +	      SELF_CHECK (ok);
>> +	      SELF_CHECK (!optim);
>> +	      SELF_CHECK (!unavail);
>> +
>> +	      bool saw_no_process_error = false;
>> +	      TRY
>> +		{
>> +		  gdbarch_value_to_register (gdbarch, frame, regnum, type,
>> +					     buf.data ());
>> +		}
>> +	      CATCH (ex, RETURN_MASK_ERROR)
>> +		{
>> +		  if (strcmp (ex.message,
>> +			      "You can't do that without a process to debug.")
>> +		      == 0)
>> +		    saw_no_process_error = true;
>> +		}
>> +
>> +	      /* GDB wants to write registers to target, so it is expected
>> +		 to see no process exception.  */
>> +	      SELF_CHECK (saw_no_process_error);
>
> When you subclass regcache to have a "mock" regcache, do you think
> we'll be able to test the success case of this?
>

We can add two protected methods "target_store_registers" and
"target_prepare_to_store" in class regcache.

void
regcache::target_prepare_to_store ()
{
  target_prepare_to_store (this);
}

In regcache subclass for this test, we can overwrite them to do nothing,
as if the target registers store is successful.

Alternatively, we can pass an object of target when constructing
regcache.  In normal code, we pass current_target, in unit tests, we
pass a "mock" target.  Taking multi-target support into account,
regcache can retrieve the target object from inferior or address space
(different inferiors or address spaces may be associated with different
targets.)

>> +	    }
>> +	}
>> +    }
>> +}
>> +
>> +} // namespace selftests
>> +#endif /* GDB_SELF_TEST */
>> +
>> +/* Suppress warning from -Wmissing-prototypes.  */
>> +extern initialize_file_ftype _initialize_gdbarch_selftests;
>
> I don't think we still need this anymore.

I'll remove it.

-- 
Yao (齐尧)

Re: [PATCH 9/9] Add unit test to gdbarch methods register_to_value and value_to_register

[Simon Marchi]

On 2017-04-19 06:26, Yao Qi wrote:
> Simon Marchi <simon.marchi@polymtl.ca> writes:
>>> +  current_inferior()->gdbarch = gdbarch;
>>> +
>>> +  struct frame_info *frame = create_new_frame (gdbarch);
>> 
>> If we need to reuse this in other tests, it
>> 
> 
> It is an incomplete sentence.

Ah, I think I was going to say:  we might need this in other tests, so 
it could be in a function "setup_test_environment", or something like 
that.  That function would reset all global state, prepare a fake 
inferior, frame, etc.

>>> +  const int num_regs = (gdbarch_num_regs (gdbarch)
>>> +			+ gdbarch_num_pseudo_regs (gdbarch));
>>> +
>>> +  /* Test gdbarch methods register_to_value and value_to_register 
>>> with
>>> +     different combinations of register numbers and types.  */
>>> +  for (const auto &type : types)
>>> +    {
>>> +      for (auto regnum = 0; regnum < num_regs; regnum++)
>>> +	{
>>> +	  if (gdbarch_convert_register_p (gdbarch, regnum, type))
>>> +	    {
>>> +	      std::vector<gdb_byte> buf (TYPE_LENGTH (type));
>>> +	      int optim, unavail, ok;
>>> +
>>> +	      ok = gdbarch_register_to_value (gdbarch, frame, regnum, type,
>>> +					      buf.data (), &optim, &unavail);
>>> +
>>> +	      SELF_CHECK (ok);
>>> +	      SELF_CHECK (!optim);
>>> +	      SELF_CHECK (!unavail);
>>> +
>>> +	      bool saw_no_process_error = false;
>>> +	      TRY
>>> +		{
>>> +		  gdbarch_value_to_register (gdbarch, frame, regnum, type,
>>> +					     buf.data ());
>>> +		}
>>> +	      CATCH (ex, RETURN_MASK_ERROR)
>>> +		{
>>> +		  if (strcmp (ex.message,
>>> +			      "You can't do that without a process to debug.")
>>> +		      == 0)
>>> +		    saw_no_process_error = true;
>>> +		}
>>> +
>>> +	      /* GDB wants to write registers to target, so it is expected
>>> +		 to see no process exception.  */
>>> +	      SELF_CHECK (saw_no_process_error);
>> 
>> When you subclass regcache to have a "mock" regcache, do you think
>> we'll be able to test the success case of this?
>> 
> 
> We can add two protected methods "target_store_registers" and
> "target_prepare_to_store" in class regcache.
> 
> void
> regcache::target_prepare_to_store ()
> {
>   target_prepare_to_store (this);
> }
> 
> In regcache subclass for this test, we can overwrite them to do 
> nothing,
> as if the target registers store is successful.
> 
> Alternatively, we can pass an object of target when constructing
> regcache.  In normal code, we pass current_target, in unit tests, we
> pass a "mock" target.  Taking multi-target support into account,
> regcache can retrieve the target object from inferior or address space
> (different inferiors or address spaces may be associated with different
> targets.)

Ok, that sounds good!

Simon

[PATCH 9/9] Add unit test to gdbarch methods register_to_value and value_to_register

[Yao Qi]

This patch adds one unit test for gdbarch methods register_to_value and
value_to_register.  The test pass different combinations of {regnu, type}
to gdbarch_register_to_value and gdbarch_value_to_register.  In order
to do the test, add a new function create_new_frame to create a fake
frame.  It can be improved after we converted frame_info to class.

In order to isolate regcache (from target_ops operations on writing
registers, like target_store_registers), the sub-class of regcache in the
test override raw_write.  Also, in order to get the right regcache from
get_thread_arch_aspace_regcache, the sub-class of regcache inserts itself
to current_regcache.

Suppose I incorrectly modified the size of buffer as below,

@@ -1228,7 +1228,7 @@ ia64_register_to_value (struct frame_info *frame, int regnum,
                        int *optimizedp, int *unavailablep)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  gdb_byte in[MAX_REGISTER_SIZE];
+  gdb_byte in[1];

   /* Convert to TYPE.  */
   if (!get_frame_register_bytes (frame, regnum, 0,

build GDB with "-fsanitize=address" and run unittest.exp, asan can detect
such error

==2302==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff98193870 at pc 0xbd55ea bp 0x7fff981935a0 sp 0x7fff98193598
WRITE of size 16 at 0x7fff98193870 thread T0
    #0 0xbd55e9 in frame_register_unwind(frame_info*, int, int*, int*, lval_type*, unsigned long*, int*, unsigned char*) /home/yao/SourceCode/gnu/gdb/git/gdb/frame.c:1119
    #1 0xbd58c8 in frame_register(frame_info*, int, int*, int*, lval_type*, unsigned long*, int*, unsigned char*) /home/yao/SourceCode/gnu/gdb/git/gdb/frame.c:1147
    #2 0xbd6e25 in get_frame_register_bytes(frame_info*, int, unsigned long, int, unsigned char*, int*, int*) /home/yao/SourceCode/gnu/gdb/git/gdb/frame.c:1427
    #3 0x70080a in ia64_register_to_value /home/yao/SourceCode/gnu/gdb/git/gdb/ia64-tdep.c:1236
    #4 0xbf570e in gdbarch_register_to_value(gdbarch*, frame_info*, int, type*, unsigned char*, int*, int*) /home/yao/SourceCode/gnu/gdb/git/gdb/gdbarch.c:2619
    #5 0xc05975 in register_to_value_test /home/yao/SourceCode/gnu/gdb/git/gdb/gdbarch-selftests.c:131

Or, even if GDB is not built with asan, GDB just crashes.

*** stack smashing detected ***: ./gdb terminated
Aborted (core dumped)

gdb:

2017-05-09  Yao Qi  <yao.qi@linaro.org>

	* Makefile.in (SFILES): Add gdbarch-selftests.c.
	(COMMON_OBS): Add gdbarch-selftests.o.
	* frame.c [GDB_SELF_TESTS] (create_new_frame): New function.
	* frame.h [GDB_SELF_TESTS] (create_new_frame): Declare.
	* gdbarch-selftests.c: New file.
	* regcache.h (regcache) <~regcache>: Mark it virtual if
	GDB_SELF_TEST.
	<raw_write>: Likewise.
---
 gdb/Makefile.in         |   2 +
 gdb/frame.c             |  17 ++++++
 gdb/frame.h             |   8 +++
 gdb/gdbarch-selftests.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/regcache.h          |  11 ++++
 5 files changed, 197 insertions(+)
 create mode 100644 gdb/gdbarch-selftests.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 70d7d50..6061e79 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1106,6 +1106,7 @@ SFILES = \
 	gdb_obstack.c \
 	gdb_usleep.c \
 	gdbarch.c \
+	gdbarch-selftests.c \
 	gdbtypes.c \
 	gnu-v2-abi.c \
 	gnu-v3-abi.c \
@@ -1719,6 +1720,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	gdb_usleep.o \
 	gdb_vecs.o \
 	gdbarch.o \
+	gdbarch-selftests.o \
 	gdbtypes.o \
 	gnu-v2-abi.o \
 	gnu-v3-abi.o \
diff --git a/gdb/frame.c b/gdb/frame.c
index 18937d9..a30e0b3 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1707,6 +1707,23 @@ select_frame (struct frame_info *fi)
     }
 }
 
+#if GDB_SELF_TEST
+struct frame_info *
+create_test_frame (struct regcache *regcache)
+{
+  struct frame_info *this_frame = XCNEW (struct frame_info);
+
+  sentinel_frame = create_sentinel_frame (NULL, regcache);
+  sentinel_frame->prev = this_frame;
+  sentinel_frame->prev_p = 1;;
+  this_frame->prev_arch.p = 1;
+  this_frame->prev_arch.arch = get_regcache_arch (regcache);
+  this_frame->next = sentinel_frame;
+
+  return this_frame;
+}
+#endif
+
 /* Create an arbitrary (i.e. address specified by user) or innermost frame.
    Always returns a non-NULL value.  */
 
diff --git a/gdb/frame.h b/gdb/frame.h
index 1d0644f..56cbd44 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -833,6 +833,14 @@ extern struct frame_info *deprecated_safe_get_selected_frame (void);
 
 extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR pc);
 
+#if GDB_SELF_TEST
+
+/* Create a frame for unit test.  Its next frame is sentinel frame,
+   created from REGCACHE.  */
+
+extern struct frame_info *create_test_frame (struct regcache *regcache);
+#endif
+
 /* Return true if the frame unwinder for frame FI is UNWINDER; false
    otherwise.  */
 
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
new file mode 100644
index 0000000..68ab0b7
--- /dev/null
+++ b/gdb/gdbarch-selftests.c
@@ -0,0 +1,159 @@
+/* Self tests for gdbarch for GDB, the GNU debugger.
+
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#if GDB_SELF_TEST
+#include "selftest.h"
+#include "selftest-arch.h"
+#include "inferior.h"
+
+namespace selftests {
+
+/* A read-write regcache which doesn't write the target.  */
+
+class regcache_test : public regcache
+{
+public:
+  explicit regcache_test (struct gdbarch *gdbarch)
+    : regcache (gdbarch, NULL, false)
+  {
+    set_ptid (inferior_ptid);
+
+    current_regcache.push_front (this);
+  }
+
+  void raw_write (int regnum, const gdb_byte *buf) override
+  {
+    raw_set_cached_value (regnum, buf);
+  }
+};
+
+/* Test gdbarch methods register_to_value and value_to_register.  */
+
+static void
+register_to_value_test (struct gdbarch *gdbarch)
+{
+  const struct builtin_type *builtin = builtin_type (gdbarch);
+  struct type *types[] =
+    {
+      builtin->builtin_void,
+      builtin->builtin_char,
+      builtin->builtin_short,
+      builtin->builtin_int,
+      builtin->builtin_long,
+      builtin->builtin_signed_char,
+      builtin->builtin_unsigned_short,
+      builtin->builtin_unsigned_int,
+      builtin->builtin_unsigned_long,
+      builtin->builtin_float,
+      builtin->builtin_double,
+      builtin->builtin_long_double,
+      builtin->builtin_complex,
+      builtin->builtin_double_complex,
+      builtin->builtin_string,
+      builtin->builtin_bool,
+      builtin->builtin_long_long,
+      builtin->builtin_unsigned_long_long,
+      builtin->builtin_int8,
+      builtin->builtin_uint8,
+      builtin->builtin_int16,
+      builtin->builtin_uint16,
+      builtin->builtin_int32,
+      builtin->builtin_uint32,
+      builtin->builtin_int64,
+      builtin->builtin_uint64,
+      builtin->builtin_int128,
+      builtin->builtin_uint128,
+      builtin->builtin_char16,
+      builtin->builtin_char32,
+    };
+
+  current_inferior()->gdbarch = gdbarch;
+
+  struct regcache *regcache = new regcache_test (gdbarch);
+  struct frame_info *frame = create_test_frame (regcache);
+  const int num_regs = (gdbarch_num_regs (gdbarch)
+			+ gdbarch_num_pseudo_regs (gdbarch));
+
+  SELF_CHECK (regcache == get_current_regcache ());
+
+  /* Test gdbarch methods register_to_value and value_to_register with
+     different combinations of register numbers and types.  */
+  for (const auto &type : types)
+    {
+      for (auto regnum = 0; regnum < num_regs; regnum++)
+	{
+	  if (gdbarch_convert_register_p (gdbarch, regnum, type))
+	    {
+	      std::vector<gdb_byte> expected (TYPE_LENGTH (type), 0);
+
+	      if (TYPE_CODE (type) == TYPE_CODE_FLT)
+		{
+		  DOUBLEST d = 1.25;
+
+		  /* Generate valid float format.  */
+		  floatformat_from_doublest (floatformat_from_type (type),
+					     &d, expected.data ());
+		}
+	      else
+		{
+		  for (auto j = 0; j < expected.size (); j++)
+		    expected[j] = (regnum + j) % 16;
+		}
+
+	      gdbarch_value_to_register (gdbarch, frame, regnum, type,
+					 expected.data ());
+
+	      /* Allocate two bytes more for overflow check.  */
+	      std::vector<gdb_byte> buf (TYPE_LENGTH (type) + 2, 0);
+	      int optim, unavail, ok;
+
+	      /* Set the fingerprint in the last two bytes.  */
+	      buf [TYPE_LENGTH (type)]= 'w';
+	      buf [TYPE_LENGTH (type) + 1]= 'l';
+	      ok = gdbarch_register_to_value (gdbarch, frame, regnum, type,
+					      buf.data (), &optim, &unavail);
+
+	      SELF_CHECK (ok);
+	      SELF_CHECK (!optim);
+	      SELF_CHECK (!unavail);
+
+	      SELF_CHECK (buf[TYPE_LENGTH (type)] == 'w');
+	      SELF_CHECK (buf[TYPE_LENGTH (type) + 1] == 'l');
+
+	      for (auto k = 0;
+		   k < std::min ((unsigned int) register_size (gdbarch, regnum),
+				 TYPE_LENGTH (type));
+		   k++)
+		SELF_CHECK (buf[k] == expected[k]);
+	    }
+	}
+    }
+}
+
+} // namespace selftests
+#endif /* GDB_SELF_TEST */
+
+void
+_initialize_gdbarch_selftests (void)
+{
+#if GDB_SELF_TEST
+  register_self_test_foreach_arch (selftests::register_to_value_test);
+#endif
+}
diff --git a/gdb/regcache.h b/gdb/regcache.h
index fdc47ba..11126e6 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -250,6 +250,11 @@ public:
   regcache (const regcache &) = delete;
   void operator= (const regcache &) = delete;
 
+  /* class regcache is only extended in unit test, so only mark it
+     virtual when selftest is enabled.  */
+#if GDB_SELF_TEST
+  virtual
+#endif
   ~regcache ()
   {
     xfree (m_registers);
@@ -269,6 +274,12 @@ public:
   void cooked_write (int regnum, const gdb_byte *buf);
 
   enum register_status raw_read (int regnum, gdb_byte *buf);
+
+  /* class regcache is only extended in unit test, so only mark it
+     virtual when selftest is enabled.  */
+#if GDB_SELF_TEST
+  virtual
+#endif
   void raw_write (int regnum, const gdb_byte *buf);
 
   enum register_status raw_read_signed (int regnum, LONGEST *val);
-- 
1.9.1