This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/2] Add unit test to aarch64 prologue analyzer
On 11/30/2016 06:29 PM, Pedro Alves wrote:
> I'll note that it always itches me a bit when we do
> unnecessary copying. :-) In this case, you always
> start from an array of instructions known at compile-time,
> and copy it into the vector at run time. You could
> instead create the instructions array as a separate const
> array, and pass than to the reader's constructor
> as parameter, which would store a pointer to the array,
> instead of a deep copy.
I applied the series locally and gave this a go, to
show what I meant. See below.
You'd indent the body of the new scopes, of course.
I didn't do that just to keep the illustration readable.
Move each test to a separate function would be another option.
>From 9ec53dbaa9d4b463849a92b56c579b728eee2830 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 30 Nov 2016 17:31:27 +0000
Subject: [PATCH] selftest
---
gdb/aarch64-tdep.c | 60 +++++++++++++++++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 26 deletions(-)
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 8754eb0..96f9b34 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -211,8 +211,6 @@ public:
class instruction_reader : public abstract_instruction_reader
{
public:
- instruction_reader () = default;
-
ULONGEST read (CORE_ADDR memaddr, int len, enum bfd_endian byte_order)
{
return read_memory_unsigned_integer (memaddr, len, byte_order);
@@ -495,7 +493,7 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
CORE_ADDR start, CORE_ADDR limit,
struct aarch64_prologue_cache *cache)
{
- instruction_reader reader { };
+ instruction_reader reader;
return aarch64_analyze_prologue (gdbarch, start, limit, cache,
reader);
@@ -509,21 +507,24 @@ namespace selftests {
class instruction_reader_test : public abstract_instruction_reader
{
public:
- instruction_reader_test () = delete;
- instruction_reader_test (std::initializer_list<uint32_t> init)
- : insns{init} {}
+
+ template<size_t SIZE>
+ 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 < insns.size());
+ SELF_CHECK (memaddr / 4 < m_insns_size);
- return insns[memaddr / 4];
+ return m_insns[memaddr / 4];
}
private:
- std::vector<uint32_t> insns;
+ const uint32_t *m_insns;
+ size_t m_insns_size;
};
static void
@@ -537,17 +538,19 @@ aarch64_analyze_prologue_test (void)
struct gdbarch *gdbarch = gdbarch_find_by_info (info);
SELF_CHECK (gdbarch != NULL);
+ /* Say something more descriptive here? */
+ {
struct aarch64_prologue_cache cache;
cache.saved_regs = trad_frame_alloc_saved_regs (gdbarch);
- instruction_reader_test test {
- 0xa9af7bfd, /* stp x29, x30, [sp,#-272]! */
- 0x910003fd, /* mov x29, sp */
- 0x97ffffe6, /* bl 0x400580 */
- };
+ 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, test);
+ CORE_ADDR end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache, reader);
SELF_CHECK (end == 4 * 2);
SELF_CHECK (cache.framereg == AARCH64_FP_REGNUM);
@@ -569,20 +572,24 @@ aarch64_analyze_prologue_test (void)
SELF_CHECK (cache.saved_regs[i + regnum + AARCH64_D0_REGNUM].addr == -1);
}
+ }
- /* Second test. */
+ /* Second test. Say something more descriptive here? */
+ {
+ struct aarch64_prologue_cache cache;
cache.saved_regs = trad_frame_alloc_saved_regs (gdbarch);
- instruction_reader_test test2 {
- 0xf81d0ff3, /* str x19, [sp, #-48]! */
- 0xb9002fe0, /* str w0, [sp, #44] */
- 0xf90013e1, /* str x1, [sp, #32]*/
- 0xfd000fe0, /* str d0, [sp, #24] */
- 0xaa0203f3, /* mov x19, x2 */
- 0xf94013e0, /* ldr x0, [sp, #32] */
- };
+ const uint32_t insns[] = {
+ 0xf81d0ff3, /* str x19, [sp, #-48]! */
+ 0xb9002fe0, /* str w0, [sp, #44] */
+ 0xf90013e1, /* str x1, [sp, #32]*/
+ 0xfd000fe0, /* str d0, [sp, #24] */
+ 0xaa0203f3, /* mov x19, x2 */
+ 0xf94013e0, /* ldr x0, [sp, #32] */
+ };
+ instruction_reader_test reader (insns);
- end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache, test2);
+ CORE_ADDR end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache, reader);
SELF_CHECK (end == 4 * 5);
@@ -608,6 +615,7 @@ aarch64_analyze_prologue_test (void)
else
SELF_CHECK (cache.saved_regs[i + regnum + AARCH64_D0_REGNUM].addr == -1);
}
+ }
}
}
#endif /* GDB_SELF_TEST */
--
2.5.5