Re: [rfa] Restore "trust-readonly-section"

[Michael Snyder <msnyder@redhat.com>]

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

Daniel Jacobowitz wrote:
> On Thu, May 12, 2005 at 04:14:19PM -0700, Michael Snyder wrote:
> 
>>Hmm, tabs fubar -- I'll try again with the patch as an attachment.
>>
>>----- Original Message ----- 
>>From: "Michael Snyder" <msnyder@redhat.com>
>>To: "GDB Patches" <gdb-patches@sources.redhat.com>
>>Sent: Thursday, May 12, 2005 3:55 PM
>>Subject: [rfa] Restore "trust-readonly-section"
>>
>>
>>
>>>This seems to have succumbed to bit-rot -- there are new target-read 
>>>functions
>>>in target.c that don't pay any attention to this user-settable mode bit.
>>>
>>>The purpose of "trust-readonly-sections" is to improve speed on
>>>targets where reading memory is expensive (mostly remote).
>>>It checks to see if a read is from a read-only section, and if so,
>>>reads it from the exec file.  It defaults to "off" for safety, but if
>>>users choose to use it, it really speeds up prologue analysis
>>>(and therefore stepping).
>>>
>>>This patch just makes it work again.
> 
> 
> It seems odd to add the test both in target_xfer_partial (a dispatcher)
> and default_xfer_partial (an implementation).  Are they really both
> necessary?

I suppose you're right -- but I'm bewildered by the number of
possible entry points and paths thru this code.  In theory
I think I could get away with just covering three points:
target_read_memory, target_read_partial, and target_read_memory_partial,
were it not for the fact that do_xfer_memory is also an entry point.

do_xfer_memory seems to only be called from dcache.c --
but it's extern, so nothing prevents others from calling it.

And dcache_xfer_memory is extern too, so one can get in thru there
(although again, no one currently does AFAICT).

> The code might be simpler if you push the trust_readonly check inside
> target_read_trusted.  Also, could you name that something involving
> memory?

OK -- how about the attached, which includes four entry points?






