From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27262 invoked by alias); 29 Sep 2009 00:16:55 -0000 Received: (qmail 27248 invoked by uid 22791); 29 Sep 2009 00:16:53 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 29 Sep 2009 00:16:49 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 475ED2BAB56; Mon, 28 Sep 2009 20:16:47 -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 hFbaxagET0PV; Mon, 28 Sep 2009 20:16:47 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 0DE5A2BAB4D; Mon, 28 Sep 2009 20:16:47 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 151EBF593D; Mon, 28 Sep 2009 17:16:41 -0700 (PDT) Date: Tue, 29 Sep 2009 00:16:00 -0000 From: Joel Brobecker To: Jan Kratochvil Cc: gdb-patches@sourceware.org Subject: Re: [RFA/commit] ia64: incorrect breakpoint save/restore with L-type instruction at slot 1 Message-ID: <20090929001641.GH9003@adacore.com> References: <20090925204835.GF5077@adacore.com> <20090926215709.GA26221@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090926215709.GA26221@host0.dyn.jankratochvil.net> User-Agent: Mutt/1.5.18 (2008-05-17) 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 X-SW-Source: 2009-09/txt/msg00892.txt.bz2 > b *(0x4000000000000691 + 1) > Breakpoint 5 at 0x4000000000000692: file ./gdb.base/breakpoint-shadow.c, line 28. > ia64-tdep.c:672: internal-error: Address 0x4000000000000692 already contains a breakpoint. > + > 0x4000000000000690 : [MLX] st8.rel [r2]=r14 > 0x4000000000000691 : movl r14=0x7fffffff;; > -> > 0x4000000000000690 : [MLX] data8 0x1fe8dc021c0 > 0x4000000000000691 : data8 0x0cfffefe3 Yeah, good catch. The only comment I had on the patch is that it doesn't error out while trying to insert the breakpoint in the normal case where "breakpoint always-inserted" is off. Instead, it errors out while trying to resume the execution of the inferior, and the breakpoint prevents us from continuing until the breakpoint is either removed or deleted. I don't think we really have any mechanism in place at the moment that would allow us to catch the problem early. But your patch already improves the situation. So I checked it in both HEAD and branch (I want to add some comments as discussed on IRC). Given that this only occurs when either: debugging info points to the wrong address, or when the user specifically indicates slot 2 of an L+X instruction as the address of the breakpoint, I consider this as a non-problem. > gdb/testsuite/ > 2009-09-26 Jan Kratochvil > > * gdb.base/breakpoint-shadow.c (main): Change `int i' type to `long l'. > Use wider value for the second initialization. > * gdb.base/breakpoint-shadow.exp : Fix TCL error on missing > bpt2address value. Require now $bpt2address to be at slot 1. Move > "Third breakpoint" from slot 2 to slot 0. New test `Slot X breakpoint > refusal'. You found an astute solution to provoking the situation. However, even if the logical way of storing the new value for our long is to use an L+X instruction on ia64, I am wondering if we wouldn't be better off building our executable from assembly instead of from source, the way we do in gdb.arch. That way, we would know for sure in which slot each breakpoint is inserted and what type of instruction we have underneath. We can also make sure we test all sort of combinations. I wanted to write a testcase when I initially submitted my own patch, but I couldn't work from my Ada example, because Ada programs usually require the Ada Runtime. What do you think? I've held the testsuite patch for now, because the comment in breakpoint-shadow.c is a little confusing, and the following comment in breakpoint-shadow.exp becomes a little outdated: # Unoptimized code should not use the 3rd slot for the first instruction of # a source line. This is important for our test, because we want both # breakpoints ("Second breakpoint" and the following one) to be in the same # bundle. If we go for the gdb.arch approach, we can then remove the who section specific to ia64 from breakpoint-shadow.exp. -- Joel