[rfa:ppc64] Use target in convert_from_func_ptr_addr

[rfa:ppc64] Use target in convert_from_func_ptr_addr

[Andrew Cagney]

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

Hello,

The attached rewrites the PPC64 GNU/Linux convert_from_func_ptr_addr 
method so that it makes use of the explicit "struct target_ops" 
parameter  I recently added (previously it was indirectly using 
current_target).

ok?
Andrew

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

2003-10-17  Andrew Cagney  <cagney@redhat.com>

	* ppc-linux-tdep.c (ppc64_linux_convert_from_func_ptr_addr):
	Rewrite to use target_ops when transfering memory.

Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.44
diff -u -r1.44 ppc-linux-tdep.c
--- ppc-linux-tdep.c	22 Oct 2003 23:54:11 -0000	1.44
+++ ppc-linux-tdep.c	23 Oct 2003 01:32:51 -0000
@@ -936,14 +936,14 @@
 					CORE_ADDR addr,
 					struct target_ops *targ)
 {
-  struct obj_section *s;
-
-  s = find_pc_section (addr);
-
-  /* Check if ADDR points to a function descriptor.  */
-  if (s && strcmp (s->the_bfd_section->name, ".opd") == 0)
-    return read_memory_unsigned_integer (addr, 8);
-
+  struct section_table *s = target_section_by_addr (targ, addr);
+  if (s != NULL && s->the_bfd_section != NULL
+      && strcmp (s->the_bfd_section->name, ".opd") == 0)
+    {
+      char desc[8];
+      target_read (targ, TARGET_OBJECT_MEMORY, NULL, desc, addr, 8);
+      return extract_unsigned_integer (desc, 8);
+    }
   return addr;
 }
 

Re: [rfa:ppc64] Use target in convert_from_func_ptr_addr

[Kevin Buettner]

On Oct 22,  9:37pm, Andrew Cagney wrote:

> The attached rewrites the PPC64 GNU/Linux convert_from_func_ptr_addr 
> method so that it makes use of the explicit "struct target_ops" 
> parameter  I recently added (previously it was indirectly using 
> current_target).
[...]
> Index: ppc-linux-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
> retrieving revision 1.44
> diff -u -r1.44 ppc-linux-tdep.c
> --- ppc-linux-tdep.c	22 Oct 2003 23:54:11 -0000	1.44
> +++ ppc-linux-tdep.c	23 Oct 2003 01:32:51 -0000
> @@ -936,14 +936,14 @@
>  					CORE_ADDR addr,
>  					struct target_ops *targ)
>  {
> -  struct obj_section *s;
> -
> -  s = find_pc_section (addr);
> -
> -  /* Check if ADDR points to a function descriptor.  */
> -  if (s && strcmp (s->the_bfd_section->name, ".opd") == 0)
> -    return read_memory_unsigned_integer (addr, 8);
> -
> +  struct section_table *s = target_section_by_addr (targ, addr);
> +  if (s != NULL && s->the_bfd_section != NULL
> +      && strcmp (s->the_bfd_section->name, ".opd") == 0)
> +    {
> +      char desc[8];
> +      target_read (targ, TARGET_OBJECT_MEMORY, NULL, desc, addr, 8);
> +      return extract_unsigned_integer (desc, 8);
> +    }
>    return addr;
>  }

IMO, this version of the code is harder to read than the old version.

Can you explain what using an explicit `struct target_ops'' parameter
buys us?

Kevin

Re: [rfa:ppc64] Use target in convert_from_func_ptr_addr

[Andrew Cagney]

> On Oct 22,  9:37pm, Andrew Cagney wrote:
> 
> 
>> The attached rewrites the PPC64 GNU/Linux convert_from_func_ptr_addr 
>> method so that it makes use of the explicit "struct target_ops" 
>> parameter  I recently added (previously it was indirectly using 
>> current_target).

> 
> IMO, this version of the code is harder to read than the old version.
> 
> Can you explain what using an explicit `struct target_ops'' parameter
> buys us?

By using the explicit "struct target_ops", I've eliminated the 
assumption that the _single_ _global_ current_target contains the 
function descriptor.

Andrew

Re: [rfa:ppc64] Use target in convert_from_func_ptr_addr

[Kevin Buettner]

On Oct 23, 12:26pm, Andrew Cagney wrote:

