From patchwork Thu Mar 30 18:31:26 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antoine Tremblay X-Patchwork-Id: 19777 Received: (qmail 61667 invoked by alias); 30 Mar 2017 18:31:48 -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 61547 invoked by uid 89); 30 Mar 2017 18:31:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.2 spammy=H*RU:193.180.251.48, Hx-spam-relays-external:193.180.251.48 X-HELO: sesbmg22.ericsson.net Received: from sesbmg22.ericsson.net (HELO sesbmg22.ericsson.net) (193.180.251.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 30 Mar 2017 18:31:43 +0000 Received: from ESESSHC011.ericsson.se (Unknown_Domain [153.88.183.51]) by (Symantec Mail Security) with SMTP id 01.1A.25230.D0F4DD85; Thu, 30 Mar 2017 20:31:42 +0200 (CEST) Received: from EUR01-VE1-obe.outbound.protection.outlook.com (153.88.183.145) by oa.msg.ericsson.com (153.88.183.51) with Microsoft SMTP Server (TLS) id 14.3.339.0; Thu, 30 Mar 2017 20:31:40 +0200 Authentication-Results: sourceware.org; dkim=none (message not signed) header.d=none; sourceware.org; dmarc=none action=none header.from=ericsson.com; Received: from elxa4wqvvz1 (192.75.88.130) by HE1PR0701MB1883.eurprd07.prod.outlook.com (10.167.247.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1005.2; Thu, 30 Mar 2017 18:31:37 +0000 References: <20161129120702.9490-1-antoine.tremblay@ericsson.com> <20170127150139.GB24676@E107787-LIN> <2255ed6f-a146-026c-f871-00e9a33dfcf0@redhat.com> <86d1cy4umo.fsf@gmail.com> User-agent: mu4e 0.9.19; emacs 25.1.1 From: Antoine Tremblay To: Yao Qi CC: Antoine Tremblay , Pedro Alves , "gdb-patches@sourceware.org" Subject: Re: [PATCH 1/2] This patch fixes GDBServer's run control for single stepping In-Reply-To: <86d1cy4umo.fsf@gmail.com> Date: Thu, 30 Mar 2017 14:31:26 -0400 Message-ID: MIME-Version: 1.0 X-ClientProxiedBy: CY4PR20CA0015.namprd20.prod.outlook.com (10.173.116.153) To HE1PR0701MB1883.eurprd07.prod.outlook.com (10.167.247.23) X-MS-Office365-Filtering-Correlation-Id: 225bd0e0-a1db-4b18-683d-08d4779b0156 X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(201703131423075)(201703031133081); SRVR:HE1PR0701MB1883; X-Microsoft-Exchange-Diagnostics: 1; HE1PR0701MB1883; 3:xVVZ28FCOFEUS2zTy43I9ieD6yK06QhNZKbURBMbODTrQKW96KKycAaBTy9WtPU2QvD+EBPOzkqh73z9Mpvx/JUiKonKc4BM0PGAhZ7mGaOhjb96Fz3YrQ4gqDaj8946+MWnlfgI+AjxfS9PBoxeruFGmc7HaLA/rI8c5WM4SliJpnD7hCOaO7pG0vJX+jmqOw8SS4h8GEFdL2Zoj7og7aerNpheGUUqRZk77/punJL4VQE4FZpZcyN/+Ll9buiwqM2apD1ljre/bexGfUQTqRc5EUO7048VOlpkYlCGAiEjiPI6OwJNitxFmfqrWc5d51vsWlu3LkUfWrKJ96SwcA==; 25:QnB6xjfqMRYi8+TdalSBr67pDlVhqPm+g5V59QhCuLzGpMLW6q+CRn0E4S9ERPu2mYrfH1wFt3FVytThpzo6GJuesJC/81j4b3f50YXDgtKo+eupsvhfy7ZJmDm3dZ2pGw8fQp080uobSMD9/nZNoBLZTh55NBhmz28R6TRIFTqsBQsz57pJxQEeazXArxUK1gKYwCvpQHDmroTb4XUbO00amyAMTrpQ0zDYSmwRX8sNTmvnghPpwyuowG4sQCqhHYoWJn1ONiyxEQIOU7S/0x3p8x0EW6S9TlDrn2ddXr6Rt6X1U2bcgwR0iW4/NHd8CgOYr7pPIh4W2vGkyGfT8ViqiIv+8fxXIf4UkGbUc4Y8slTnNaRMnsOTq+YlUSDwEOmKkB/KGGpmnH7aRnP10ZHExeAdVpjlS9FmitWh1UM56IYqj3jiInXgEnpMAWAZNxbsXDbSiS00K1nIreEDvg== X-Microsoft-Exchange-Diagnostics: 1; HE1PR0701MB1883; 31:Tw/CwOaq5buKvTavZY3QKV01sydI/t9aGcXjADNrg46/tCNyoqzaO3gAyKOz7lqwvYUcZVEwES6iFuLYBwhbDu/KNBn/MlZhZdimp7A1W/9j/LnkfWg6VNKheg2+pSc+XCuRoOncUX6bCElYwkXsKOrx6ljE50VNlBk9Mrm6PGlH4chTyLgW25vkINZ3kO7Xe+12bdMkE3JYgIRUa1T31Dv7a1wWEX3MUk30eKj+FVRFFmNoSF88M/E54wAblMEk; 20:IWNUxxZJCUetuYQSvuB3TIS/OWQCFqtong+FLhWkwgNAqAOmg72/WmgYTjLzc3ZQmjVlsl2BN0a29YPVIJwxgEIUOIkHnVu2tj0w+Kit1jVDXsLpIUrpcsBdqsDT1PWkaJb8c0uTjHRXx+Z2zV0Kw1GjGE7nnb44JVVHHYY8IRzMv1hr7wnL3FlgVJitKGZDjJ1s6DeIi/d72wJfKJO67OazRgk7Fjiju2k3lnMGWQy+lHCtfvZ5jXo7lx2hHBuHAVAVH4FQ6cz8XYglAhXL8ogI5CJ31BPPqMYiQyLhD18ZvhEUwryRtQnhwqATWnZ0RaR0gMGViWd+h0usSJeNBR9ivhb0l3sZdDzH1/fynVDmv20OHmjWDLNYvCjwi7fEOibU8M6gV0QC8JuXoVOXmPZgZpXFQawQQK/BI9krN5R3KZqoJR971x1uMNrBLVPK49ePk4pNlpoMI1kjCzF7tFsOsKR+vzbzEbH5Ueyx3Iy6+Y2Rci5Lec0ihHQyB4m1 X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(37575265505322); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6040450)(601004)(2401047)(8121501046)(5005006)(10201501046)(93006067)(93001067)(3002001)(6041248)(20161123564025)(20161123560025)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(20161123562025)(6072148); SRVR:HE1PR0701MB1883; BCL:0; PCL:0; RULEID:; SRVR:HE1PR0701MB1883; X-Microsoft-Exchange-Diagnostics: 1; HE1PR0701MB1883; 4:z3w37DNs2f7gmAQiE0RgZIJE1WHS2g2jCbFlOn3kYckswNElfKsHAQZO9xgnpb5SgwgrliKKmUmVX8LAdnH+7v8R2zIxWID7bp0FHcZVn7ykH8sa3fPtaWkO30/ZVtwAmkfUVYYZBvizbXqTYO89qoHwCEBeh9JipLBd5dlU0lwa4Jw/bdFG/DdomW4YGFQ9MyetUw6xX0Oxi46CIKzktlNtcsFQVAAs46exybR1FCrMq6s/JwIUp0VGAze3AK/h9spTGItfqjK1+Brr9LvCgmPyEZ1NF+RSPkGBHtGjLJF5ntonidvrVoGf7HLFXer7CovkOnLyOWP7zd0EUDJFKRPZDO6amkCbyWMV8CdKm3uuAekLii9dUniUdFmXg9+Acqf8epJm5UOTBdlYmxt6rLyGQmQKOHJ75AXH1F3IkmM1C7rkksOyfxtCKaJhWKHGg5LSbzLxvckEwPZdYmw275m6i0SSs2a9O4x+Ckt0EGpO4PLx1Yz3ceh/hFjI+A73Mddtf5E26VfLsVQDd1Tkr1WRTHtkbs0JmaKGrHJt+LFDxvYXEmDLtsF8eUJWDc3iA0GUsixmJF/auaR4PVjtN4Sub6KOK0LE8+GO+Wh0W1rBuFqIuBwDm2uea62+AIKUj35/hW9CwWknQkt/urCMKu2wIoaVyBTrch9xQzOImxEoVg3UjUr1rX/uFKB361jgl6wZElRhNY1XU+lGnTx/FFyIXIu0FJrY9XN8MS8LmX5n8FM0H07XwOiciN51ea5gZ4GOgLcVqtbTP2Hm1QAzgw== X-Forefront-PRVS: 02622CEF0A X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(4630300001)(6009001)(39850400002)(39400400002)(39860400002)(39840400002)(39450400003)(39410400002)(93886004)(86362001)(575784001)(6486002)(42186005)(50986999)(6666003)(76176999)(229853002)(36756003)(7736002)(54356999)(6916009)(81166006)(33646002)(2950100002)(5660300001)(8676002)(305945005)(6116002)(53936002)(3846002)(4001350100001)(38730400002)(110136004)(25786009)(1411001)(6496005)(66066001)(47776003)(5003940100001)(6246003)(4326008)(54906002)(189998001)(83506001)(48376002)(50466002)(2906002); DIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR0701MB1883; H:elxa4wqvvz1; FPR:; SPF:None; MLV:sfv; LANG:en; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; HE1PR0701MB1883; 23:utctkzvdKJMcI4V7uY4WEhuhY/wHw0Qe8jb73cI?= =?us-ascii?Q?Ljp8qHqoMHQqgjnjQ5S9Toba2Bgs62bcvMfdRksg6cHVD172vjr7Is1iMbQn?= =?us-ascii?Q?REwE1Xps+eFByI7zlGND3ksKTjoHIPZOqbHySehUMwEYVASksdJTJKYjEJKF?= =?us-ascii?Q?iwnXyHCB8OY3CAJE93hZ8pIKPDuycwOTXONQoq/JVodY7FjHdIDsACr7+5N5?= =?us-ascii?Q?gKz9Z3KYRJnm3m2gU48VZFURwikngGkcW+RF0vTbT6mHAeqbekPpGLj+O0dN?= =?us-ascii?Q?GcYgZJdpqlTr6lXhMBDLH9K8pHZtfjTf6EE2SNXDwIkwQVm+1gHUqgmgzZoE?= =?us-ascii?Q?ccxFGd5NaDzYX6ykuUmY9wU9q1YmjjSP/jzoQAnCdGrSi0MFyrC61a4eir4p?= =?us-ascii?Q?urC8mDhLUGcvAt7LmLLIyaauG0E/QCw1RxOtEg54W8qqxOluAdVQ6O7C+QyJ?= =?us-ascii?Q?mVr/5NEdGpN8+7P4G2iTQsDPJYCtt1bxwB7HgsFmvzQlZUPamY5WCaUWLpQM?= =?us-ascii?Q?/8KSMdVVUB//r9dMkjelWX2c922x+smWDfkteMLYOwmtagFdY+XFB0SRpJ7T?= =?us-ascii?Q?czCz9MJqwtewJnpGubv+kVIeqARriDTE8E+hzsukZRI0zW0Gd6nJc/+HFl4V?= =?us-ascii?Q?e5Y3MzIhv9szlKA2IRvgjnjJ+rWE86ut06X581mrUWlflZLLcKVXnQ2sQitB?= =?us-ascii?Q?xScrou0XJVW3eB8+DfIAxNVjMmSt/c1D8QXiYxcaE8SxxsfTFdKz36SknVco?= =?us-ascii?Q?UmHKRAkB2tpfYIuNcGOt/ADL6HURnCZr3mM2YkFnFSdgVxcBmOmu8KN2ZEZe?= =?us-ascii?Q?Tm8E6n8ksdrcg+T++M3m15vqFvt51K3NUkE3YDsBx6T4fFRuWlFHYBpkzu3+?= =?us-ascii?Q?3GngKlGQaVTl+yrf/QZWdHs9W2T6nLvw6z8xUZrFkO7Lj1E1hssMB/4tPGxV?= =?us-ascii?Q?m4yokWwUltlUYBmGIrJJb7MGi5OTD2iRhER+rjxx6YplfctW29niHcbM5clQ?= =?us-ascii?Q?MCGXtTdLtLFDAgYNgQxDKPwn9B2KFHtc3sr1cV+gzqUBkV6cejnvXUy2OPT3?= =?us-ascii?Q?5XDfezW7nuJUMxrNWxZHCIo7nPtruhYgBvag5OeEKR9fHfejWMMUufKwI+QR?= =?us-ascii?Q?urGcl2jTBjLppvN2SfayzygO6bvY3g6ZcIP/B8lB65lLIgtBqmH01kLH7Qw9?= =?us-ascii?Q?ijdEYa5sovuwe72s=3D?= X-Microsoft-Exchange-Diagnostics: 1; HE1PR0701MB1883; 6:U34Xh2oMP7q2hWIcrlglKjjrFPHAJJiETqpao3uGc95Ug1E6KBSDGxzskDI9+LAOkZjHoKl2c5Dtrh4yXu1AvKwKBHgkytw38dMkfdhdQKobT0uvtZIDbrBZ+/+Ck0szdTKf4kinvK+GljgarrsqQTLK0E4aXTIGKFdPMnY0ZXzrLxXbVknmgnNSBS7Ioz9IBPPyYSPnermI8PX5d+AGOwp6FCJigiL8LIAvtF2WiofjmTFc3Vwehz4OCsv2PeeEKaaKPge23g23g2qgPxhWKZ+1+AlUp8I5n9w2EBH/GGDHTJaTR++FlK1YSiLVSm3ptCVWuvkthRIja1ATuVOqnhHtpVt5tQ/P6KTn0/tvMBuUrfVOnjDCdva/S0LYviKig6kfqMTaQjhnYVfjc+8fgQ==; 5:D0dT1DyhhPpa9TM8JpriNyKgMzuxmWb4Dl8Ffiq1jMslUoy0RVCskLYoQct4Mri+8G/xOqPhf85GkTbHIy4PgB0VLbl0gJy2po0qJG/YK/orD0Ms93aUX29sZ3RcNdVrSlM67Z0zFAZEQSCCmY0ZIwsNT7qnnmivjOY5G+UHGjY=; 24:zFxQSI41fwEwyrYsi8Lv/TzzcP0B4ptdmuwQMb7ccCMUnpHGi20+niQx1G9b2QgejRM+cvlm+PY05BzDVU9pR3KqD2PJbw30v2VXfPGsYPE= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; HE1PR0701MB1883; 7:V+L0XKGIvnZ68MWyJgMICcIZ6R7S8WIEpqic6y2YQdyOBeygMzY4n+07SPAW7jBbrAg+S/nHr2Jn+kUG1Y2v8FuBtW0ECl96HlGmHlJgFIhQoJBrEnK3aY5tUNgQqXr6u5Vkqr4mq+8apIemAQAqVoia7ijKuVoxxg1pAwREu05bbVZPNmMvWeKV186h7xobzFlJ6afxUjnaBjW4PZ1cqebtSA81ET8l30H8NR9UYpCH5dNgNUXP6trni265UdtB/Zx9YzGiwTj8hhfhUah5tZ23lcYpxNjH6d8oWvuBjYGWdklJPfatXvTdD5Vgv5WqUINVMGmBaI1dz4mypriPzA== X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Mar 2017 18:31:37.9108 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0701MB1883 X-OriginatorOrg: ericsson.com X-IsSubscribed: yes Yao Qi writes: > Antoine Tremblay writes: > >>> In software single step, we calculate the next pcs, and select >>> breakpoint kinds of them, according to current pc. If current >>> pc is not within IT block (!InITBlock ()) or the last instruction >>> in IT block (LastInITBlock ()), we can safely use 16-bit thumb >>> breakpoint for any thumb instruction. That is, in >>> gdbserver/linux-aarch32-low.c:arm_breakpoint_kind_from_current_state, >>> we can return ARM_BP_KIND_THUMB if (!InITBlock () || LastInITBlock ()). >> >> This is not entirely true since we have to check if the next pcs are in >> an IT block rather then only the current one, so there's multiple >> scenarios. > > In fact, we need to know whether the next pcs will be conditionally > executed or not, right? If they won't be conditionally executed, we can > use 16-bit thumb breakpoint instruction. By checking CPSR, if > current PC is not in IT block or on the last instruction in IT block, > the next pcs can't be conditionally executed. I've already had a patch > below to implement what I described. > You have to add if the current instruction is an IT instruction in wich case the next instruction will be in an IT block. Also if you have a conditional instruction that would evalutate to true and is not the last one, get_next_pcs may return an instruction after the IT block, arm_breakpoint_kind_from_current_state will be called from the IT block with that PC and return a THUMB2_KIND when it should not. See the else case in arm-get-next-pcs.c:~351 My point was that we should use get_next_pc directly since it's the best place to detect if the next_pc is in the IT block. And the intent would be clear. It would give something like the patch below. (Note the GDB part of this is missing but it works with GDBServer) > The problem of this patch is that we end up inserting different > kinds of breakpoints on the same instruction. For a given 32-bit thumb > instruction, GDB and GDBserver knows 32-bit thumb breakpoint instruction > is used for GDB breakpoint, but only GDBserver knows 16-bit thumb > breakpoint is used for GDBserver single-step breakpoint, so GDB will be > confused on this. I stopped here, and start to do something else. Humm but how will the GDBServer 16-bit breakpoint be reported to GDB ? Won't it always be hit and handled by GDBServer ? And if you have a GDB breakpoint on an instruction and GDBServer puts a single step breakpoint on that GDB breakpoint instruction, GDBServer still knows of the GDB and GDBServer breakpoint types. So how does GDB get confused ? ---- commit a18806af57b6c8906594ded036009c6b30c5b7db Author: Antoine Tremblay Date: Thu Mar 30 12:57:29 2017 -0400 getnextpc encodes the kind diff --git a/gdb/arch/arm-get-next-pcs.c b/gdb/arch/arm-get-next-pcs.c index 398dd59a..f9b5bf1 100644 --- a/gdb/arch/arm-get-next-pcs.c +++ b/gdb/arch/arm-get-next-pcs.c @@ -315,6 +315,7 @@ thumb_get_next_pcs_raw (struct arm_get_next_pcs *self) itstate = thumb_advance_itstate (itstate); } + pc = MAKE_THUMB2_BP_KIND (pc); VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc)); return next_pcs; } @@ -335,6 +336,7 @@ thumb_get_next_pcs_raw (struct arm_get_next_pcs *self) itstate = thumb_advance_itstate (itstate); } + pc = MAKE_THUMB2_BP_KIND (pc); VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc)); return next_pcs; } @@ -361,8 +363,13 @@ thumb_get_next_pcs_raw (struct arm_get_next_pcs *self) /* Set a breakpoint on the following instruction. */ gdb_assert ((itstate & 0x0f) != 0); + pc = MAKE_THUMB2_BP_KIND (pc); VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc)); + /* Reset the thumb2 kind bit to avoid affecting the next pc + value. */ + pc = UNMAKE_THUMB2_BP_KIND (pc); + cond_negated = (itstate >> 4) & 1; /* Skip all following instructions with the same @@ -378,6 +385,11 @@ thumb_get_next_pcs_raw (struct arm_get_next_pcs *self) } while (itstate != 0 && ((itstate >> 4) & 1) == cond_negated); + /* Only set a thumb2 breakpoint kind if we are still in an + IT block. */ + if (itstate != 0 && ((itstate >> 4) & 1) == cond_negated) + pc = MAKE_THUMB2_BP_KIND (pc); + VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc)); return next_pcs; diff --git a/gdb/arch/arm-linux.c b/gdb/arch/arm-linux.c index 943efe7..4b20f77 100644 --- a/gdb/arch/arm-linux.c +++ b/gdb/arch/arm-linux.c @@ -73,7 +73,7 @@ arm_linux_get_next_pcs_fixup (struct arm_get_next_pcs *self, step this instruction, this instruction isn't executed yet, and LR may not be updated yet. In other words, GDB can get the target address from LR if this instruction isn't BL or BLX. */ - if (nextpc > 0xffff0000) + if ((nextpc & 0xffffffff) > 0xffff0000) { int bl_blx_p = 0; CORE_ADDR pc = regcache_read_pc (self->regcache); diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h index a86dd37..b83fc79 100644 --- a/gdb/arch/arm.h +++ b/gdb/arch/arm.h @@ -101,6 +101,9 @@ enum arm_breakpoint_kinds #define IS_THUMB_ADDR(addr) ((addr) & 1) #define MAKE_THUMB_ADDR(addr) ((addr) | 1) #define UNMAKE_THUMB_ADDR(addr) ((addr) & ~1) +#define IS_THUMB2_BP_KIND(addr) (((addr) & 1ULL << 32) >> 32) +#define MAKE_THUMB2_BP_KIND(addr) ((addr) | (1ULL << 32)) +#define UNMAKE_THUMB2_BP_KIND(addr) ((addr) & ~(1ULL << 32)) /* Support routines for instruction parsing. */ #define submask(x) ((1L << ((x) + 1)) - 1) diff --git a/gdb/gdbserver/linux-aarch32-low.c b/gdb/gdbserver/linux-aarch32-low.c index 2b710ba..fc66028 100644 --- a/gdb/gdbserver/linux-aarch32-low.c +++ b/gdb/gdbserver/linux-aarch32-low.c @@ -242,10 +242,16 @@ arm_breakpoint_kind_from_pc (CORE_ADDR *pcptr) unsigned short inst1 = 0; target_read_memory (*pcptr, (gdb_byte *) &inst1, 2); - if (thumb_insn_size (inst1) == 4) - return ARM_BP_KIND_THUMB2; + if (thumb_insn_size (inst1) == 4 && IS_THUMB2_BP_KIND (*pcptr)) + { + *pcptr = UNMAKE_THUMB2_BP_KIND (*pcptr); + return ARM_BP_KIND_THUMB2; + } + + return ARM_BP_KIND_THUMB; } - return ARM_BP_KIND_THUMB; + else + return ARM_BP_KIND_THUMB; } else return ARM_BP_KIND_ARM;