[SCM] archer-jankratochvil-misc: http://sourceware.org/ml/gdb-patches/2008-11/msg00260.html

[SCM] archer-jankratochvil-misc: http://sourceware.org/ml/gdb-patches/2008-11/msg00260.html

[jkratoch]

The branch, archer-jankratochvil-misc has been updated
       via  6960310bd606803281f65354c8265127cab076ad (commit)
       via  e99a71f9d675fabf196bd24f526cfaf9d14babee (commit)
      from  b4584fb7df1e4142b62d2cba5fa51181d32f29a4 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email.

- Log -----------------------------------------------------------------
commit 6960310bd606803281f65354c8265127cab076ad
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date:   Thu Nov 13 14:07:09 2008 +0100

    http://sourceware.org/ml/gdb-patches/2008-11/msg00260.html
    
    Hi,
    
    Nemiver could not parse the MI2 output.  The list of commands is a `list', not
    a `tuple'.
    
    Regards,
    Jan
    
    2008-11-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
    
    	* breakpoint.c (print_one_breakpoint_location): "script" is a list.

commit e99a71f9d675fabf196bd24f526cfaf9d14babee
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date:   Tue Oct 28 20:09:55 2008 +0100

    Fix automatic restoration of breakpoints memory for ia64.
    Red Hat private: https://bugzilla.redhat.com/show_bug.cgi?id=467502
    ------------------------------------------------------------------------------
    http://sourceware.org/ml/gdb-patches/2008-10/msg00678.html
    
    Hi,
    
    fix a corruption of ia64 `disass' with breakpoints placed in the target.
    
    It is another part of the Joel Brobecker's ia64-specific breakpoints fix:
    [commit] SIGILL in ld.so when running program from GDB on ia64-linux
    http://sourceware.org/ml/gdb-patches/2008-04/msg00674.html
    
    Currently ia64-tdep.c considers SHADOW_CONTENTS as a private memory space. But
    it is in use by breakpoint_restore_shadows to undo the breakpoints for display.
    
    Not going to speculate here if it may fix a breakpoints functionality.  It is
    at least not a regression.  No regressions found (on ia64).  New testcase.
    
    Thanks,
    Jan

-----------------------------------------------------------------------

Summary of changes:
 gdb/ChangeLog.archer-jankratochvil-misc           |   15 ++++
 gdb/breakpoint.c                                  |    2 +-
 gdb/ia64-tdep.c                                   |   87 ++++++++++++++++-----
 gdb/testsuite/ChangeLog.archer-jankratochvil-misc |    3 +
 gdb/testsuite/gdb.base/breakpoint-shadow.c        |   27 +++++++
 gdb/testsuite/gdb.base/breakpoint-shadow.exp      |   64 +++++++++++++++
 6 files changed, 177 insertions(+), 21 deletions(-)
 create mode 100644 gdb/ChangeLog.archer-jankratochvil-misc
 create mode 100644 gdb/testsuite/ChangeLog.archer-jankratochvil-misc
 create mode 100644 gdb/testsuite/gdb.base/breakpoint-shadow.c
 create mode 100644 gdb/testsuite/gdb.base/breakpoint-shadow.exp

First 500 lines of diff:
diff --git a/gdb/ChangeLog.archer-jankratochvil-misc b/gdb/ChangeLog.archer-jankratochvil-misc
new file mode 100644
index 0000000..926928f
--- /dev/null
+++ b/gdb/ChangeLog.archer-jankratochvil-misc
@@ -0,0 +1,15 @@
+2008-10-28  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	Fix automatic restoration of breakpoints memory for ia64.
+	* ia64-tdep.c (ia64_memory_insert_breakpoint): New comment part for
+	SHADOW_CONTENTS content.  Remova variable instr.  New variable cleanup.
+	Force automatic breakpoints restoration.  PLACED_SIZE and SHADOW_LEN
+	are now set larger, to BUNDLE_LEN - 2.
+	(ia64_memory_remove_breakpoint): Rename variables bundle to bundle_mem
+	and instr to instr_saved.  New variables bundle_saved and
+	instr_breakpoint.  Comment new reasons why we need to disable automatic
+	restoration of breakpoints.  Assert PLACED_SIZE and SHADOW_LEN.  New
+	check of the original memory content.
+	(ia64_breakpoint_from_pc): Array breakpoint extended to BUNDLE_LEN.
+	Sanity check the PCPTR parameter SLOTNUM value.  New #if check on
+	BREAKPOINT_MAX vs. BUNDLE_LEN.  Increase LENPTR to BUNDLE_LEN - 2.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 01b2990..a37bfdd 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3824,7 +3824,7 @@ print_one_breakpoint_location (struct breakpoint *b,
       struct cleanup *script_chain;
 
       annotate_field (9);
-      script_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "script");
+      script_chain = make_cleanup_ui_out_list_begin_end (uiout, "script");
       print_command_lines (uiout, l, 4);
       do_cleanups (script_chain);
     }
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 2f2e261..7d52fd7 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -545,7 +545,21 @@ fetch_instruction (CORE_ADDR addr, instruction_type *it, long long *instr)
    simulators.  So I changed the pattern slightly to do "break.i 0x080001"
    instead.  But that didn't work either (I later found out that this
    pattern was used by the simulator that I was using.)  So I ended up
-   using the pattern seen below. */
+   using the pattern seen below.
+
+   SHADOW_CONTENTS has byte-based addressing (PLACED_ADDRESS and SHADOW_LEN)
+   while we need bit-based addressing as the instructions length is 41 bits and
+   we must not modify/corrupt the adjacent ones in the same bundle.
+   Fortunately we may store larger memory incl. the adjacent bits with the
+   original memory content (not the possibly already stored breakpoints there).
+   We need to be careful in ia64_memory_remove_breakpoint to always restore
+   only the specific bits of this instruction ignoring any adjacent stored
+   bits.
+
+   We use the original addressing with the low nibble 0..2 which gets
+   incorrectly interpreted by the generic GDB code as the byte offset of
+   SHADOW_CONTENTS.  We store whole BUNDLE_LEN bytes just without these two
+   possibly skipped bytes.  */
 
 #if 0
 #define IA64_BREAKPOINT 0x00002000040LL
@@ -559,15 +573,21 @@ ia64_memory_insert_breakpoint (struct gdbarch *gdbarch,
   CORE_ADDR addr = bp_tgt->placed_address;
   char bundle[BUNDLE_LEN];
   int slotnum = (int) (addr & 0x0f) / SLOT_MULTIPLIER;
-  long long instr;
   int val;
   int template;
+  struct cleanup *cleanup;
 
   if (slotnum > 2)
     error (_("Can't insert breakpoint for slot numbers greater than 2."));
 
   addr &= ~0x0f;
 
+  /* Enable the automatic memory restoration from breakpoints while
+     we read our instruction bundle.  Otherwise, we could store into
+     SHADOW_CONTENTS an already stored breakpoint at the same location.
+     In practice it is already being prevented by the DUPLICATE field and
+     update_global_location_list.  */
+  cleanup = make_show_memory_breakpoints_cleanup (0);
   val = target_read_memory (addr, bundle, BUNDLE_LEN);
 
   /* Check for L type instruction in 2nd slot, if present then
@@ -578,13 +598,18 @@ ia64_memory_insert_breakpoint (struct gdbarch *gdbarch,
       slotnum = 2;
     }
 
-  instr = slotN_contents (bundle, slotnum);
-  memcpy (bp_tgt->shadow_contents, &instr, sizeof (instr));
-  bp_tgt->placed_size = bp_tgt->shadow_len = sizeof (instr);
+  /* Slot number 2 may skip at most 2 bytes at the beginning.  */
+  bp_tgt->placed_size = 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);
+
   replace_slotN_contents (bundle, IA64_BREAKPOINT, slotnum);
   if (val == 0)
-    target_write_memory (addr, bundle, BUNDLE_LEN);
+    target_write_memory (addr + slotnum, bundle + slotnum, bp_tgt->shadow_len);
 
+  do_cleanups (cleanup);
   return val;
 }
 
@@ -593,9 +618,9 @@ ia64_memory_remove_breakpoint (struct gdbarch *gdbarch,
 			       struct bp_target_info *bp_tgt)
 {
   CORE_ADDR addr = bp_tgt->placed_address;
-  char bundle[BUNDLE_LEN];
+  char bundle_mem[BUNDLE_LEN], bundle_saved[BUNDLE_LEN];
   int slotnum = (addr & 0x0f) / SLOT_MULTIPLIER;
-  long long instr;
+  long long instr_breakpoint, instr_saved;
   int val;
   int template;
   struct cleanup *cleanup;
@@ -604,23 +629,39 @@ ia64_memory_remove_breakpoint (struct gdbarch *gdbarch,
 
   /* Disable the automatic memory restoration from breakpoints while
      we read our instruction bundle.  Otherwise, the general restoration
-     mechanism kicks in and ends up corrupting our bundle, because it
-     is not aware of the concept of instruction bundles.  */
+     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.  */
   cleanup = make_show_memory_breakpoints_cleanup (1);
-  val = target_read_memory (addr, bundle, BUNDLE_LEN);
+  val = target_read_memory (addr, bundle_mem, BUNDLE_LEN);
 
   /* Check for L type instruction in 2nd slot, if present then
      bump up the slot number to the 3rd slot */
-  template = extract_bit_field (bundle, 0, 5);
+  template = extract_bit_field (bundle_mem, 0, 5);
   if (slotnum == 1 && template_encoding_table[template][1] == L)
     {
       slotnum = 2;
     }
 
-  memcpy (&instr, bp_tgt->shadow_contents, sizeof instr);
-  replace_slotN_contents (bundle, instr, slotnum);
+  gdb_assert (bp_tgt->placed_size == BUNDLE_LEN - 2);
+  gdb_assert (bp_tgt->placed_size == bp_tgt->shadow_len);
+
+  instr_breakpoint = slotN_contents (bundle_mem, slotnum);
+  if (instr_breakpoint != IA64_BREAKPOINT)
+    warning (_("Breakpoint removal cannot find the placed breakpoint at %s"),
+             paddr_nz (bp_tgt->placed_address));
+
+  /* Extract the original saved instruction from SLOTNUM normalizing its
+     bit-shift for INSTR_SAVED.  */
+  memcpy (bundle_saved, bundle_mem, BUNDLE_LEN);
+  memcpy (bundle_saved + slotnum, bp_tgt->shadow_contents, bp_tgt->shadow_len);
+  instr_saved = slotN_contents (bundle_saved, slotnum);
+
+  /* In BUNDLE_MEM be careful to modify only the bits belonging to SLOTNUM and
+     never any other possibly also stored in SHADOW_CONTENTS.  */
+  replace_slotN_contents (bundle_mem, instr_saved, slotnum);
   if (val == 0)
-    target_write_memory (addr, bundle, BUNDLE_LEN);
+    target_write_memory (addr, bundle_mem, BUNDLE_LEN);
 
   do_cleanups (cleanup);
   return val;
@@ -631,12 +672,18 @@ ia64_memory_remove_breakpoint (struct gdbarch *gdbarch,
 const unsigned char *
 ia64_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, int *lenptr)
 {
-  static unsigned char breakpoint[] =
-    { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
-  *lenptr = sizeof (breakpoint);
-#if 0
-  *pcptr &= ~0x0f;
+  static unsigned char breakpoint[BUNDLE_LEN];
+  int slotnum = (int) (*pcptr & 0x0f) / SLOT_MULTIPLIER;
+
+  if (slotnum > 2)
+    error (_("Can't insert breakpoint for slot numbers greater than 2."));
+
+#if BREAKPOINT_MAX < BUNDLE_LEN
+# error "BREAKPOINT_MAX < BUNDLE_LEN"
 #endif
+
+  *lenptr = BUNDLE_LEN - 2;
+
   return breakpoint;
 }
 
diff --git a/gdb/testsuite/ChangeLog.archer-jankratochvil-misc b/gdb/testsuite/ChangeLog.archer-jankratochvil-misc
new file mode 100644
index 0000000..06e4a60
--- /dev/null
+++ b/gdb/testsuite/ChangeLog.archer-jankratochvil-misc
@@ -0,0 +1,3 @@
+2008-10-28  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.base/breakpoint-shadow.exp, gdb.base/breakpoint-shadow.c: New.
diff --git a/gdb/testsuite/gdb.base/breakpoint-shadow.c b/gdb/testsuite/gdb.base/breakpoint-shadow.c
new file mode 100644
index 0000000..81ffc76
--- /dev/null
+++ b/gdb/testsuite/gdb.base/breakpoint-shadow.c
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2008 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (void)
+{
+  volatile int i;
+  
+  i = 1;	/* break-first */
+  i = 2;	/* break-second */
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/breakpoint-shadow.exp b/gdb/testsuite/gdb.base/breakpoint-shadow.exp
new file mode 100644
index 0000000..6aa62c1
--- /dev/null
+++ b/gdb/testsuite/gdb.base/breakpoint-shadow.exp
@@ -0,0 +1,64 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile breakpoint-shadow
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested "Couldn't compile test program"
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# We need to start the inferior to place the breakpoints in the memory at all.
+if { [gdb_start_cmd] < 0 } {
+    untested start
+    return -1
+}
+gdb_test "" "main \\(\\) at .*" "start"
+
+# The default "auto" mode removes all the breakpoints when we stop (and not
+# running the nonstop mode).  We would not be able to test the shadow.
+gdb_test "set breakpoint always-inserted on"
+gdb_test "show breakpoint always-inserted" "Always inserted breakpoint mode is on."
+
+set match "\nDump of assembler code for function main:\r\n(.*)End of assembler dump.\r\n$gdb_prompt $"
+
+set test "disassembly without breakpoints"
+gdb_test_multiple "disass main" $test {
+    -re $match {
+    	set orig $expect_out(1,string)
+	pass $test
+    }
+}
+
+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 "disassembly with breakpoints"
+gdb_test_multiple "disass main" $test {
+    -re $match {
+    	set got $expect_out(1,string)
+	if [string equal -nocase $orig $got] {
+	    pass $test
+	} else {
+	    fail $test
+	}
+    }
+}


hooks/post-receive
--
Repository for Project Archer.