This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA/RFC Prec] Add Linux AMD64 process record support second version, (64 bits system call support) 2/3


Hui Zhu wrote:

@@ -80,6 +81,133 @@
 #define RECORD_Q_XGETQSTAT     (('5' << 8) + 5)
 #define RECORD_Q_XGETQUOTA     (('3' << 8) + 3)

+#define OUTPUT_REG(val, num)      phex_nz ((val), \
+    TYPE_LENGTH (gdbarch_register_type (get_regcache_arch (regcache), (num))))
+
+static int
+record_linux_sockaddr (struct regcache *regcache,
+                       struct linux_record_tdep *tdep, ULONGEST addr,
+                       ULONGEST len)
+{
+  gdb_byte *a;
+  int addrlen;
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+  if (!addr)
+    return 0;
+
+  a = alloca (tdep->size_int);
+
+  if (record_arch_list_add_mem ((CORE_ADDR) len, tdep->size_int))
+    return -1;
+
+  /* Get the addrlen.  */
+  if (target_read_memory ((CORE_ADDR) len, a, tdep->size_int))
+    {
+      if (record_debug)
+        fprintf_unfiltered (gdb_stdlog,
+                            "Process record: error reading "
+                            "memory at addr = 0x%s len = %d.\n",
+                            phex_nz (len, tdep->size_pointer),
+                            tdep->size_int);
+        return -1;
+    }
+  addrlen = (int) extract_unsigned_integer(a, tdep->size_int, byte_order);

Space between function name and left paren.



+  /* msg_iov msg_iovlen */
+  addr = extract_unsigned_integer (a, tdep->size_pointer, byte_order);
+  a += tdep->size_pointer;
+  if (addr)
+    {
+      ULONGEST i;
+      ULONGEST len = extract_unsigned_integer (a, tdep->size_size_t,
+                                               byte_order);
+      gdb_byte *iov = alloca (tdep->size_iovec);
+
+      for (i = 0; i < len; i++)
+        {
+          if (target_read_memory ((CORE_ADDR) addr, iov, tdep->size_iovec))
+            {
+              if (record_debug)
+                fprintf_unfiltered (gdb_stdlog,
+                                    "Process record: error "
+                                    "reading memory at "
+                                    "addr = 0x%s "
+                                    "len = %d.\n",
+                                    phex_nz (addr,tdep->size_pointer),
+                                    tdep->size_iovec);
+                return -1;
+            }
+          if (record_arch_list_add_mem ((CORE_ADDR) extract_unsigned_integer
+                                                      (iov, tdep->size_pointer,
+                                                       byte_order),
+                                        (int) extract_unsigned_integer
+                                                (iov + tdep->size_pointer,
+                                                 tdep->size_size_t,
+                                                 byte_order)))

This statement is so ugly and badly indented. It would be great if you could just use a couple of temporary variables, one CORE_ADDR and one int, and break up the line. Call extract_unsigned_integer first, and then record_arch_list_add_mem.


+            return -1;
+          addr += tdep->size_iovec;
+        }
+    }
+  a += tdep->size_size_t;
+
+  /* msg_control msg_controllen */
+  addr = extract_unsigned_integer (a, tdep->size_pointer, byte_order);
+  a += tdep->size_pointer;
+  if (record_arch_list_add_mem ((CORE_ADDR) addr,
+                                (int) extract_unsigned_integer
+                                        (a, tdep->size_size_t, byte_order)))

Same here, maybe use the same temporary variable.



+    case 514:
+      regcache_raw_read_unsigned (regcache, tdep->arg5, &tmpulongest);
+      if (tmpulongest)
+        {
+          ULONGEST optvalp;
+          gdb_byte *optlenp = alloca (tdep->size_int);
+          if (target_read_memory ((CORE_ADDR) tmpulongest, optlenp,
+                                  tdep->size_int))
+            {
+              if (record_debug)
+                fprintf_unfiltered (gdb_stdlog,
+                                    "Process record: error reading "
+                                    "memory at addr = 0x%s "
+                                    "len = %d.\n",
+                                    OUTPUT_REG (tmpulongest, tdep->arg5),
+                                    tdep->size_int);
+              return -1;
+            }
+          regcache_raw_read_unsigned (regcache, tdep->arg4, &optvalp);
+          if (record_arch_list_add_mem ((CORE_ADDR) optvalp,
+                                        (int) extract_signed_integer
+                                                (optlenp, tdep->size_int,
+                                                 byte_order)))

Another great place for a temporary variable, just to avoid having to break up such a long line so awkwardly.



+                tmpulongest += tdep->size_ulong;
+                if (target_read_memory ((CORE_ADDR) tmpulongest, a,
+                                        tdep->size_ulong * 2))
+                  {
+                    if (record_debug)
+                      fprintf_unfiltered (gdb_stdlog,
+                                          "Process record: error reading "
+                                          "memory at addr = 0x%s len = %d.\n",
+                                          OUTPUT_REG (tmpulongest, tdep->arg2),
+                                          tdep->size_ulong * 2);
+                    return -1;
+                  }
+                if (record_linux_sockaddr (regcache, tdep,
+                                           extract_unsigned_integer
+                                             (a, tdep->size_ulong, byte_order),
+                                           extract_unsigned_integer
+                                             (a + tdep->size_ulong,
+                                              tdep->size_ulong, byte_order)))

... and again, same thing.


+                  return -1;
+              }
+          }
+          break;

