[patch rfc] Always disassemble the target

[patch rfc] Always disassemble the target

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

Hello,

This follows up:
http://sources.redhat.com/ml/gdb-patches/2003-04/msg00111.html
by ``going all the way''.

It modifies "disasm.c" so that it always goes to the target, and never 
the local executable.

The problem is that, regardless of the selected target vector, the 
disassembler can end up disassembling memory that is not in the 
executable - the assumption was wrong.  This is easily seen with:

	disassemble &variable &variable+1

The testsuite part of the patch does just this (fails before, passes after).

As for avoiding target memory read overhead, there is "set 
trust-read-only-sections on" which is a definite improvement.

Andrew

PS: I stumbled across this when trying to convert convert "x/i" to use 
the new disassembler (display.exp mysteriously gained three failures on 
a simulator target).

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 6697 bytes --]

2003-04-28  Andrew Cagney  <cagney@redhat.com>

	* disasm.c (gdb_disassemble_from_exec): Delete global variable.
	(gdb_disassembly): Make "di" non static, always initialize and
	cleanup.  Always use dis_asm_read_memory.
	(gdb_dis_asm_read_memory): Delete function.

Index: testsuite/ChangeLog
2003-04-28  Andrew Cagney  <cagney@redhat.com>

	* gdb.asm/asm-source.exp: Check that the disassm and x/i of a
	variable doesn't give a memory error.

Index: disasm.c
===================================================================
RCS file: /cvs/src/src/gdb/disasm.c,v
retrieving revision 1.6
diff -u -r1.6 disasm.c
--- disasm.c	8 Apr 2003 01:40:31 -0000	1.6
+++ disasm.c	28 Apr 2003 20:57:57 -0000
@@ -43,29 +43,6 @@
   CORE_ADDR end_pc;
 };
 
-/* This variable determines where memory used for disassembly is read from. */
-int gdb_disassemble_from_exec = -1;
-
-/* This is the memory_read_func for gdb_disassemble when we are
-   disassembling from the exec file. */
-static int
-gdb_dis_asm_read_memory (bfd_vma memaddr, bfd_byte * myaddr,
-			 unsigned int len, disassemble_info * info)
-{
-  extern struct target_ops exec_ops;
-  int res;
-
-  errno = 0;
-  res = xfer_memory (memaddr, myaddr, len, 0, 0, &exec_ops);
-
-  if (res == len)
-    return 0;
-  else if (errno == 0)
-    return EIO;
-  else
-    return errno;
-}
-
 static int
 compare_lines (const void *mle1p, const void *mle2p)
 {
@@ -315,62 +292,31 @@
 		int mixed_source_and_assembly,
 		int how_many, CORE_ADDR low, CORE_ADDR high)
 {
-  static disassemble_info di;
-  static int di_initialized;
+  struct ui_stream *stb = ui_out_stream_new (uiout);
+  struct cleanup *cleanups = make_cleanup_ui_out_stream_delete (stb);
+  disassemble_info di;
   /* To collect the instruction outputted from opcodes. */
-  static struct ui_stream *stb = NULL;
   struct symtab *symtab = NULL;
   struct linetable_entry *le = NULL;
   int nlines = -1;
 
-  if (!di_initialized)
-    {
-      /* We don't add a cleanup for this, because the allocation of
-         the stream is done once only for each gdb run, and we need to
-         keep it around until the end. Hopefully there won't be any
-         errors in the init code below, that make this function bail
-         out. */
-      stb = ui_out_stream_new (uiout);
-      INIT_DISASSEMBLE_INFO_NO_ARCH (di, stb->stream,
-				     (fprintf_ftype) fprintf_unfiltered);
-      di.flavour = bfd_target_unknown_flavour;
-      di.memory_error_func = dis_asm_memory_error;
-      di.print_address_func = dis_asm_print_address;
-      di_initialized = 1;
-    }
-
+  INIT_DISASSEMBLE_INFO_NO_ARCH (di, stb->stream,
+				 (fprintf_ftype) fprintf_unfiltered);
+  di.flavour = bfd_target_unknown_flavour;
+  di.memory_error_func = dis_asm_memory_error;
+  di.print_address_func = dis_asm_print_address;
+  /* NOTE: cagney/2003-04-28: The original code, from the old Insight
+     disassembler had a local optomization here.  By default it would
+     access the executable file, instead of the target memory (there
+     was a growing list of exceptions though).  Unfortunatly, the
+     heuristic was flawed.  Commands like "disassemble &variable"
+     didn't work as they relied on the access going to the target.
+     Further, it has been supperseeded by trust-read-only-sections
+     (although that should be superseeded by target_trust..._p()).  */
+  di.read_memory_func = dis_asm_read_memory;
   di.mach = gdbarch_bfd_arch_info (current_gdbarch)->mach;
   di.endian = gdbarch_byte_order (current_gdbarch);
 
-  /* If gdb_disassemble_from_exec == -1, then we use the following heuristic to
-     determine whether or not to do disassembly from target memory or from the
-     exec file:
-
-     If we're debugging a local process, read target memory, instead of the
-     exec file.  This makes disassembly of functions in shared libs work
-     correctly.  Also, read target memory if we are debugging native threads.
-
-     Else, we're debugging a remote process, and should disassemble from the
-     exec file for speed.  However, this is no good if the target modifies its
-     code (for relocation, or whatever).  */
-
-  if (gdb_disassemble_from_exec == -1)
-    {
-      if (strcmp (target_shortname, "child") == 0
-	  || strcmp (target_shortname, "procfs") == 0
-	  || strcmp (target_shortname, "vxprocess") == 0
-          || strcmp (target_shortname, "core") == 0
-	  || strstr (target_shortname, "-thread") != NULL)
-	gdb_disassemble_from_exec = 0;	/* It's a child process, read inferior mem */
-      else
-	gdb_disassemble_from_exec = 1;	/* It's remote, read the exec file */
-    }
-
-  if (gdb_disassemble_from_exec)
-    di.read_memory_func = gdb_dis_asm_read_memory;
-  else
-    di.read_memory_func = dis_asm_read_memory;
-
   /* Assume symtab is valid for whole PC range */
   symtab = find_pc_symtab (low);
 
@@ -389,5 +335,6 @@
     do_mixed_source_and_assembly (uiout, &di, nlines, le, low,
 				  high, symtab, how_many, stb);
 
+  do_cleanups (cleanups);
   gdb_flush (gdb_stdout);
 }