> > On Oct 22,  9:37pm, Andrew Cagney wrote:
> > 
> > 
> >> The attached rewrites the PPC64 GNU/Linux convert_from_func_ptr_addr 
> >> method so that it makes use of the explicit "struct target_ops" 
> >> parameter  I recently added (previously it was indirectly using 
> >> current_target).
> 
> > 
> > IMO, this version of the code is harder to read than the old version.
> > 
> > Can you explain what using an explicit `struct target_ops'' parameter
> > buys us?
> 
> By using the explicit "struct target_ops", I've eliminated the 
> assumption that the _single_ _global_ current_target contains the 
> function descriptor.

Yes.  But what does this buy us?  I.e, what will we be able to do after
this patch that we couldn't do before?

Kevin

Re: [rfa:ppc64] Use target in convert_from_func_ptr_addr

[Andrew Cagney]

> On Oct 23, 12:26pm, Andrew Cagney wrote:
> 
> 
>> > On Oct 22,  9:37pm, Andrew Cagney wrote:
>> > 
>> > 
> 
>> >> The attached rewrites the PPC64 GNU/Linux convert_from_func_ptr_addr 
>> >> method so that it makes use of the explicit "struct target_ops" 
>> >> parameter  I recently added (previously it was indirectly using 
>> >> current_target).
> 
>> 
> 
>> > 
>> > IMO, this version of the code is harder to read than the old version.

If you want I can add a wrapper method, reducing the actual change to 
just the modification of:

-    return read_memory_unsigned_integer (addr, 8);

to:

+	return get_target_memory_unsigned (targ, addr, 8);

>> > Can you explain what using an explicit `struct target_ops'' parameter
>> > buys us?
> 
>> 
>> By using the explicit "struct target_ops", I've eliminated the 
>> assumption that the _single_ _global_ current_target contains the 
>> function descriptor.
> 
> 
> Yes.  But what does this buy us?  I.e, what will we be able to do after
> this patch that we couldn't do before?

The ability to apply this architecture method to an arbitrary target 
instance?  Sorry, but I'm afraid that I'm missing something in your 
question.

Andrew

Re: [rfa:ppc64] Use target in convert_from_func_ptr_addr

[Kevin Buettner]

On Oct 23,  5:02pm, Andrew Cagney wrote:

> If you want I can add a wrapper method, reducing the actual change to 
> just the modification of:
> 
> -    return read_memory_unsigned_integer (addr, 8);
> 
> to:
> 
> +	return get_target_memory_unsigned (targ, addr, 8);

That'd be great.  (With this change, consider it approved.)

> > Yes.  But what does this buy us?  I.e, what will we be able to do after
> > this patch that we couldn't do before?
> 
> The ability to apply this architecture method to an arbitrary target 
> instance?  Sorry, but I'm afraid that I'm missing something in your 
> question.

I'm trying to understand where you're going with these changes.  Do
you anticipate that this architecture method will ever be called
with anything other than current_target?  If so, then under what
circumstances?  Will a GDB user see enhanced functionality as a
result?

Kevin

Re: [rfa:ppc64] Use target in convert_from_func_ptr_addr

[Andrew Cagney]

> I'm trying to understand where you're going with these changes.

See my most recent posts to gdb@, along with the archives.  There is a 
long standing and oft repeated goal of cleaning up the target stack.  In 
part making it more OO.

> Will a GDB user see enhanced functionality as a
> result?

I guess so.  At present FSF's GDB when debugging shared libraries on 
GNU/Linux does this:

(top-gdb) run
Starting program: 
/home/cagney/PENDING/YYYY-MM-DD-target-convert-func/64/gdb/gdb
Warning:
Cannot insert breakpoint -1.
Error accessing memory address 0x7fdffec35c: Input/output error.
(top-gdb)

Andrew

[commit] Add get_target_memory ... + Use target in convert_from_func_ptr_addr

[Andrew Cagney]

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

> On Oct 23,  5:02pm, Andrew Cagney wrote:
> 
> 
>> If you want I can add a wrapper method, reducing the actual change to 
>> just the modification of:
>> 
>> -    return read_memory_unsigned_integer (addr, 8);
>> 
>> to:
>> 
>> +	return get_target_memory_unsigned (targ, addr, 8);
> 
> 
> That'd be great.  (With this change, consider it approved.)

I've committed the attached.  It also adds the new target wrapper methods:

	get_target_memory
and
	get_target_memory_unsigned

(I'm wondering about putting them in a separate file, but that's 
separate ...)

Andrew


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