[-- Attachment #2: trust-readonly --]
[-- Type: text/plain, Size: 5641 bytes --]

2005-05-12  Michael Snyder  <msnyder@redhat.com>

	* target.c (target_read_memory_trusted): New function.
	Implements "trust-readonly-section".
	(target_read_partial): Honor trust-readonly-section.
	(target_read_memory_partial): Ditto.
	(target_read_memory): Ditto.
	(do_xfer_memory): Ditto.

Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.108
diff -p -r1.108 target.c
*** target.c	16 May 2005 16:36:24 -0000	1.108
--- target.c	24 May 2005 01:00:20 -0000
*************** target_section_by_addr (struct target_op
*** 845,850 ****
--- 845,884 ----
    return NULL;
  }
  
+ static int trust_readonly = 0;
+ static void
+ show_trust_readonly (struct ui_file *file, int from_tty,
+ 		     struct cmd_list_element *c, const char *value)
+ {
+   fprintf_filtered (file, 
+ 		    _("Mode for reading from readonly sections is %s.\n"),
+ 		    value);
+ }
+ 
+ /* If trust-readonly-sections, and if MEMADDR is within a
+    read-only section, read LEN bytes from the bfd at MEMADDR,
+    placing the results in GDB's memory at MYADDR.  Returns
+    the number of bytes read.  */
+ 
+ static int
+ target_read_memory_trusted (CORE_ADDR memaddr, char *myaddr, int len)
+ {
+   struct section_table *secp;
+   /* User-settable option, "trust-readonly-sections".  If true,
+      then memory from any SEC_READONLY bfd section may be read
+      directly from the bfd file.  */
+   if (trust_readonly)
+     {
+       secp = target_section_by_addr (&current_target, memaddr);
+       if (secp != NULL
+ 	  && (bfd_get_section_flags (secp->bfd, secp->the_bfd_section)
+ 	      & SEC_READONLY))
+ 	return xfer_memory (memaddr, myaddr, len, 0, NULL, &current_target);
+     }
+ 
+   return 0;
+ }
+ 
  /* Return non-zero when the target vector has supplied an xfer_partial
     method and it, rather than xfer_memory, should be used.  */
  static int
*************** xfer_using_stratum (enum target_object o
*** 999,1004 ****
--- 1033,1044 ----
  int
  target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
  {
+   int ret;
+ 
+   /* Honor "trust-readonly-sections" if set.  */
+   if ((ret = target_read_memory_trusted (memaddr, myaddr, len)) > 0)
+     return (ret != len);
+ 
    if (target_xfer_partial_p ())
      return xfer_using_stratum (TARGET_OBJECT_MEMORY, NULL,
  			       memaddr, len, myaddr, NULL);
*************** target_stopped_data_address_p (struct ta
*** 1033,1048 ****
  }
  #endif
  
- static int trust_readonly = 0;
- static void
- show_trust_readonly (struct ui_file *file, int from_tty,
- 		     struct cmd_list_element *c, const char *value)
- {
-   fprintf_filtered (file, _("\
- Mode for reading from readonly sections is %s.\n"),
- 		    value);
- }
- 
  /* Move memory to or from the targets.  The top target gets priority;
     if it cannot handle it, it is offered to the next one down, etc.
  
--- 1073,1078 ----
*************** do_xfer_memory (CORE_ADDR memaddr, gdb_b
*** 1060,1082 ****
    if (len == 0)
      return 0;
  
    /* deprecated_xfer_memory is not guaranteed to set errno, even when
       it returns 0.  */
    errno = 0;
  
-   if (!write && trust_readonly)
-     {
-       struct section_table *secp;
-       /* User-settable option, "trust-readonly-sections".  If true,
-          then memory from any SEC_READONLY bfd section may be read
-          directly from the bfd file.  */
-       secp = target_section_by_addr (&current_target, memaddr);
-       if (secp != NULL
- 	  && (bfd_get_section_flags (secp->bfd, secp->the_bfd_section)
- 	      & SEC_READONLY))
- 	return xfer_memory (memaddr, myaddr, len, 0, attrib, &current_target);
-     }
- 
    /* The quick case is that the top target can handle the transfer.  */
    res = current_target.deprecated_xfer_memory
      (memaddr, myaddr, len, write, attrib, &current_target);
--- 1090,1104 ----
    if (len == 0)
      return 0;
  
+   /* Honor "trust-readonly-sections" if set.  */
+   if (!write && 
+       (res = target_read_memory_trusted (memaddr, myaddr, len)) > 0)
+ 	return res;
+ 
    /* deprecated_xfer_memory is not guaranteed to set errno, even when
       it returns 0.  */
    errno = 0;
  
    /* The quick case is that the top target can handle the transfer.  */
    res = current_target.deprecated_xfer_memory
      (memaddr, myaddr, len, write, attrib, &current_target);
*************** target_xfer_memory_partial (CORE_ADDR me
*** 1244,1253 ****
  int
  target_read_memory_partial (CORE_ADDR memaddr, char *buf, int len, int *err)
  {
    if (target_xfer_partial_p ())
      {
-       int retval;
- 
        retval = target_xfer_partial (target_stack, TARGET_OBJECT_MEMORY,
  				    NULL, buf, NULL, memaddr, len);
  
--- 1266,1279 ----
  int
  target_read_memory_partial (CORE_ADDR memaddr, char *buf, int len, int *err)
  {
+   int retval;
+ 
+   /* Honor "trust-readonly-sections" if set.  */
+   if ((retval = target_read_memory_trusted (memaddr, buf, len)) > 0)
+     return retval;
+ 
    if (target_xfer_partial_p ())
      {
        retval = target_xfer_partial (target_stack, TARGET_OBJECT_MEMORY,
  				    NULL, buf, NULL, memaddr, len);
  
*************** target_read_partial (struct target_ops *
*** 1351,1356 ****
--- 1377,1389 ----
  		     const char *annex, gdb_byte *buf,
  		     ULONGEST offset, LONGEST len)
  {
+   int ret;
+ 
+   /* Honor "trust-readonly-sections" if set.  */
+   if (object == TARGET_OBJECT_MEMORY)
+     if ((ret = target_read_memory_trusted (offset, buf, len)) > 0)
+       return ret;
+ 
    return target_xfer_partial (ops, object, annex, buf, NULL, offset, len);
  }