From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 574 invoked by alias); 4 Sep 2014 17:42:45 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 562 invoked by uid 89); 4 Sep 2014 17:42:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-yh0-f74.google.com Received: from mail-yh0-f74.google.com (HELO mail-yh0-f74.google.com) (209.85.213.74) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 04 Sep 2014 17:42:43 +0000 Received: by mail-yh0-f74.google.com with SMTP id 29so1184245yhl.3 for ; Thu, 04 Sep 2014 10:42:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:mime-version:content-type :content-transfer-encoding:message-id:date:to:cc:subject:in-reply-to :references; bh=XEJ/Is1JzdEy/k2FKOGQqp9VzEz4vU1fO4H+ud5UYZg=; b=R+NMGxn8HlMN8wEq7IDUTaH2RRnAYVRq3bdsJg4IuA+cwH67aOVwri/lGkntJi3eqC VPBvNJBMII5SKSB1nqnHpg//JPsdapjX9StsVUTWZls/9rObEx42B3GkUG7VVSxkk6w9 6/7K/z7+I24rayagtayXhf9VAcpPJ497TyiZqJE59+JzX6jC4C7XdZzUpT0wxGQMxMBe Ze/w02cscyrEaLs5y0DDNgzOHfqS9tcdcghxMFkQRAgmeBmCgWmhyis8hpL5raqT0zc0 0LYRAjnfR0Z9ZvlssQDB/Nuyo5r5xB9p12wiMS4jfIjGOt9mNtE3FC2BoIxI9dVfBtjp vl7w== X-Gm-Message-State: ALoCoQn02+CSsaayB625XoDPdzjrO5zw+p9njwR8gGFGsgCutYsencNcFYrHozXKnnst/Xu8hdwz X-Received: by 10.236.19.52 with SMTP id m40mr3074174yhm.35.1409852561355; Thu, 04 Sep 2014 10:42:41 -0700 (PDT) Received: from corp2gmr1-1.hot.corp.google.com (corp2gmr1-1.hot.corp.google.com [172.24.189.92]) by gmr-mx.google.com with ESMTPS id h24si1788401yhb.6.2014.09.04.10.42.41 for (version=TLSv1.1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 04 Sep 2014 10:42:41 -0700 (PDT) Received: from ruffy2.mtv.corp.google.com (ruffy2.mtv.corp.google.com [172.17.128.107]) by corp2gmr1-1.hot.corp.google.com (Postfix) with ESMTP id 78F1331C31D; Thu, 4 Sep 2014 10:42:40 -0700 (PDT) From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <21512.42127.940240.832062@ruffy2.mtv.corp.google.com> Date: Thu, 04 Sep 2014 17:42:00 -0000 To: Yao Qi Cc: , , Subject: Re: [PATCH] Remove some obfuscation from ${arch}_skip_prologue functions In-Reply-To: <87ha0ndaub.fsf@codesourcery.com> References: <87ha0ndaub.fsf@codesourcery.com> X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg00125.txt.bz2 Yao Qi writes: > Doug Evans writes: > > > 1) There's no need to call find_pc_partial_function before > > calling skip_prologue_using_sal: The first thing skip_prologue_using_sal > > does is call find_pc_partial_function! > > Nowadays we have: > > if (find_pc_partial_function (pc, NULL, &func_addr, NULL)) > { > CORE_ADDR post_prologue_pc > = skip_prologue_using_sal (gdbarch, func_addr); > > if (post_prologue_pc != 0) > return max (pc, post_prologue_pc); > } > > so your statement is valid if PC equals to FUNC_ADDR. There are other reasons to worry about cases when PC != FUNC_ADDR (which is why I'm still trying to find examples of when that is true), but I don't see how this is one of them. Remember, the first thing skip_prologue_using_sal does is also call find_pc_partial_function. So what we have is: ${arch}_skip_prologue calls find_pc_partial_function (PC) and gets back a start address, called FUNC_ADDR here. Then, skip_prologue_using_sal calls find_pc_partial_function (FUNC_ADDR) and gets back a start address, called START_PC. Then, skip_prologue uses START_PC for the rest of the function. So, under what circumstances is (figuratively speaking): (find_pc_partial_function (pc) != find_pc_partial_function (find_pc_partial_function (pc))) And if there is such a case, I think we've got another problem ... btw, note that arm_skip_prologue dropped the max (pc, post_prologue_pc) check. It may have been accidental. I found the relevant commit and emails, it's not clear yet why the check was dropped. > I don't have a > case that PC and FUNC_ADDR are different, but I'd like to add an assert > to check this, in each target's implementation of skip_prologue hook, or > in the callers of gdbarch_skip_prologue, something like: > > if (find_pc_partial_function (pc, NULL, &func_addr, NULL)) > gdb_assert (pc == func_addr); I have found two cases where, I think, ${arch}_skip_prologue can be called with pc != func_addr: vax and ppc. I'd be happy with simplifying the target API so that we could have such an assert, though I'd rather not put it in ${arch}_skip_prologue. I currently have the problem that the treatment of gcc vs clang is not as consistent as it could be across all ${arch}_skip_prologue functions, so I'm on a path of keeping as much code that should be common out of target-specific routines: cleaning it up later is not always fun or easy. > Note that this assert is triggered on arm in > gdb.cp/re-set-overloaded.exp, that is PC is [1] but FUNC_ADDR is [2]. > > (gdb) disassemble _ZN1CC1Ei > Dump of assembler code for function _ZN1CC1Ev: > 0x0000090c <+0>: ldr r12, [pc, #4] ; 0x918 <_ZN1CC1Ev+12> <- [2] > 0x00000910 <+4>: add r12, r12, pc > 0x00000914 <+8>: bx r12 > 0x00000918 <+12>: ; instruction: 0xffffffc5 > 0x0000091c <+0>: ldr r12, [pc, #4] ; 0x928 <_ZN1CC1Ei+12> <- [1] > 0x00000920 <+4>: add r12, r12, pc > > AFAICS, PC is still the function address but find_pc_partial_function > computes the FUNC_ADDR incorrectly and it is nothing wrong about your > patch. Thanks, this is good data. I did a similar experiment on amd64-linux after writing https://sourceware.org/ml/gdb-patches/2014-08/msg00539.html and got no hits. I'd be curious to see how arm_skip_prologue handles this. What flavor of arm and what version of gcc? I can't recreate that example with arm-linux-gnueabi-g++-4.7. Also, can you send me gdb.log plus re-set-overloaded{,.so} ? [don't cc the list :-)] > > > nios2: yao@codesourcery.com > > I tested your patch on nios2-linux, and no regression is found. > > > tic6x:yao@codesourcery.com > > My c6x board is dead in data center, so I can't test this patch for it. Thanks!