This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/2 v2] Add unit test to aarch64 prologue analyzer


On Thu, Dec 01, 2016 at 06:05:30PM +0000, Pedro Alves wrote:
> > 
> > I managed to get emacs indent at column 0 with the following changes in
> > gdb/.dir-locals.el.
> 
> That unfortunately somehow makes emacs indent members of classes
> (even if not in a namespace) at column 0, like:
> 
> struct foo
> {
> int a;
> };
> 
> Do you get that too?
> 

Yes, :(

> BTW, the don't-indent-body-of-namespace guideline is here:
> 
>   https://gcc.gnu.org/codingconventions.html#Namespace_Form
> 
> and it does look like GCC generally follows it.

I still can't get me emacs indent code inside namespace that way, but
I manually fixed it.  I'll ask gcc people how do they set emacs.

> 
> (The '{'-on-same-line rule is there too.)
> 

Fixed.  Patch is pushed in.

-- 
Yao (齐尧)

>From 4d9a9006139d1ceea787cdda871dff8943e493f0 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Fri, 2 Dec 2016 09:37:30 +0000
Subject: [PATCH] Add unit test to aarch64 prologue analyzer

We don't have an effective way to test prologue analyzer which is
highly dependent on instruction patterns in prologue generated by
compiler.  GDB prologue analyzer may not handle the new sequences
generated by new compiler, or may still handle some sequences that
generated by very old compilers which are no longer used.  The
former is a functionality issue, while the latter is a maintenance
issue.

The input and output of prologue analyzer is quite clear, so it
fits for unit test.  The input is series of instructions, and the
output are 1) where prologue end, 2) where registers are saved.
In aarch64, they are represented in 'struct aarch64_prologue_cache'.

This patch refactors aarch64_analyze_prologue so it can read
instructions from either real target or test harness.  In unit
test aarch64_analyze_prologue_test, aarch64_analyze_prologue gets
instructions we prepared in the test, as the input of prologue
analyzer.  Then, we checked various fields in
'struct aarch64_prologue_cache'.

gdb:

2016-12-02  Yao Qi  <yao.qi@linaro.org>
	    Pedro Alves  <palves@redhat.com>

	* aarch64-tdep.c: Include "selftest.h".
	(abstract_instruction_reader): New class.
	(instruction_reader): New class.
	(aarch64_analyze_prologue): Add new parameter reader.  Call
	reader.read instead of read_memory_unsigned_integer.
	[GDB_SELF_TEST] (instruction_reader_test): New class.
	(aarch64_analyze_prologue_test): New function.
	(_initialize_aarch64_tdep) [GDB_SELF_TEST]: Register
	selftests::aarch64_analyze_prologue_test.
	* trad-frame.c (trad_frame_cache_zalloc):
	(trad_frame_alloc_saved_regs): Add a new function.
	* trad-frame.h (trad_frame_alloc_saved_regs): Declare.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 620b582..b4dd117 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,19 @@
+2016-12-02  Yao Qi  <yao.qi@linaro.org>
+	    Pedro Alves  <palves@redhat.com>
+
+	* aarch64-tdep.c: Include "selftest.h".
+	(abstract_instruction_reader): New class.
+	(instruction_reader): New class.
+	(aarch64_analyze_prologue): Add new parameter reader.  Call
+	reader.read instead of read_memory_unsigned_integer.
+	[GDB_SELF_TEST] (instruction_reader_test): New class.
+	(aarch64_analyze_prologue_test): New function.
+	(_initialize_aarch64_tdep) [GDB_SELF_TEST]: Register
+	selftests::aarch64_analyze_prologue_test.
+	* trad-frame.c (trad_frame_cache_zalloc):
+	(trad_frame_alloc_saved_regs): Add a new function.
+	* trad-frame.h (trad_frame_alloc_saved_regs): Declare.
+
 2016-12-01  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	* ui-out.c (enum ui_out_table_state): Move to class
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 6b95d7c..576ee70 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -44,6 +44,7 @@
 #include "infcall.h"
 #include "ax.h"
 #include "ax-gdb.h"
