From patchwork Wed Nov 30 18:38:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 18077 Received: (qmail 29943 invoked by alias); 30 Nov 2016 18:38:36 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 29932 invoked by uid 89); 30 Nov 2016 18:38:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.8 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=50724, illustration X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 30 Nov 2016 18:38:34 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 36D8865738; Wed, 30 Nov 2016 18:38:33 +0000 (UTC) Received: from [127.0.0.1] (ovpn03.gateway.prod.ext.phx2.redhat.com [10.5.9.3]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uAUIcWqF018029; Wed, 30 Nov 2016 13:38:32 -0500 Subject: Re: [PATCH 1/2] Add unit test to aarch64 prologue analyzer To: Yao Qi , gdb-patches@sourceware.org References: <1480428758-2481-1-git-send-email-yao.qi@linaro.org> <249b2f2d-a678-07e8-bc75-df8128f3d8f5@redhat.com> From: Pedro Alves Message-ID: <8d6a008a-c4f0-1197-e423-eac2b0898edd@redhat.com> Date: Wed, 30 Nov 2016 18:38:31 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <249b2f2d-a678-07e8-bc75-df8128f3d8f5@redhat.com> 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 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 init) - : insns{init} {} + + template + 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 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 */