From patchwork Fri Oct 24 12:23:21 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 3356 Received: (qmail 21926 invoked by alias); 24 Oct 2014 12:23:27 -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 21912 invoked by uid 89); 24 Oct 2014 12:23:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL, BAYES_00, KAM_STOCKGEN autolearn=no 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; Fri, 24 Oct 2014 12:23:26 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 1888111637E; Fri, 24 Oct 2014 08:23:24 -0400 (EDT) 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 R-T6lpnvN+qL; Fri, 24 Oct 2014 08:23:24 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id DDE9F116380; Fri, 24 Oct 2014 08:23:23 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 5557440F9C; Fri, 24 Oct 2014 05:23:21 -0700 (PDT) Date: Fri, 24 Oct 2014 05:23:21 -0700 From: Joel Brobecker To: Yao Qi Cc: gdb-patches@sourceware.org Subject: Re: [RFA] ARM: stricter __stack_chk_guard check during prologue Message-ID: <20141024122321.GA5318@adacore.com> References: <20141022142231.GF4786@adacore.com> <87y4s7h553.fsf@codesourcery.com> <20141023153947.GA11707@adacore.com> <87oat1x4bj.fsf@codesourcery.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87oat1x4bj.fsf@codesourcery.com> User-Agent: Mutt/1.5.21 (2010-09-15) > I run regression tests on arm-linux-gnueabi with your patch. There > are some fails on armv4t arm and thumb mode. It is an existing bug > introduced by my patch :( and your patch just exposed it. I'll fix it > separately. Thanks, Yao. > > /* If name of symbol doesn't start with '__stack_chk_guard', this > > instruction sequence is not for stack protector. If symbol is > > removed, we conservatively think this sequence is for stack protector. */ > > We need to update the comment to sync with the code below. Indeed! > > - if (stack_chk_guard.minsym > > - && strncmp (MSYMBOL_LINKAGE_NAME (stack_chk_guard.minsym), > > + if (stack_chk_guard.minsym == NULL > > + || strncmp (MSYMBOL_LINKAGE_NAME (stack_chk_guard.minsym), > > "__stack_chk_guard", > > strlen ("__stack_chk_guard")) != 0) > > Otherwise, it looks good to me. Thanks! Attached is a revised patch. I will push it after you fix the latent bug you noticed - just let me know when? From fae3d8ff9fc547d879aa64b7d3f89ed7323a7e1d Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Thu, 23 Oct 2014 08:25:20 -0700 Subject: [PATCH] ARM: stricter __stack_chk_guard check during prologue analysis We are trying to insert a breakpoint on line 4 for the following Ada code. 3 procedure STR is 4 XX : String (1 .. Blocks.Sz) := (others => 'X'); -- STOP 5 K : Integer; 6 begin 7 K := 13; The code generated on ARM (-march=armv7-m) starts like this: (gdb) disass str'address Dump of assembler code for function _ada_str: --# Line str.adb:3 0x08000014 <+0>: push {r4, r7, lr} 0x08000016 <+2>: sub sp, #28 0x08000018 <+4>: add r7, sp, #0 0x0800001a <+6>: mov r3, sp 0x0800001c <+8>: mov r4, r3 --# Line str.adb:4 0x0800001e <+10>: ldr r3, [pc, #84] ; (0x8000074 <_ada_str+96>) 0x08000020 <+12>: ldr r3, [r3, #0] 0x08000022 <+14>: str r3, [r7, #20] 0x08000024 <+16>: ldr r3, [r7, #20] [...] When computing the address related to str.adb:4, GDB correctly resolves it to 0x0800001e first, but then considers the next 3 instructions as being part of the prologue because it thinks they are part of stack-protector code. As a result, instead of inserting the breakpoint at line 4, it skips those instruction and consequently the rest of the instructions until the next line start, which his line 7. The stack-protector code is expected to start like this... ldr Rn, .Label .... .Lable: .word __stack_chk_guard ... but the implementation actually accepts a sequence where the ldr location points to an address for which there is no symbol. It only aborts if the address points to a symbol which is not __stack_chk_guard. Since the __stack_chk_guard symbol is always expected to exist when used (it lives in .dynsym), this patch fixes the issue by requiring that the ldr gets the address of the __stack_chk_guard symbol. If the address could not be resolved, then it rejects the sequence as being stack-protector code. gdb/ChangeLog: arm-tdep.c (arm_skip_stack_protector): Return early if address loaded by first "ldr" instruction does not have a corresponding minimal symbol. Update comment. Tested on arm-eabi using AdaCore's testsuite. --- gdb/arm-tdep.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index e2559ec..172e54f 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -1306,11 +1306,10 @@ arm_skip_stack_protector(CORE_ADDR pc, struct gdbarch *gdbarch) return pc; stack_chk_guard = lookup_minimal_symbol_by_pc (addr); - /* If name of symbol doesn't start with '__stack_chk_guard', this - instruction sequence is not for stack protector. If symbol is - removed, we conservatively think this sequence is for stack protector. */ - if (stack_chk_guard.minsym - && strncmp (MSYMBOL_LINKAGE_NAME (stack_chk_guard.minsym), + /* ADDR must correspond to a symbol whose name is __stack_chk_guard. + Otherwise, this sequence cannot be for stack protector. */ + if (stack_chk_guard.minsym == NULL + || strncmp (MSYMBOL_LINKAGE_NAME (stack_chk_guard.minsym), "__stack_chk_guard", strlen ("__stack_chk_guard")) != 0) return pc; -- 1.9.1