+#include "selftest.h"
 
 #include "aarch64-tdep.h"
 
@@ -195,6 +196,27 @@ show_aarch64_debug (struct ui_file *file, int from_tty,
   fprintf_filtered (file, _("AArch64 debugging is %s.\n"), value);
 }
 
+/* Abstract instruction reader.  */
+
+class abstract_instruction_reader
+{
+public:
+  /* Read in one instruction.  */
+  virtual ULONGEST read (CORE_ADDR memaddr, int len,
+			 enum bfd_endian byte_order) = 0;
+};
+
+/* Instruction reader from real target.  */
+
+class instruction_reader : public abstract_instruction_reader
+{
+ public:
+  ULONGEST read (CORE_ADDR memaddr, int len, enum bfd_endian byte_order)
+  {
+    return read_memory_unsigned_integer (memaddr, len, byte_order);
+  }
+};
+
 /* Analyze a prologue, looking for a recognizable stack frame
    and frame pointer.  Scan until we encounter a store that could
    clobber the stack frame unexpectedly, or an unknown instruction.  */
@@ -202,7 +224,8 @@ show_aarch64_debug (struct ui_file *file, int from_tty,
 static CORE_ADDR
 aarch64_analyze_prologue (struct gdbarch *gdbarch,
 			  CORE_ADDR start, CORE_ADDR limit,
-			  struct aarch64_prologue_cache *cache)
+			  struct aarch64_prologue_cache *cache,
+			  abstract_instruction_reader& reader)
 {
   enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
   int i;
@@ -221,7 +244,7 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
       uint32_t insn;
       aarch64_inst inst;
 
-      insn = read_memory_unsigned_integer (start, 4, byte_order_for_code);
+      insn = reader.read (start, 4, byte_order_for_code);
 
       if (aarch64_decode_insn (insn, &inst, 1) != 0)
 	break;
@@ -436,6 +459,96 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
   return start;
 }
 
+static CORE_ADDR
+aarch64_analyze_prologue (struct gdbarch *gdbarch,
+			  CORE_ADDR start, CORE_ADDR limit,
+			  struct aarch64_prologue_cache *cache)
+{
+  instruction_reader reader;
+
+  return aarch64_analyze_prologue (gdbarch, start, limit, cache,
+				   reader);
+}
+
+#if GDB_SELF_TEST
+
+namespace selftests {
+
+/* Instruction reader from manually cooked instruction sequences.  */
+
+class instruction_reader_test : public abstract_instruction_reader
+{
+public:
+  template<size_t SIZE>
+  explicit instruction_reader_test (const uint32_t (&insns)[SIZE])
+  : m_insns (insns), m_insns_size (SIZE)
+  {}
+
+  ULONGEST read (CORE_ADDR memaddr, int len, enum bfd_endian byte_order)
+  {
+    SELF_CHECK (len == 4);
+    SELF_CHECK (memaddr % 4 == 0);
+    SELF_CHECK (memaddr / 4 < m_insns_size);
+
+    return m_insns[memaddr / 4];
+  }
+
+private:
+  const uint32_t *m_insns;
+  size_t m_insns_size;
+};
+
+static void
+aarch64_analyze_prologue_test (void)
+{
+  struct gdbarch_info info;
+
+  gdbarch_info_init (&info);
+  info.bfd_arch_info = bfd_scan_arch ("aarch64");
+
+  struct gdbarch *gdbarch = gdbarch_find_by_info (info);
+  SELF_CHECK (gdbarch != NULL);
+
+  /* Test the simple prologue in which frame pointer is used.  */
+  {
+    struct aarch64_prologue_cache cache;
+    cache.saved_regs = trad_frame_alloc_saved_regs (gdbarch);
+
+    static const uint32_t insns[] = {
+      0xa9af7bfd, /* stp     x29, x30, [sp,#-272]! */
+      0x910003fd, /* mov     x29, sp */
+      0x97ffffe6, /* bl      0x400580 */
+    };
+    instruction_reader_test reader (insns);
+
+    CORE_ADDR end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache, reader);
+    SELF_CHECK (end == 4 * 2);
+
+    SELF_CHECK (cache.framereg == AARCH64_FP_REGNUM);
+    SELF_CHECK (cache.framesize == 272);
+
+    for (int i = 0; i < AARCH64_X_REGISTER_COUNT; i++)
+      {
+	if (i == AARCH64_FP_REGNUM)
+	  SELF_CHECK (cache.saved_regs[i].addr == -272);
+	else if (i == AARCH64_LR_REGNUM)
+	  SELF_CHECK (cache.saved_regs[i].addr == -264);
+	else
+	  SELF_CHECK (cache.saved_regs[i].addr == -1);
+      }
+
+    for (int i = 0; i < AARCH64_D_REGISTER_COUNT; i++)
+      {
+	int regnum = gdbarch_num_regs (gdbarch);
+
+	SELF_CHECK (cache.saved_regs[i + regnum + AARCH64_D0_REGNUM].addr
+		    == -1);
+      }
+  }
+}
+} // namespace selftests
+#endif /* GDB_SELF_TEST */
+
 /* Implement the "skip_prologue" gdbarch method.  */
 
 static CORE_ADDR