-           regcache_raw_read (regcache, tdep->arg2,
-                              (gdb_byte *) & tmpu32);
-           if (tmpu32)
-             {
-               if (target_read_memory (tmpu32, (gdb_byte *) a, sizeof (a)))
-                 {
-                   if (record_debug)
-                     fprintf_unfiltered (gdb_stdlog,
-                                         "Process record: error reading "
-                                         "memory at addr = %s len = %lu.\n",
-                                         paddress (gdbarch, tmpu32),
-                                         (unsigned long)sizeof (a));
-                   return -1;
-                 }
-               if (a[4])
-                 {
-                   if (target_read_memory
-                       (a[4], (gdb_byte *) & av, sizeof (av)))

How about: if (target_read_memory (a[4], gdb_byte *) & av, sizeof (av)))


+        case RECORD_SYS_SOCKETPAIR:
+          {
+            gdb_byte *a = alloca (tdep->size_ulong);
+            regcache_raw_read_unsigned (regcache, tdep->arg2,
+                                        &tmpulongest);
+            if (tmpulongest)
+              {
+                tmpulongest += tdep->size_ulong * 3;
+                if (target_read_memory ((CORE_ADDR) tmpulongest, a,
+                                        tdep->size_ulong))
+                  {
+                    if (record_debug)
+                      fprintf_unfiltered (gdb_stdlog,
+                                          "Process record: error reading "
+                                          "memory at addr = 0x%s len = %d.\n",
+                                          OUTPUT_REG (tmpulongest, tdep->arg2),
+                                          tdep->size_ulong);
+                    return -1;
+                  }
+                if (record_arch_list_add_mem ((CORE_ADDR)
extract_unsigned_integer
+                                                           (a,
+                                                            tdep->size_ulong,
+                                                            byte_order),
+                                              tdep->size_int))

And another great place for a temp variable.



+              tmpulongest += tdep->size_ulong * 4;
+              if (target_read_memory ((CORE_ADDR) tmpulongest, a,
+                                      tdep->size_ulong * 2))
+                {
+                  if (record_debug)
+                    fprintf_unfiltered (gdb_stdlog,
+                                        "Process record: error reading "
+                                        "memory at addr = 0x%s len = %d.\n",
+                                        OUTPUT_REG (tmpulongest, tdep->arg2),
+                                        tdep->size_ulong * 2);
+                  return -1;
+                }
+              if (record_linux_sockaddr (regcache, tdep,
+                                         extract_unsigned_integer
+                                           (a, tdep->size_ulong, byte_order),
+                                         extract_unsigned_integer
+                                           (a + tdep->size_ulong,
+                                            tdep->size_ulong, byte_order)))
+                return -1;
+            }
+        case RECORD_SYS_RECV:

And again...


+          regcache_raw_read_unsigned (regcache, tdep->arg2,
+                                      &tmpulongest);
+          if (tmpulongest)
+            {
+              gdb_byte *a = alloca (tdep->size_ulong * 2);
+
+              tmpulongest += tdep->size_ulong;
+              if (target_read_memory ((CORE_ADDR) tmpulongest, a,
+                                      tdep->size_ulong))
+                {
+                  if (record_debug)
+                    fprintf_unfiltered (gdb_stdlog,
+                                        "Process record: error reading "
+                                        "memory at addr = 0x%s len = %d.\n",
+                                        OUTPUT_REG (tmpulongest, tdep->arg2),
+                                        tdep->size_ulong);
+                    return -1;
+                }
+              tmpulongest = extract_unsigned_integer (a, tdep->size_ulong,
+                                                      byte_order);
+              if (tmpulongest)
+                {
+                  a += tdep->size_ulong;
+                  if (record_arch_list_add_mem
+                        ((CORE_ADDR) tmpulongest,
+                         (int) extract_unsigned_integer (a, tdep->size_ulong,
+                                                         byte_order)))
+                    return -1;
+                }
+            }
+          break;
+        case RECORD_SYS_SHUTDOWN:

And again....



+        case RECORD_SYS_SETSOCKOPT:
+          break;
+        case RECORD_SYS_GETSOCKOPT:
+          {
+            gdb_byte *a = alloca (tdep->size_ulong * 2);
+            gdb_byte *av = alloca (tdep->size_int);
+
+            regcache_raw_read_unsigned (regcache, tdep->arg2,
+                                        &tmpulongest);
+            if (tmpulongest)
+              {
+                tmpulongest += tdep->size_ulong * 3;
+                if (target_read_memory ((CORE_ADDR) tmpulongest, a,
+                                        tdep->size_ulong * 2))
+                  {
+                    if (record_debug)
+                      fprintf_unfiltered (gdb_stdlog,
+                                          "Process record: error reading "
+                                          "memory at addr = 0x%s len = %d.\n",
+                                          OUTPUT_REG (tmpulongest, tdep->arg2),
+                                          tdep->size_ulong * 2);
+                    return -1;
+                  }
+                tmpulongest = extract_unsigned_integer (a + tdep->size_ulong,
+                                                        tdep->size_ulong,
+                                                        byte_order);
+                if (tmpulongest)
+                  {
+                    if (target_read_memory ((CORE_ADDR) tmpulongest, av,
+                                            tdep->size_int))
+                      {
+                        if (record_debug)
+                          fprintf_unfiltered (gdb_stdlog,
+                                              "Process record: error reading "
+                                              "memory at addr = 0x%s "
+                                              "len = %d.\n",
+                                              phex_nz (tmpulongest,
+                                                       tdep->size_ulong),
+                                              tdep->size_int);
+                        return -1;
+                      }
+                    if (record_arch_list_add_mem
+                          ((CORE_ADDR) extract_unsigned_integer
+                                         (a, tdep->size_ulong, byte_order),
+                           (int) extract_unsigned_integer (av, tdep->size_int,
+                                                           byte_order)))

And again...



+                      return -1;
+                    a += tdep->size_ulong;
+                    if (record_arch_list_add_mem
+                          ((CORE_ADDR) extract_unsigned_integer
+                                         (a, tdep->size_ulong, byte_order),
+                           tdep->size_int))

And again...


+                      return -1;
+                  }
+              }
+          }
+          break;
+        case RECORD_SYS_SENDMSG:
+          break;
+        case RECORD_SYS_RECVMSG:
+          {
+            gdb_byte *a = alloca (tdep->size_ulong);
+
+            regcache_raw_read_unsigned (regcache, tdep->arg2,
+                                        &tmpulongest);
+            if (tmpulongest)
+              {
+                tmpulongest += tdep->size_ulong;
+                if (target_read_memory ((CORE_ADDR) tmpulongest, a,
+                                        tdep->size_ulong))
+                  {
+                    if (record_debug)
+                      fprintf_unfiltered (gdb_stdlog,
+                                          "Process record: error reading "
+                                          "memory at addr = 0x%s len = %d.\n",
+                                          OUTPUT_REG (tmpulongest, tdep->arg2),
+                                          tdep->size_ulong);
+                    return -1;
+                  }
+                tmpulongest = extract_unsigned_integer (a, tdep->size_ulong,
+                                                        byte_order);
+                if (record_linux_msghdr (regcache, tdep,
+                                         extract_unsigned_integer
+                                           (a, tdep->size_ulong, byte_order)))

And again...



@@ -1005,47 +1231,121 @@ record_linux_system_call (int num, struc

       /* sys_sysinfo */
     case 116:
-      regcache_raw_read (regcache, tdep->arg1, (gdb_byte *) & tmpu32);
-      if (record_arch_list_add_mem (tmpu32, tdep->size_sysinfo))
-       return -1;
+      regcache_raw_read_unsigned (regcache, tdep->arg1, &tmpulongest);
+      if (record_arch_list_add_mem ((CORE_ADDR) tmpulongest,
+                                    tdep->size_sysinfo))
+        return -1;
+      break;
+
+      /* sys_shmget */
+    case 520:
+      /* sys_semget */
+    case 523:
+      /* sys_semop */
+    case 524:
+      /* sys_msgget */
+    case 528:
+      /* sys_shmdt */
+      /* XXX maybe need do some record works wiht sys_shmdt.  */

"with"



+        ULONGEST vec, vlen;
+
+        regcache_raw_read_unsigned (regcache, tdep->arg2, &vec);
+        if (vec)
+          {
+            gdb_byte *iov = alloca (tdep->size_iovec);
+
+            regcache_raw_read_unsigned (regcache, tdep->arg3, &vlen);
+            for (tmpulongest = 0; tmpulongest < vlen; tmpulongest++)
+              {
+                if (target_read_memory ((CORE_ADDR) vec, iov,
+                                        tdep->size_iovec))
+                  {
+                    if (record_debug)
+                      fprintf_unfiltered (gdb_stdlog,
+                                          "Process record: error reading "
+                                          "memory at addr = 0x%s len = %d.\n",
+                                          OUTPUT_REG (vec, tdep->arg2),
+                                          tdep->size_iovec);
+                    return -1;
+                  }
+                if (record_arch_list_add_mem
+                      ((CORE_ADDR) extract_unsigned_integer
+                                     (iov, tdep->size_pointer, byte_order),
+                       (int) extract_unsigned_integer
+                               (iov + tdep->size_pointer, tdep->size_size_t,
+                                byte_order)))

And same thing...



+      regcache_raw_read_unsigned (regcache, tdep->arg3, &tmpulongest);
+      if (tmpulongest)
+        {
+          ULONGEST nr, i;
+          gdb_byte *iocbp;
+
+          regcache_raw_read_unsigned (regcache, tdep->arg2, &nr);
+          iocbp = alloca (nr * tdep->size_pointer);
+          if (target_read_memory ((CORE_ADDR) tmpulongest, iocbp,
+                                  nr * tdep->size_pointer))
+            {
+              if (record_debug)
+                fprintf_unfiltered (gdb_stdlog,
+                                    "Process record: error reading memory "
+                                    "at addr = 0x%s len = %u.\n",
+                                    OUTPUT_REG (tmpulongest, tdep->arg2),
+                                    (int) (nr * tdep->size_pointer));
+              return -1;
+            }
+          for (i = 0; i < nr; i++)
+            {
+              if (record_arch_list_add_mem
+                    ((CORE_ADDR) extract_unsigned_integer (iocbp,
+                                                           tdep->size_pointer,
+                                                           byte_order),
+                     tdep->size_iocb))

And again...



     case 319:
-      regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
-      if (tmpu32)
-       {
-         int32_t maxevents;
-         regcache_raw_read (regcache, tdep->arg3,
-                            (gdb_byte *) & maxevents);
-         if (record_arch_list_add_mem
-             (tmpu32, maxevents * tdep->size_epoll_event))
-           return -1;
-       }
+      regcache_raw_read_unsigned (regcache, tdep->arg2, &tmpulongest);
+      if (tmpulongest)
+        {
+          ULONGEST maxevents;
+          regcache_raw_read_unsigned (regcache, tdep->arg3, &maxevents);
+          if (record_arch_list_add_mem ((CORE_ADDR) tmpulongest,
+                                        (int) maxevents *
tdep->size_epoll_event))

I think you don't need the (int) cast here, and if you remove it, the line will be less than 80 chars.


Aside from those comments, this patch looks great!



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]