From patchwork Mon Nov 9 18:08:33 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 9617 Received: (qmail 15447 invoked by alias); 9 Nov 2015 18:08:43 -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 15436 invoked by uid 89); 9 Nov 2015 18:08:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 09 Nov 2015 18:08:41 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id A4F5611630F for ; Mon, 9 Nov 2015 13:08:39 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id gWi9FJGeNVFm for ; Mon, 9 Nov 2015 13:08:39 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 72D141162E0 for ; Mon, 9 Nov 2015 13:08:39 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 0FF30405BA; Mon, 9 Nov 2015 10:08:38 -0800 (PST) From: Joel Brobecker To: gdb-patches@sourceware.org Subject: RFA: [ARM] "svc" insn check at irrelevant address in ARM unwind info sniffer Date: Mon, 9 Nov 2015 10:08:33 -0800 Message-Id: <1447092513-20690-1-git-send-email-brobecker@adacore.com> Hello, The following issue has been observed on arm-android, trying to step over the following line of code: Put_Line (">>> " & Integer'Image (Message (I))); Below is a copy of the GDB transcript: (gdb) cont Breakpoint 1, q.dump (message=...) at q.adb:11 11 Put_Line (">>> " & Integer'Image (Message (I))); (gdb) next 0x00016000 in system.concat_2.str_concat_2 () The expected behavior for the "next" command is to step over the call to Put_Line and stop at line 12: (gdb) next 12 I := I + 1; What happens during the next step is that the code for line 11 above make a call to system.concat_2.str_concat_2 (to implement the '&' string concatenation operator) before making the call to Put_Line. While stepping, GDB stops eventually stops at the first instruction of that function, and fails to detect that it's a function call from where we were before, and so decides to stop stepping. And the reason why it fails to detect that we landed inside a function call is because it fails to unwind from that function: (gdb) bt #0 0x00016000 in system.concat_2.str_concat_2 () #1 0x0001bc74 in ?? () Debugging GDB, I found that GDB decides to use the ARM unwind info for that function, which contains the following data: 0x16000 : 0x80acb0b0 Compact model index: 0 0xac pop {r4, r5, r6, r7, r8, r14} 0xb0 finish 0xb0 finish But, in fact, using that data is wrong, in this case, because it mentions a pop of 6 registers, and therefore hints at a frame size of 24 bytes. The problem is that, because we're at the first instruction of the function, the 6 registers haven't been pushed to the stack yet. In other words, using the ARM unwind entry above, GDB is tricked into thinking that the frame size is 24 bytes, and that the return address (r14) is available on the stack. One visible manifestation of this issue can been seen by looking at the value of the stack pointer, and the frame's base address: (gdb) p /x $sp $2 = 0xbee427b0 (gdb) info frame Stack level 0, frame at 0xbee427c8: ^^^^^^^^^^ |||||||||| The frame's base address should be equal to the value of the stack pointer at entry. And you eventually get the correct frame address, as well as the correct backtrace if you just single-step one additional instruction, past the push: (gdb) x /i $pc => 0x16000 : push {r4, r5, r6, r7, r8, lr} (gdb) stepi (gdb) bt #0 0x00016004 in system.concat_2.str_concat_2 () #1 0x00012b6c in q.dump (message=...) at q.adb:11 #2 0x00012c3c in q () at q.adb:19 Digging further, I found that GDB tries to use the ARM unwind info only when sure that it is relevant, as explained in the following comment: /* The ARM exception table does not describe unwind information for arbitrary PC values, but is guaranteed to be correct only at call sites. We have to decide here whether we want to use ARM exception table information for this frame, or fall back [...] There is one case where it decides that the info is relevant, described in the following comment: /* We also assume exception information is valid if we're currently blocked in a system call. The system library is supposed to ensure this, so that e.g. pthread cancellation works. For that, it just parses the instruction at the address it believes to be the point of call, and matches it against an "svc" instruction. For instance, for a non-thumb instruction, it is at... get_frame_pc (this_frame) - 4 ... and the code checking looks like the following. if (safe_read_memory_integer (get_frame_pc (this_frame) - 4, 4, byte_order_for_code, &insn) && (insn & 0x0f000000) == 0x0f000000 /* svc */) exc_valid = 1; However, the reason why this doesn't work in our case is that because we are at the first instruction of a function in the innermost frame. That frame can't possibly be making a call, and therefore be stuck on a system call. What the code above ends up doing is checking the instruction just before the start of our function, which in our case is not even an actual instruction, but unlucky for us, happens to match the pattern it is looking for, thus leading GDB to improperly trust the ARM unwinding data. gdb/ChangeLog: * arm-tdep.c (arm_exidx_unwind_sniffer): Do not check for a frame stuck on a system call if the given frame is the innermost frame. Tested on arm-android, as well as arm-linux. OK to commit? Thanks! diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 308d484..9296424 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -2814,26 +2814,37 @@ arm_exidx_unwind_sniffer (const struct frame_unwind *self, /* We also assume exception information is valid if we're currently blocked in a system call. The system library is supposed to - ensure this, so that e.g. pthread cancellation works. */ - if (arm_frame_is_thumb (this_frame)) - { - LONGEST insn; + ensure this, so that e.g. pthread cancellation works. - if (safe_read_memory_integer (get_frame_pc (this_frame) - 2, 2, - byte_order_for_code, &insn) - && (insn & 0xff00) == 0xdf00 /* svc */) - exc_valid = 1; - } - else + But before verifying the instruction at the point of call, make + sure this_frame is actually making a call (or, said differently, + that it is not the innermost frame). For that, we compare + this_frame's PC vs this_frame's addr_in_block. If equal, it means + there is no call (otherwise, the PC would be the return address, + which is the instruction after the call). */ + + if (get_frame_pc (this_frame) != addr_in_block) { - LONGEST insn; + if (arm_frame_is_thumb (this_frame)) + { + LONGEST insn; - if (safe_read_memory_integer (get_frame_pc (this_frame) - 4, 4, - byte_order_for_code, &insn) - && (insn & 0x0f000000) == 0x0f000000 /* svc */) - exc_valid = 1; + if (safe_read_memory_integer (get_frame_pc (this_frame) - 2, 2, + byte_order_for_code, &insn) + && (insn & 0xff00) == 0xdf00 /* svc */) + exc_valid = 1; + } + else + { + LONGEST insn; + + if (safe_read_memory_integer (get_frame_pc (this_frame) - 4, 4, + byte_order_for_code, &insn) + && (insn & 0x0f000000) == 0x0f000000 /* svc */) + exc_valid = 1; + } } - + /* Bail out if we don't know that exception information is valid. */ if (!exc_valid) return 0;