2003-10-24  Andrew Cagney  <cagney@redhat.com>

	* target.c: Include "gdbcore.h".
	(get_target_memory, get_target_memory_unsigned): New functions.
	* target.h (get_target_memory, get_target_memory_unsigned): Declare.
	* ppc-linux-tdep.c (ppc64_linux_convert_from_func_ptr_addr):
	Use get_target_memory_unsigned.
	* Makefile.in (target.o): Update dependencies.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.462
diff -u -r1.462 Makefile.in
--- Makefile.in	23 Oct 2003 22:36:14 -0000	1.462
+++ Makefile.in	24 Oct 2003 20:21:53 -0000
@@ -2367,7 +2367,8 @@
 	$(block_h) $(dictionary_h) $(gdb_string_h) $(gdb_stat_h) $(cp_abi_h)
 target.o: target.c $(defs_h) $(gdb_string_h) $(target_h) $(gdbcmd_h) \
 	$(symtab_h) $(inferior_h) $(bfd_h) $(symfile_h) $(objfiles_h) \
-	$(gdb_wait_h) $(dcache_h) $(regcache_h) $(gdb_assert_h)
+	$(gdb_wait_h) $(dcache_h) $(regcache_h) $(gdb_assert_h) \
+	$(gdbcore_h)
 thread.o: thread.c $(defs_h) $(symtab_h) $(frame_h) $(inferior_h) \
 	$(environ_h) $(value_h) $(target_h) $(gdbthread_h) $(command_h) \
 	$(gdbcmd_h) $(regcache_h) $(gdb_h) $(gdb_string_h) $(ui_out_h)
Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.44
diff -u -r1.44 ppc-linux-tdep.c
--- ppc-linux-tdep.c	22 Oct 2003 23:54:11 -0000	1.44
+++ ppc-linux-tdep.c	24 Oct 2003 20:21:54 -0000
@@ -936,13 +936,11 @@
 					CORE_ADDR addr,
 					struct target_ops *targ)
 {
-  struct obj_section *s;
-
-  s = find_pc_section (addr);
+  struct section_table *s = target_section_by_addr (targ, addr);
 
   /* Check if ADDR points to a function descriptor.  */
   if (s && strcmp (s->the_bfd_section->name, ".opd") == 0)
-    return read_memory_unsigned_integer (addr, 8);
+    return get_target_memory_unsigned (targ, addr, 8);
 
   return addr;
 }
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.62
diff -u -r1.62 target.c
--- target.c	23 Oct 2003 00:13:53 -0000	1.62
+++ target.c	24 Oct 2003 20:21:54 -0000
@@ -37,6 +37,7 @@
 #include <signal.h>
 #include "regcache.h"
 #include "gdb_assert.h"
+#include "gdbcore.h"
 
 static void target_info (char *, int);
 
@@ -1212,6 +1213,28 @@
       QUIT;
     }
   return len;
+}
+
+/* Memory transfer methods.  */
+
+void
+get_target_memory (struct target_ops *ops, CORE_ADDR addr, void *buf,
+		   LONGEST len)
+{
+  if (target_read (ops, TARGET_OBJECT_MEMORY, NULL, buf, addr, len)
+      != len)
+    memory_error (EIO, addr);
+}
+
+ULONGEST
+get_target_memory_unsigned (struct target_ops *ops,
+			    CORE_ADDR addr, int len)
+{
+  char buf[sizeof (ULONGEST)];
+
+  gdb_assert (len <= sizeof (buf));
+  get_target_memory (ops, addr, buf, len);
+  return extract_unsigned_integer (buf, len);
 }
 
 static void
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.48
diff -u -r1.48 target.h
--- target.h	23 Oct 2003 03:01:55 -0000	1.48
+++ target.h	24 Oct 2003 20:21:54 -0000
@@ -248,6 +248,18 @@
 			     enum target_object object,
 			     const char *annex, const void *buf,
 			     ULONGEST offset, LONGEST len);
+
+/* Wrappers to target read/write that perform memory transfers.  They
+   throw an error if the memory transfer fails.
+
+   NOTE: cagney/2003-10-23: The naming schema is lifted from
+   "frame.h".  The parameter order is lifted from get_frame_memory,
+   which in turn lifted it from read_memory.  */
+
+extern void get_target_memory (struct target_ops *ops, CORE_ADDR addr,
+			       void *buf, LONGEST len);
+extern ULONGEST get_target_memory_unsigned (struct target_ops *ops,
+					    CORE_ADDR addr, int len);
 \f
 
 /* If certain kinds of activity happen, target_wait should perform