From patchwork Mon Feb 9 14:51:09 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Machado X-Patchwork-Id: 4978 Received: (qmail 29754 invoked by alias); 9 Feb 2015 14:51:18 -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 29734 invoked by uid 89); 9 Feb 2015 14:51:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Feb 2015 14:51:16 +0000 Received: from svr-orw-fem-02x.mgc.mentorg.com ([147.34.96.206] helo=SVR-ORW-FEM-02.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1YKpg8-0001f7-Nv from Luis_Gustavo@mentor.com ; Mon, 09 Feb 2015 06:51:12 -0800 Received: from [172.30.4.6] (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.3.224.2; Mon, 9 Feb 2015 06:51:12 -0800 Message-ID: <54D8C95D.7000600@codesourcery.com> Date: Mon, 9 Feb 2015 12:51:09 -0200 From: Luis Machado Reply-To: User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Pedro Alves , "'gdb-patches@sourceware.org'" Subject: Re: [PATCH] Relax ARM prologue unwinder assumption References: <54D3BF52.5070709@codesourcery.com> <54D4DCE4.4060209@redhat.com> In-Reply-To: <54D4DCE4.4060209@redhat.com> X-IsSubscribed: yes On 02/06/2015 01:25 PM, Pedro Alves wrote: > On 02/05/2015 07:06 PM, Luis Machado wrote: >> Just recently i ran into a problem with a bare metal ARM target where >> GDB would not allow some registers to be changed after the PC was >> manually set to some other value. >> >> In reality the target had just started and one of its cores, out of >> four, was running the program, but the other ones were in some random state. >> >> The PC of one of the other 3 cores was then adjusted to point to a known >> function address. >> >> GDB's reaction to this change is to invalidate the regcache and frame >> and build a brand new chain and cache, while trying to retain the >> previously selected frame - #0 in this case. >> >> What i noticed is that GDB was selecting frame #1 instead of #0 due to >> unfortunate coincidences with both frames' SP's being 0. And we can't >> modify some registers on non-innermost frames for obvious reasons. >> >> Here's a brief log of what happens: >> >> >> [Switching to thread 2 (Thread 2)] >> #0 0x0093ff10 in ?? () >> (gdb) set $pc=functioncore2 >> (gdb) bt >> #0 functioncore2 () at test.c:32 >> #1 0x0000fc44 in ?? () > > So in this case, frame #0's unwinder must now be the dwarf > unwinder, right? > Correct. >> The attached patch removes this restriction and does not cause any >> regressions for ARM bare metal, but i'd like feedback. > > Nowadays we have the frame_unwind_stop_reason hook to make it possible > to have different outermost frame ids, but still have the frame claim > that its the outermost, instead of forcing all outermost frame ids > use the outer_frame_id id. See i386_frame_unwind_stop_reason. > > Sounds like ARM could add an arm_frame_unwind_stop_reason that > returns UNWIND_OUTERMOST when prev_sp is 0. > > And it looks like this bit here: > > /* This is meant to halt the backtrace at "_start". */ > pc = get_frame_pc (this_frame); > if (pc <= gdbarch_tdep (get_frame_arch (this_frame))->lowest_pc) > return; > > can well cause a fail in the same way. > Thanks. That mechanism does sound a bit more flexible and it indeed solves the problem i'm seeing. How does the updated patch look? I have arm_prologue_unwind_stop_reason implemented now. > (It may also make sense to think through the cases where we are > trying to restore the selected frame, and change that code. It may be > that it never makes sense to go from frame #0 to any other frame > number, for instance.) Trying to come up with the best match for a certain frame that potentially does not exist anymore can be troublesome, but it's been working well so far. I'd go for always picking up frame 0 as well. It is easier, cleaner and less prone to failures. Luis 2015-02-09 Luis Machado * arm-tdep.c (arm_prologue_unwind_stop_reason): New function. (arm_prologue_this_id): Move PC and SP limit checks to arm_prologue_unwind_stop_reason. (arm_prologue_unwind) : Set to arm_prologue_unwind_stop_reason. diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 8e9552a..f3a6325 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -2021,6 +2021,31 @@ arm_make_prologue_cache (struct frame_info *this_frame) return cache; } +/* Implementation of the stop_reason hook for arm_prologue frames. */ + +static enum unwind_stop_reason +arm_prologue_unwind_stop_reason (struct frame_info *this_frame, + void **this_cache) +{ + struct arm_prologue_cache *cache; + CORE_ADDR pc; + + if (*this_cache == NULL) + *this_cache = arm_make_prologue_cache (this_frame); + cache = *this_cache; + + /* This is meant to halt the backtrace at "_start". */ + pc = get_frame_pc (this_frame); + if (pc <= gdbarch_tdep (get_frame_arch (this_frame))->lowest_pc) + return UNWIND_OUTERMOST; + + /* If we've hit a wall, stop. */ + if (cache->prev_sp == 0) + return UNWIND_OUTERMOST; + + return UNWIND_NO_REASON; +} + /* Our frame ID for a normal frame is the current function's starting PC and the caller's SP when we were called. */ @@ -2037,18 +2062,10 @@ arm_prologue_this_id (struct frame_info *this_frame, *this_cache = arm_make_prologue_cache (this_frame); cache = *this_cache; - /* This is meant to halt the backtrace at "_start". */ - pc = get_frame_pc (this_frame); - if (pc <= gdbarch_tdep (get_frame_arch (this_frame))->lowest_pc) - return; - - /* If we've hit a wall, stop. */ - if (cache->prev_sp == 0) - return; - /* Use function start address as part of the frame ID. If we cannot identify the start address (due to missing symbol information), fall back to just using the current PC. */ + pc = get_frame_pc (this_frame); func = get_frame_func (this_frame); if (!func) func = pc; @@ -2117,7 +2134,7 @@ arm_prologue_prev_register (struct frame_info *this_frame, struct frame_unwind arm_prologue_unwind = { NORMAL_FRAME, - default_frame_unwind_stop_reason, + arm_prologue_unwind_stop_reason, arm_prologue_this_id, arm_prologue_prev_register, NULL,