Index: testsuite/gdb.asm/asm-source.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.asm/asm-source.exp,v
retrieving revision 1.30
diff -u -r1.30 asm-source.exp
--- testsuite/gdb.asm/asm-source.exp	21 Mar 2003 20:34:38 -0000	1.30
+++ testsuite/gdb.asm/asm-source.exp	28 Apr 2003 20:58:00 -0000
@@ -293,11 +293,35 @@
 gdb_test "return" "\#0  main .*37\[ \t\]+gdbasm_exit0" "return from foo2" \
 	"Make (foo2|selected stack frame) return now\?.*" "y"
 
-# See if we can look at a global variable
+# Disassemble something, check the output
+proc test_dis { command var } {
+    global gdb_prompt
+    send_gdb "${command}\n"
+    gdb_expect {
+	-re "${var}.*:.*Cannot access" {
+	    # The "disassembler" was only accessing the local
+	    # executable and that would cause attempts to disassemble
+	    # variables to fail (memory not valid).
+	    fail "${command} (memory read error)"
+	}
+	-re "${var}.*:.*${gdb_prompt}" {
+	    pass "${command}"
+	}
+	timeout {
+	    fail "${command} (timeout)"
+	}
+    }
+}
+
+# See if we can look at a global variable, three ways
 gdb_test "print globalvar" ".* = 11" "look at global variable"
+test_dis "x/i globalvar" "globalvar"
+test_dis "disassem &globalvar &globalvar+1" "globalvar"
 
-# See if we can look at a static variable
+# See if we can look at a static variable, three ways
 gdb_test "print staticvar" ".* = 5" "look at static variable"
+test_dis "x/i &staticvar" "staticvar"
+test_dis "disassem &staticvar &staticvar+1" "staticvar"
 
 # See if we can look at a static function
 gdb_test "disassem foostatic" ".*<foostatic\\+0>:.*End of assembler dump." \

Re: [patch rfc] Always disassemble the target

[Andrew Cagney]

> 2003-04-28  Andrew Cagney  <cagney@redhat.com>
> 
> 	* disasm.c (gdb_disassemble_from_exec): Delete global variable.
> 	(gdb_disassembly): Make "di" non static, always initialize and
> 	cleanup.  Always use dis_asm_read_memory.
> 	(gdb_dis_asm_read_memory): Delete function.
> 
> Index: testsuite/ChangeLog
> 2003-04-28  Andrew Cagney  <cagney@redhat.com>
> 
> 	* gdb.asm/asm-source.exp: Check that the disassm and x/i of a
> 	variable doesn't give a memory error.
> 

I've checked this in.

Andrew