Building solib-target.c when CORE_ADDR != ULONGEST.

Building solib-target.c when CORE_ADDR != ULONGEST.

[Pedro Alves]

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

Hi,

Building solib-target.c on host where CORE_ADDR != ULONGEST is broken,
because of this:

static void
library_list_start_segment (struct gdb_xml_parser *parser,
			    const struct gdb_xml_element *element,
			    void *user_data, VEC(gdb_xml_value_s) *attributes)
{
    VEC(lm_info_p) **list = user_data;
    struct lm_info *last = VEC_last (lm_info_p, *list);
    ULONGEST *address_p = VEC_index (gdb_xml_value_s, attributes, 0)->value;

    VEC_safe_push (CORE_ADDR, last->segment_bases, address_p);
}

Pushing a ULONGEST* into a CORE_ADDR VEC doesn't work.

I'm using the attached patch to try to catch invalid data send by a
(remote) target, but I guess it isn't correct for all archs.

I was building arm-wince-mingw32ce on i686-pc-cygwin.

Cheers,
Pedro Alves


[-- Attachment #2: solib-target_CORE_ADDR.diff --]
[-- Type: text/x-diff, Size: 1952 bytes --]

2007-07-08  Pedro Alves  <pedro_alves@portugalmail.pt>

	* solib-target.c (core_addr_from_ulongest): New.
	(library_list_start_segment): Use core_addr_from_ulongest.

---
 gdb/solib-target.c |   43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

Index: src/gdb/solib-target.c
===================================================================
--- src.orig/gdb/solib-target.c	2007-07-04 01:19:56.000000000 +0100
+++ src/gdb/solib-target.c	2007-07-07 16:32:44.000000000 +0100
@@ -72,6 +72,43 @@ solib_target_parse_libraries (const char
 
 #include "xml-support.h"
 
+static int
+core_addr_from_ulongest (CORE_ADDR* to, const ULONGEST* from)
+{
+  int erange = 0;
+
+  if (sizeof (*to) == 4 && sizeof (*from) == 8)
+    {
+      unsigned int high_part;
+      unsigned int low_part;
+      CORE_ADDR neg_one = ~ (CORE_ADDR) 0;
+
+      high_part = *from >> 32;
+      low_part = *from & 0xffffffff;
+
+      if (neg_one < 1)
+	{
+	  /* CORE_ADDR is signed.  */
+	  if (high_part != 0xffffffff && high_part != 0)
+	    erange = 1;
+	  else if (high_part == 0xffffffff && low_part < 0x80000000)
+	    erange = 1;
+	}
+      else if (high_part != 0)
+	erange = 1;
+    }
+
+  *to = (CORE_ADDR) *from;
+
+  if (erange)
+    {
+      errno = ERANGE;
+      return -1;
+    }
+
+  return 0;
+}
+
 /* Handle the start of a <segment> element.  */
 
 static void
@@ -82,8 +119,12 @@ library_list_start_segment (struct gdb_x
   VEC(lm_info_p) **list = user_data;
   struct lm_info *last = VEC_last (lm_info_p, *list);
   ULONGEST *address_p = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+  CORE_ADDR address;
+
+  if (core_addr_from_ulongest (&address, address_p) < 0)
+    warning (_("Target reported an out-of-range address."));
 
-  VEC_safe_push (CORE_ADDR, last->segment_bases, address_p);
+  VEC_safe_push (CORE_ADDR, last->segment_bases, &address);
 }
 
 /* Handle the start of a <library> element.  */

Re: Building solib-target.c when CORE_ADDR != ULONGEST.

[Daniel Jacobowitz]

On Sun, Jul 08, 2007 at 02:20:49AM +0100, Pedro Alves wrote:
> I'm using the attached patch to try to catch invalid data send by a
> (remote) target, but I guess it isn't correct for all archs.

I don't think this check is worth doing, honestly.  Does it work fine
with just the cast?


-- 
Daniel Jacobowitz
CodeSourcery

Re: Building solib-target.c when CORE_ADDR != ULONGEST.

[Pedro Alves]

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

Daniel Jacobowitz wrote:
> On Sun, Jul 08, 2007 at 02:20:49AM +0100, Pedro Alves wrote:
>> I'm using the attached patch to try to catch invalid data send by a
>> (remote) target, but I guess it isn't correct for all archs.
> 
> I don't think this check is worth doing, honestly.  Does it work fine
> with just the cast?
> 

Sure.  I used it to double check gdbserver was passing right
values, and I agree that it is overkill.

About the VEC in question.  I noticed that the xml parsing
xmalloc's the memory for the ULONGEST, but it is safe to
pass a pointer in a local var, right?

I got confused because in vec.h:

      "Because of the different behavior of structure objects, scalar
       objects and of pointers, there are three flavors, one for each of
       these variants.  Both the structure object and pointer variants
       pass pointers to objects around -- in the former case the pointers
       are stored into the vector and in the latter case the pointers are
       dereferenced and the objects copied into the vector.  The scalar
       object variant is suitable for int-like objects, and the vector
       elements are returned by value.  "

Something in there confuses me.  The former/latter type of
descriptions adds an indirection that I always find
distracting.

Here:

"in the latter case the pointers are dereferenced and the objects
copied into the vector."

... is it talking about the latter from the immediately previous sentence ?:

"Both the structure object and pointer variants pass pointers
to objects around"
Seems strange if so.

The 'by value' statement isn't clear to me either:
"object variant is suitable for int-like objects, and the vector
       elements are returned by value.  "

T *VEC_T_index(VEC(T) *v, unsigned ix); // Object

?  What does 'by value' mean then?

How about this, it is right?

      "Because of the different behavior of structure objects, scalar
       objects and of pointers, there are three flavors, one for each of
       these variants.  Both the structure object and pointer variants
       pass pointers to objects around.
       The pointer variant simply stores pointer directly in the vector.
       In the structure object variant, the pointers are dereferenced
       and the objects copied into the vector.  In this variant, the
       vector elements are returned by value, making it suitable for
       int-like objects.  "

I understood it as if they were analogous of:

struct type;
pointer -> std::vector<type*>
object -> std::vector<type>
integral -> std::vector<int>

I guess I'm pretty much confused.  :(

Attached is a new patch.  Tested that shreloc.exp still passes
all the tests on WinCE, testing from a Cygwin host.

Cheers,
Pedro Alves


[-- Attachment #2: solib-target_CORE_ADDR_cast.diff --]
[-- Type: text/x-diff, Size: 902 bytes --]

2007-07-08  Pedro Alves  <pedro_alves@portugalmail.pt>

	* solib-target.c (library_list_start_segment): Cast address to CORE_ADDR.

---
 gdb/solib-target.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: src/gdb/solib-target.c
===================================================================
--- src.orig/gdb/solib-target.c	2007-07-08 02:04:24.000000000 +0100
+++ src/gdb/solib-target.c	2007-07-08 12:19:12.000000000 +0100
@@ -82,8 +82,9 @@ library_list_start_segment (struct gdb_x
   VEC(lm_info_p) **list = user_data;
   struct lm_info *last = VEC_last (lm_info_p, *list);
   ULONGEST *address_p = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+  CORE_ADDR address = (CORE_ADDR) *address_p;
 
-  VEC_safe_push (CORE_ADDR, last->segment_bases, address_p);
+  VEC_safe_push (CORE_ADDR, last->segment_bases, &address);
 }
 
 /* Handle the start of a <library> element.  */

Re: Building solib-target.c when CORE_ADDR != ULONGEST.

[Daniel Jacobowitz]

On Sun, Jul 08, 2007 at 03:32:02PM +0100, Pedro Alves wrote:
> About the VEC in question.  I noticed that the xml parsing
> xmalloc's the memory for the ULONGEST, but it is safe to
> pass a pointer in a local var, right?

Yes, that's right.  The patch is OK.

> I got confused because in vec.h:
> 
>      "Because of the different behavior of structure objects, scalar
>       objects and of pointers, there are three flavors, one for each of
>       these variants.  Both the structure object and pointer variants
>       pass pointers to objects around -- in the former case the pointers
>       are stored into the vector and in the latter case the pointers are
>       dereferenced and the objects copied into the vector.  The scalar
>       object variant is suitable for int-like objects, and the vector
>       elements are returned by value.  "
> 
> Something in there confuses me.  The former/latter type of
> descriptions adds an indirection that I always find
> distracting.

Far as I can tell, Nathan got his formers and his latters crossed, and
that's why it doesn't make sense.

It would have made more sense for me to use DEF_VEC_I for these than
the DEF_VEC_O I actually used, but to be honest I didn't realize it
was there.  The use of DEF_VEC_O makes this rather more confusing
than necessary.

> The 'by value' statement isn't clear to me either:
> "object variant is suitable for int-like objects, and the vector
>       elements are returned by value.  "
> 
> T *VEC_T_index(VEC(T) *v, unsigned ix); // Object
> 
> ?  What does 'by value' mean then?

The previous word is "scalar".  Scalar object -> integer, not
structure object.  So the last bit of your correction is wrong.


> 2007-07-08  Pedro Alves  <pedro_alves@portugalmail.pt>
> 
> 	* solib-target.c (library_list_start_segment): Cast address to CORE_ADDR.

OK.

-- 
Daniel Jacobowitz
CodeSourcery