[commit/Tru64] bring back the Tru64 port to life...

[commit/Tru64] bring back the Tru64 port to life...

[Joel Brobecker]

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

Hello,

Following some changes in some of the target_ops methods, GDB on Tru64
was having problem when debugging programs that had threads. This was
a good opportunity to modernize a bit the thread layer for this OS...

I also noticed a warning in osf-solib.c, which I fixed in this commit.

2009-03-05  Joel Brobecker  <brobecker@adacore.com>

        Get rid of the global "base_target" and use "find_target_beneath"
        to find the underlying target.
        * dec-thread.c (base_target): Delete.
        (enable_dec_thread): Remove assignement to base_target.
        (dec_thread_detach, dec_thread_wait, dec_thread_wait)
        (dec_thread_fetch_registers, dec_thread_store_registers)
        (dec_thread_mourn_inferior, dec_thread_pid_to_str):
        Update the function profile if necessary.
        Use find_target_beneath to call the same method but from
        the underlying target, removing the need for "base_target".

        * dec-thread.c (dec_thread_get_regsets, dec_thread_set_regsets):
        Fix a copy/paste error in a few debug traces...

        * solib-osf.c (init_so): Use a simpler method for computing
        the size of lm_info structure. This also gets rid of warning
        emitted by the compiler.

Tested on alpha-tru64. Checked in.

-- 
Joel

[-- Attachment #2: osf.diff --]
[-- Type: text/plain, Size: 6883 bytes --]

Index: dec-thread.c
===================================================================
RCS file: /cvs/src/src/gdb/dec-thread.c,v
retrieving revision 1.4
diff -u -p -r1.4 dec-thread.c
--- dec-thread.c	6 Feb 2009 22:21:26 -0000	1.4
+++ dec-thread.c	5 Mar 2009 23:26:14 -0000
@@ -40,9 +40,6 @@ pthreadDebugContext_t debug_context;
 /* The dec-thread target_ops structure.  */
 static struct target_ops dec_thread_ops;
 
-/* A copy of the target_ops over which our dec_thread_ops is pushed.  */
-static struct target_ops base_target;
-
 /* Print a debug trace if DEBUG_DEC_THREAD is set (its value is adjusted
    by the user using "set debug dec-thread ...").  */
 
@@ -253,7 +250,6 @@ enable_dec_thread (void)
       return;
     }
 
-  base_target = current_target;
   push_target (&dec_thread_ops);
   dec_thread_active = 1;
 
@@ -424,12 +420,14 @@ resync_thread_list (void)
 /* The "to_detach" method of the dec_thread_ops.  */
 
 static void
-dec_thread_detach (char *args, int from_tty)
+dec_thread_detach (struct target_ops *ops, char *args, int from_tty)
 {   
+  struct target_ops *beneath = find_target_beneath (ops);
+
   debug ("dec_thread_detach");
 
   disable_dec_thread ();
-  base_target.to_detach (&base_target, args, from_tty);
+  beneath->to_detach (beneath, args, from_tty);
 }
 
 /* Return the ptid of the thread that is currently active.  */