@@ -2864,6 +2977,10 @@ When on, AArch64 specific debugging is enabled."),
 			    NULL,
 			    show_aarch64_debug,
 			    &setdebuglist, &showdebuglist);
+
+#if GDB_SELF_TEST
+  register_self_test (selftests::aarch64_analyze_prologue_test);
+#endif
 }
 
 /* AArch64 process record-replay related structures, defines etc.  */
diff --git a/gdb/trad-frame.c b/gdb/trad-frame.c
index ebf19df..4430dd5 100644
--- a/gdb/trad-frame.c
+++ b/gdb/trad-frame.c
@@ -43,16 +43,10 @@ trad_frame_cache_zalloc (struct frame_info *this_frame)
   return this_trad_cache;
 }
 
-/* A traditional frame is unwound by analysing the function prologue
-   and using the information gathered to track registers.  For
-   non-optimized frames, the technique is reliable (just need to check
-   for all potential instruction sequences).  */
-
 struct trad_frame_saved_reg *
-trad_frame_alloc_saved_regs (struct frame_info *this_frame)
+trad_frame_alloc_saved_regs (struct gdbarch *gdbarch)
 {
   int regnum;
-  struct gdbarch *gdbarch = get_frame_arch (this_frame);
   int numregs = gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch);
   struct trad_frame_saved_reg *this_saved_regs
     = FRAME_OBSTACK_CALLOC (numregs, struct trad_frame_saved_reg);
@@ -65,6 +59,19 @@ trad_frame_alloc_saved_regs (struct frame_info *this_frame)
   return this_saved_regs;
 }
 
+/* A traditional frame is unwound by analysing the function prologue
+   and using the information gathered to track registers.  For
+   non-optimized frames, the technique is reliable (just need to check
+   for all potential instruction sequences).  */
+
+struct trad_frame_saved_reg *
+trad_frame_alloc_saved_regs (struct frame_info *this_frame)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+
+  return trad_frame_alloc_saved_regs (gdbarch);
+}
+
 enum { TF_REG_VALUE = -1, TF_REG_UNKNOWN = -2 };
 
 int
diff --git a/gdb/trad-frame.h b/gdb/trad-frame.h
index b8aed16..d1c24b0 100644
--- a/gdb/trad-frame.h
+++ b/gdb/trad-frame.h
@@ -104,6 +104,7 @@ int trad_frame_realreg_p (struct trad_frame_saved_reg this_saved_regs[],
 
 /* Return a freshly allocated (and initialized) trad_frame array.  */
 struct trad_frame_saved_reg *trad_frame_alloc_saved_regs (struct frame_info *);
+struct trad_frame_saved_reg *trad_frame_alloc_saved_regs (struct gdbarch *);
 
 /* Given the trad_frame info, return the location of the specified
    register.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]