From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2812 invoked by alias); 8 Sep 2009 17:44:58 -0000 Received: (qmail 2705 invoked by uid 22791); 8 Sep 2009 17:44:54 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 08 Sep 2009 17:44:45 +0000 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n88Hi2AO016890; Tue, 8 Sep 2009 13:44:02 -0400 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx03.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id n88Hi086022902 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 8 Sep 2009 13:44:01 -0400 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.3) with ESMTP id n88HhxQm006927; Tue, 8 Sep 2009 19:43:59 +0200 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.3/Submit) id n88Hhx3G006926; Tue, 8 Sep 2009 19:43:59 +0200 Date: Tue, 08 Sep 2009 17:44:00 -0000 From: Jan Kratochvil To: Joel Brobecker Cc: gdb-patches@sourceware.org Subject: Re: [patch] [ia64] Fix (#2) shadowing of breakpoints [testcase fixup] Message-ID: <20090908174359.GB3223@host0.dyn.jankratochvil.net> References: <20090905185856.GA16389@host0.dyn.jankratochvil.net> <20090907041040.GA25651@host0.dyn.jankratochvil.net> <20090907181750.GF30677@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090907181750.GF30677@adacore.com> User-Agent: Mutt/1.5.19 (2009-01-05) X-IsSubscribed: yes 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/msg00205.txt.bz2 On Mon, 07 Sep 2009 20:17:50 +0200, Joel Brobecker wrote: > Patch is approved. Checked-in. > This is where I think it would be worth explaining that we are *re* > reading the bundle except that, this time, we are reading it in order OK, I agree, thanks, updated. > > +if [istarget "ia64-*-*"] then { > > + # 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 > (break-first and break-second) to be in the same bundle. In fact there are two breakpoints on non-ia64 and three breakpoints on ia64. I do not remember much why there were two breakpoints already before, maybe they were intended to be in the same bundle (which they are never in). Comment on their reason is missing there (from me). (+In real there are three/four breakpoints - the first one is from runto_main.) Thanks, Jan http://sourceware.org/ml/gdb-cvs/2009-09/msg00032.html --- src/gdb/ChangeLog 2009/09/08 00:50:42 1.10848 +++ src/gdb/ChangeLog 2009/09/08 17:39:19 1.10849 @@ -1,3 +1,11 @@ +2009-09-08 Jan Kratochvil + + Fix ia64 shadowing of breakpoints in multiple slots of a single bundle. + * ia64-tdep.c (ia64_memory_insert_breakpoint): New call + of make_show_memory_breakpoints_cleanup with parameter 0. Move the + reading of SHADOW_CONTENTS to this memory state point of code. Update + comment for the memory re-read. + 2009-09-07 Michael Snyder * record.c: Minor comment and white space fix-ups. --- src/gdb/ia64-tdep.c 2009/08/25 14:06:47 1.196 +++ src/gdb/ia64-tdep.c 2009/09/08 17:39:21 1.197 @@ -622,27 +622,37 @@ addr &= ~0x0f; - /* Disable the automatic memory restoration from breakpoints while - we read our instruction bundle. Otherwise, the general restoration - mechanism kicks in and we would possibly remove parts of the adjacent + /* Enable the automatic memory restoration from breakpoints while + we read our instruction bundle for the purpose of SHADOW_CONTENTS. + Otherwise, we could possibly store into the shadow parts of the adjacent placed breakpoints. It is due to our SHADOW_CONTENTS overlapping the real breakpoint instruction bits region. */ - cleanup = make_show_memory_breakpoints_cleanup (1); + cleanup = make_show_memory_breakpoints_cleanup (0); val = target_read_memory (addr, bundle, BUNDLE_LEN); - /* Check for L type instruction in slot 1, if present then bump up the slot - number to the slot 2. */ - template = extract_bit_field (bundle, 0, 5); - if (slotnum == 1 && template_encoding_table[template][slotnum] == L) - slotnum = 2; - /* Slot number 2 may skip at most 2 bytes at the beginning. */ - bp_tgt->placed_size = bp_tgt->shadow_len = BUNDLE_LEN - 2; + bp_tgt->shadow_len = BUNDLE_LEN - 2; /* Store the whole bundle, except for the initial skipped bytes by the slot number interpreted as bytes offset in PLACED_ADDRESS. */ memcpy (bp_tgt->shadow_contents, bundle + slotnum, bp_tgt->shadow_len); + /* Re-read the same bundle as above except that, this time, read it in order + to compute the new bundle inside which we will be inserting the + breakpoint. Therefore, disable the automatic memory restoration from + breakpoints while we read our instruction bundle. Otherwise, the general + restoration mechanism kicks in and we would possibly remove parts of the + adjacent placed breakpoints. It is due to our SHADOW_CONTENTS overlapping + the real breakpoint instruction bits region. */ + make_show_memory_breakpoints_cleanup (1); + val |= target_read_memory (addr, bundle, BUNDLE_LEN); + + /* Check for L type instruction in slot 1, if present then bump up the slot + number to the slot 2. */ + template = extract_bit_field (bundle, 0, 5); + if (slotnum == 1 && template_encoding_table[template][slotnum] == L) + slotnum = 2; + /* Breakpoints already present in the code will get deteacted and not get reinserted by bp_loc_is_permanent. Multiple breakpoints at the same location cannot induce the internal error as they are optimized into @@ -654,6 +664,8 @@ paddress (gdbarch, bp_tgt->placed_address)); replace_slotN_contents (bundle, IA64_BREAKPOINT, slotnum); + bp_tgt->placed_size = bp_tgt->shadow_len; + if (val == 0) val = target_write_memory (addr + slotnum, bundle + slotnum, bp_tgt->shadow_len); --- src/gdb/testsuite/ChangeLog 2009/09/03 22:03:20 1.1952 +++ src/gdb/testsuite/ChangeLog 2009/09/08 17:39:21 1.1953 @@ -1,3 +1,10 @@ +2009-09-08 Jan Kratochvil + + * gdb.base/breakpoint-shadow.exp (Second breakpoint placed): Initialize + $bpt2address. + (Second breakpoint address is valid on ia64) + (Third breakpoint on ia64 in the Second breakpoint's bundle): New. + 2009-09-03 Joseph Myers * gdb.base/ending-run.exp: Restrict regular expression matching --- src/gdb/testsuite/gdb.base/breakpoint-shadow.exp 2009/01/03 05:58:03 1.2 +++ src/gdb/testsuite/gdb.base/breakpoint-shadow.exp 2009/09/08 17:39:22 1.3 @@ -48,7 +48,29 @@ } gdb_test "b [gdb_get_line_number "break-first"]" "Breakpoint \[0-9\] at .*" "First breakpoint placed" -gdb_test "b [gdb_get_line_number "break-second"]" "Breakpoint \[0-9\] at .*" "Second breakpoint placed" +set test "Second breakpoint placed" +gdb_test_multiple "b [gdb_get_line_number "break-second"]" $test { + -re "Breakpoint \[0-9\] at (0x\[0-9a-f\]*):.*" { + pass $test + set bpt2address $expect_out(1,string) + } +} + +if [istarget "ia64-*-*"] then { + # 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. + + set test "Second breakpoint address is valid on ia64" + if [string match "*\[01\]" $bpt2address] { + pass $test + + gdb_test "b *($bpt2address + 1)" "Breakpoint \[0-9\] at .*" "Third breakpoint on ia64 in the Second breakpoint's bundle" + } else { + unresolved $test + } +} set test "disassembly with breakpoints" gdb_test_multiple "disass main" $test {