From patchwork Mon Mar 12 14:15:51 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 26283 Received: (qmail 1446 invoked by alias); 12 Mar 2018 14:16:10 -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 857 invoked by uid 89); 12 Mar 2018 14:16:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=drops X-HELO: mail-wm0-f67.google.com Received: from mail-wm0-f67.google.com (HELO mail-wm0-f67.google.com) (74.125.82.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 12 Mar 2018 14:16:00 +0000 Received: by mail-wm0-f67.google.com with SMTP id t3so16845542wmc.2 for ; Mon, 12 Mar 2018 07:15:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=Cc2EK+b6CawvNsCaLUfkOuNcK8T0Rg/P9XWkzSeiC/k=; b=V61RvGXj8Z6GjWVRD4rcFVs80XmQigXWWt+9s31+CIy7tNFwhf5HUYvQaV2w/g5SRr 3NmytkSNZeeeCC9X92cYuU3Jp7RxiH/80ziaRSNJU0SQvhCmdOPYOzWayP4SQS7qOzWa 5uSdmvIYdrTBVcu1wAi2HYOYi/XY2cuPc9mXM+XiKwZ4IlznctjAYAKFZ/ar8ir1LapU DwAOYMzpIK4xPS4lt71vYwUT5rPzw1YLVw58KrzdKpFuW1rKleUpVbqvqEg1gwjDpbeI KJ6wk33oHMLcIVUE11Vus9NCQcXZq9jMXEPbJHn4PUzapnRPBcfaamv0+TCZMIqmrHHC IESw== X-Gm-Message-State: AElRT7HVAg74U+uQ6IvqhZirrCrFIr05sRTJDfrwZ9RTfAtfqdpgm0ZR zaxh/qGMS9XIRBOJhJGGyspX4x04 X-Google-Smtp-Source: AG47ELtP/IXJsg1IM065Mvba4bR9GuE5FJ1+c9CWAFITcYf5LC3jOOB8+HCNDt0jNdkmeuFcEkh2Jw== X-Received: by 10.28.61.65 with SMTP id k62mr6200199wma.140.1520864157545; Mon, 12 Mar 2018 07:15:57 -0700 (PDT) Received: from E107787-LIN (static.42.136.251.148.clients.your-server.de. [148.251.136.42]) by smtp.gmail.com with ESMTPSA id c14sm8507681wrd.17.2018.03.12.07.15.56 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 12 Mar 2018 07:15:57 -0700 (PDT) From: Yao Qi To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Make arm_record_test work on big-endian machines References: <20180312030943.32669-1-simon.marchi@polymtl.ca> Date: Mon, 12 Mar 2018 14:15:51 +0000 In-Reply-To: <20180312030943.32669-1-simon.marchi@polymtl.ca> (Simon Marchi's message of "Sun, 11 Mar 2018 23:09:43 -0400") Message-ID: <86tvtle5wo.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Simon Marchi writes: > While running selftests on a big-endian PPC, I noticed arm_record_test > failed a test. The reason is that the gdbarch_find_by_info call is done > with BFD_ENDIAN_UNKNOWN byte order in the info structure. In that case, > it uses the byte order of the current default BFD, which happened to be > big-endian on that GDB build, and the gdbarch returned is a big-endian > one. The instruction used for the 32-bits part of the test is written > in little-endian form, so GDB fails to decode the instruction > properly. The test instructions are endianess-neutral, because they are written as uint16_t, and that is why you didn't have to adjust 16-bit thumb instructions in the tests. > > Since ARM supports both endiannesses, and it should be possible to debug > using an host of both endiannesses, I changed the test to check with > gdbarches of both endiannesses. Although ARM supports both endianesses, the big endian has two variants, BE-32 and BE-8. Before ARMv6, it was BE-32. In ARMv6, BE-8 must be supported, but support of BE-32 is implementation defined. ARMv7 drops the BE-32 support. With BE-8, data is big endian and code is little endian. Thumb-2 instruction was introduced in armv6t2, so it is still likely to have both big endian and little endian thumb-2 instructions code. As I said, the test case is written in an endianess-neutral way, static const uint16_t insns[] = { /* 1d ee 70 7f mrc 15, 0, r7, cr13, cr0, {3} */ 0xee1d, 0x7f70, }; but the arm process recording processes thumb-2 instruction as one 32-bit instruction, so that it has this, /* Swap first half of 32bit thumb instruction with second half. */ arm_record->arm_insn = (arm_record->arm_insn >> 16) | (arm_record->arm_insn << 16); I wonder this is the cause of the test fail, that is, we don't need to swap them in big endian. Since I've never run GDB tests on arm big endian target, I am not very confident on my fix below. It fixes the test fail on ppc64, and also the fail with your patch to change the test with two different endianess. diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index ef7e66b..153f568 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -13188,10 +13188,13 @@ decode_insn (abstract_memory_reader &reader, insn_decode_record *arm_record, /* As thumb does not have condition codes, we set negative. */ arm_record->cond = -1; - /* Swap first half of 32bit thumb instruction with second half. */ - arm_record->arm_insn - = (arm_record->arm_insn >> 16) | (arm_record->arm_insn << 16); - + if (gdbarch_byte_order_for_code (arm_record->gdbarch) + == BFD_ENDIAN_LITTLE) + { + /* Swap first half of 32bit thumb instruction with second half. */ + arm_record->arm_insn + = (arm_record->arm_insn >> 16) | (arm_record->arm_insn << 16); + } ret = thumb2_record_decode_insn_handler (arm_record); if (ret != ARM_RECORD_SUCCESS)