@@ -457,14 +455,14 @@ dec_thread_wait (struct target_ops *ops,
 		 ptid_t ptid, struct target_waitstatus *status)
 {
   ptid_t active_ptid;
+  struct target_ops *beneath = find_target_beneath (ops);
 
   debug ("dec_thread_wait");
 
-  ptid = base_target.to_wait (&base_target, ptid, status);
+  ptid = beneath->to_wait (beneath, ptid, status);
 
-  /* The ptid returned by the base_target is the ptid of the process.
-     We need to find which thread is currently active and return its
-     ptid.  */
+  /* The ptid returned by the target beneath us is the ptid of the process.
+     We need to find which thread is currently active and return its ptid.  */
   resync_thread_list ();
   active_ptid = get_active_ptid ();
   if (ptid_equal (active_ptid, null_ptid))
@@ -487,7 +485,7 @@ dec_thread_get_regsets (pthreadDebugId_t
   res = pthreadDebugThdGetReg (debug_context, tid, &regs);
   if (res != ESUCCESS)
     {
-      debug ("dec_thread_fetch_registers: pthreadDebugThdGetReg -> %d", res);
+      debug ("dec_thread_get_regsets: pthreadDebugThdGetReg -> %d", res);
       return -1;
     }
   memcpy (gregset->regs, &regs, sizeof (regs));
@@ -495,7 +493,7 @@ dec_thread_get_regsets (pthreadDebugId_t
   res = pthreadDebugThdGetFreg (debug_context, tid, &fregs);
   if (res != ESUCCESS)
     {
-      debug ("dec_thread_fetch_registers: pthreadDebugThdGetFreg -> %d", res);
+      debug ("dec_thread_get_regsets: pthreadDebugThdGetFreg -> %d", res);
       return -1;
     }
   memcpy (fpregset->regs, &fregs, sizeof (fregs));
@@ -510,7 +508,8 @@ dec_thread_get_regsets (pthreadDebugId_t
    registers.  */
 
 static void
-dec_thread_fetch_registers (struct regcache *regcache, int regno)
+dec_thread_fetch_registers (struct target_ops *ops,
+                            struct regcache *regcache, int regno)
 {
   pthreadDebugId_t tid = ptid_get_tid (inferior_ptid);
   gregset_t gregset;
@@ -522,7 +521,9 @@ dec_thread_fetch_registers (struct regca
 
   if (tid == 0 || ptid_equal (inferior_ptid, get_active_ptid ()))
     {
-      base_target.to_fetch_registers (regcache, regno);
+      struct target_ops *beneath = find_target_beneath (ops);
+
+      beneath->to_fetch_registers (beneath, regcache, regno);
       return;
     }
 
@@ -550,7 +551,7 @@ dec_thread_set_regsets (pthreadDebugId_t
   res = pthreadDebugThdSetReg (debug_context, tid, &regs);
   if (res != ESUCCESS)
     {
-      debug ("dec_thread_fetch_registers: pthreadDebugThdSetReg -> %d", res);
+      debug ("dec_thread_set_regsets: pthreadDebugThdSetReg -> %d", res);
       return -1;
     }
 
@@ -558,7 +559,7 @@ dec_thread_set_regsets (pthreadDebugId_t
   res = pthreadDebugThdSetFreg (debug_context, tid, &fregs);
   if (res != ESUCCESS)
     {
-      debug ("dec_thread_fetch_registers: pthreadDebugThdSetFreg -> %d", res);
+      debug ("dec_thread_set_regsets: pthreadDebugThdSetFreg -> %d", res);
       return -1;
     }
 
@@ -571,7 +572,8 @@ dec_thread_set_regsets (pthreadDebugId_t
    just one register, we store all the registers.  */
 
 static void
-dec_thread_store_registers (struct regcache *regcache, int regno)
+dec_thread_store_registers (struct target_ops *ops,
+                            struct regcache *regcache, int regno)
 {
   pthreadDebugId_t tid = ptid_get_tid (inferior_ptid);
   gregset_t gregset;
@@ -582,7 +584,9 @@ dec_thread_store_registers (struct regca
 
   if (tid == 0 || ptid_equal (inferior_ptid, get_active_ptid ()))
     {
-      base_target.to_store_registers (regcache, regno);
+      struct target_ops *beneath = find_target_beneath (ops);
+
+      beneath->to_store_registers (beneath, regcache, regno);
       return;
     }
 
@@ -601,17 +605,19 @@ dec_thread_store_registers (struct regca
 /* The "to_mourn_inferior" method of the dec_thread_ops.  */
 
 static void
-dec_thread_mourn_inferior (void)
+dec_thread_mourn_inferior (struct target_ops *ops)
 {
+  struct target_ops *beneath = find_target_beneath (ops);
+
   debug ("dec_thread_mourn_inferior");
 
   disable_dec_thread ();
-  base_target.to_mourn_inferior (&base_target);
+  beneath->to_mourn_inferior (beneath);
 }
 
 /* The "to_thread_alive" method of the dec_thread_ops.  */
 static int
-dec_thread_thread_alive (ptid_t ptid)
+dec_thread_thread_alive (struct target_ops *ops, ptid_t ptid)
 {
   debug ("dec_thread_thread_alive (tid=%ld)", ptid_get_tid (ptid));
 
@@ -628,7 +634,11 @@ dec_thread_pid_to_str (struct target_ops
   static char *ret = NULL;
 
   if (ptid_get_tid (ptid) == 0)
-    return base_target.to_pid_to_str (&base_target, ptid);
+    {
+      struct target_ops *beneath = find_target_beneath (ops);
+
+      return beneath->to_pid_to_str (beneath, ptid);
+    }
 
   /* Free previous return value; a new one will be allocated by
      xstrprintf().  */
Index: solib-osf.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-osf.c,v
retrieving revision 1.23
diff -u -p -r1.23 solib-osf.c
--- solib-osf.c	25 Feb 2009 02:14:22 -0000	1.23
+++ solib-osf.c	5 Mar 2009 23:26:14 -0000
@@ -424,8 +424,8 @@ init_so (struct so_list *so, char *name,
   memcpy (so->so_name, so->so_original_name, namelen + 1);
 
   /* Allocate section space.  */
-  so->lm_info = xmalloc ((unsigned) &(((struct lm_info *)0)->secs) +
-			 nsecs * sizeof *so->lm_info);
+  so->lm_info = xmalloc (sizeof (struct lm_info)
+                         + (nsecs - 1) * sizeof (struct lm_sec));
   so->lm_info->isloader = isloader;
   so->lm_info->nsecs = nsecs;
   for (i = 0; i < nsecs; i++)

Re: [commit/Tru64] bring back the Tru64 port to life...

[Pedro Alves]

Not knowing a thing about tru64 or osf,

On Thursday 05 March 2009 23:55:03, Joel Brobecker wrote:
>    /* Allocate section space.  */
> -  so->lm_info = xmalloc ((unsigned) &(((struct lm_info *)0)->secs) +
> -                        nsecs * sizeof *so->lm_info);
> +  so->lm_info = xmalloc (sizeof (struct lm_info)
> +                         + (nsecs - 1) * sizeof (struct lm_sec));

Can nsecs be 0 here?  Since it is checked at least here:

static void
osf_relocate_section_addresses (struct so_list *so,
				struct section_table *sec)
{
(...)
  if (lmi->nsecs && !lmi->secs[0].name)
    fetch_sec_names (lmi);

If, so, you'll allocate less memory than you want in that
case.

-- 
Pedro Alves

Re: [commit/Tru64] bring back the Tru64 port to life...

[Joel Brobecker]

> Can nsecs be 0 here?

Good question. It seems pretty highly unlikely, but I'm not sure whether
this is possible or not. I couldn't find a whole lot of documentation
about the lmi_nregion field in ldr_module_info_t. But from the code
in solib-osf.c, it looks like this field is actually the number of
sections. I don't see how a shared library could have zero section
and yet still be loaded. If the naming in solib-osf.c got me
confused, and the secs array actually refers to memory regions,
you'd think that there would be at least one region for the code.

> Since it is checked at least here:

Yeah, that's strange.

I can add an extra check, but...

> struct lm_info
>   {
>     int isloader;               /* whether the module is /sbin/loader */
>     int nsecs;                  /* length of .secs */
>     struct lm_sec secs[1];      /* variable-length array of sections, sorted
>                                    by name */
>   };

The previous code was a glorified version of 

   malloc (offsetof (struct lm_info, secs)
           + nsecs * sizeof (struct lm_sec))

The new expression I used should be strictly equivalent:

  malloc (sizeof (struct lm_info)
          + (nsecs - 1) * sizeof (struct lm_sec));

In both cases, if nsecs is zero, we end up allocating less memory
than sizeof (struct lm_info). Intuitively, it seems OK, since we
shouldn't really access the secs array if nsecs is null.

One easy way out is to change the allocation to allocate one extra
entry in the secs array. It's slightly wasteful, but it's just a
few bytes times the number of shared libraries.

What do you think?

-- 
Joel

Re: [commit/Tru64] bring back the Tru64 port to life...

[Pedro Alves]

On Friday 06 March 2009 00:57:53, Joel Brobecker wrote:
> In both cases, if nsecs is zero, we end up allocating less memory
> than sizeof (struct lm_info). Intuitively, it seems OK, since we
> shouldn't really access the secs array if nsecs is null.

Ah, you're right.

> 
> One easy way out is to change the allocation to allocate one extra
> entry in the secs array. It's slightly wasteful, but it's just a
> few bytes times the number of shared libraries.
> 
> What do you think?
> 

Nah, it's fine as is.  Sorry about the noise.  :-)

-- 
Pedro Alves