8025250: SA: Sync linux and bsd versions of ps_core file

Summary: linux/ps_core.c and bsd/ps_core.c share most of code, but it has different formatting, comments etc.
Reviewed-by: sla, minqi

agent/src/os/bsd/ps_core.c

@@ -44,6 +44,7 @@
 // close all file descriptors
 static void close_files(struct ps_prochandle* ph) {
   lib_info* lib = NULL;
+
   // close core file descriptor
   if (ph->core->core_fd >= 0)
     close(ph->core->core_fd);
@@ -149,8 +150,7 @@ static map_info* add_class_share_map_info(struct ps_prochandle* ph, off_t offset
 
 // Return the map_info for the given virtual address.  We keep a sorted
 // array of pointers in ph->map_array, so we can binary search.
-static map_info* core_lookup(struct ps_prochandle *ph, uintptr_t addr)
-{
+static map_info* core_lookup(struct ps_prochandle *ph, uintptr_t addr) {
   int mid, lo = 0, hi = ph->core->num_maps - 1;
   map_info *mp;
 
@@ -230,9 +230,9 @@ struct FileMapHeader {
     size_t _used;            // for setting space top on read
 
     // 4991491 NOTICE These are C++ bool's in filemap.hpp and must match up with
-    // the C type matching the C++ bool type on any given platform. For
-    // Hotspot on BSD we assume the corresponding C type is char but
-    // licensees on BSD versions may need to adjust the type of these fields.
+    // the C type matching the C++ bool type on any given platform.
+    // We assume the corresponding C type is char but licensees
+    // may need to adjust the type of these fields.
     char   _read_only;       // read only space?
     char   _allow_exec;      // executable code in space?
 
@@ -286,10 +286,12 @@ static bool read_string(struct ps_prochandle* ph, uintptr_t addr, char* buf, siz
 #define USE_SHARED_SPACES_SYM "_UseSharedSpaces"
 // mangled name of Arguments::SharedArchivePath
 #define SHARED_ARCHIVE_PATH_SYM "_ZN9Arguments17SharedArchivePathE"
+#define LIBJVM_NAME "/libjvm.dylib"
 #else
 #define USE_SHARED_SPACES_SYM "UseSharedSpaces"
 // mangled name of Arguments::SharedArchivePath
 #define SHARED_ARCHIVE_PATH_SYM "__ZN9Arguments17SharedArchivePathE"
+#define LIBJVM_NAME "/libjvm.so"
 #endif // __APPLE_
 
 static bool init_classsharing_workaround(struct ps_prochandle* ph) {
@@ -300,12 +302,7 @@ static bool init_classsharing_workaround(struct ps_prochandle* ph) {
     // we are iterating over shared objects from the core dump. look for
     // libjvm.so.
     const char *jvm_name = 0;
-#ifdef __APPLE__
-    if ((jvm_name = strstr(lib->name, "/libjvm.dylib")) != 0)
-#else
-    if ((jvm_name = strstr(lib->name, "/libjvm.so")) != 0)
-#endif // __APPLE__
-    {
+    if ((jvm_name = strstr(lib->name, LIBJVM_NAME)) != 0) {
       char classes_jsa[PATH_MAX];
       struct FileMapHeader header;
       int fd = -1;
@@ -1121,7 +1118,7 @@ static bool read_core_segments(struct ps_prochandle* ph, ELF_EHDR* core_ehdr) {
     * contains a set of saved /proc structures), and PT_LOAD (which
     * represents a memory mapping from the process's address space).
     *
-    * Difference b/w Solaris PT_NOTE and BSD PT_NOTE:
+    * Difference b/w Solaris PT_NOTE and Linux/BSD PT_NOTE:
     *
     *     In Solaris there are two PT_NOTE segments the first PT_NOTE (if present)
     *     contains /proc structs in the pre-2.6 unstructured /proc format. the last
@@ -1171,17 +1168,46 @@ static bool read_lib_segments(struct ps_prochandle* ph, int lib_fd, ELF_EHDR* li
   ELF_PHDR* phbuf;
   ELF_PHDR* lib_php = NULL;
 
-   if ((phbuf = read_program_header_table(lib_fd, lib_ehdr)) == NULL)
+  int page_size=sysconf(_SC_PAGE_SIZE);
+
+  if ((phbuf = read_program_header_table(lib_fd, lib_ehdr)) == NULL) {
     return false;
+  }
 
   // we want to process only PT_LOAD segments that are not writable.
   // i.e., text segments. The read/write/exec (data) segments would
   // have been already added from core file segments.
   for (lib_php = phbuf, i = 0; i < lib_ehdr->e_phnum; i++) {
     if ((lib_php->p_type == PT_LOAD) && !(lib_php->p_flags & PF_W) && (lib_php->p_filesz != 0)) {
-         if (add_map_info(ph, lib_fd, lib_php->p_offset, lib_php->p_vaddr + lib_base, lib_php->p_filesz) == NULL)
+
+      uintptr_t target_vaddr = lib_php->p_vaddr + lib_base;
+      map_info *existing_map = core_lookup(ph, target_vaddr);
+
+      if (existing_map == NULL){
+        if (add_map_info(ph, lib_fd, lib_php->p_offset,
+                          target_vaddr, lib_php->p_filesz) == NULL) {
           goto err;
         }
+      } else {
+        if ((existing_map->memsz != page_size) &&
+            (existing_map->fd != lib_fd) &&
+            (existing_map->memsz != lib_php->p_filesz)){
+
+          print_debug("address conflict @ 0x%lx (size = %ld, flags = %d\n)",
+                        target_vaddr, lib_php->p_filesz, lib_php->p_flags);
+          goto err;
+        }
+
+        /* replace PT_LOAD segment with library segment */
+        print_debug("overwrote with new address mapping (memsz %ld -> %ld)\n",
+                     existing_map->memsz, lib_php->p_filesz);
+
+        existing_map->fd = lib_fd;
+        existing_map->offset = lib_php->p_offset;
+        existing_map->memsz = lib_php->p_filesz;
+      }
+    }
+
     lib_php++;
   }
 
@@ -1192,7 +1218,7 @@ err:
   return false;
 }
 
-// process segments from interpreter (ld-elf.so.1)
+// process segments from interpreter (ld.so or ld-linux.so or ld-elf.so)
 static bool read_interp_segments(struct ps_prochandle* ph) {
    ELF_EHDR interp_ehdr;
 
@@ -1309,26 +1335,28 @@ static bool read_shared_lib_info(struct ps_prochandle* ph) {
   }
 
   // read ld_base address from struct r_debug
-  // XXX: There is no r_ldbase member on BSD
-  /*
+#if 0  // There is no r_ldbase member on BSD
   if (ps_pread(ph, (psaddr_t) debug_base + LD_BASE_OFFSET, &ld_base_addr,
                   sizeof(uintptr_t)) != PS_OK) {
     print_debug("can't read ld base address\n");
     return false;
   }
   ph->core->ld_base_addr = ld_base_addr;
-  */
+#else
   ph->core->ld_base_addr = 0;
+#endif
 
   print_debug("interpreter base address is 0x%lx\n", ld_base_addr);
 
-  // now read segments from interp (i.e ld-elf.so.1)
-  if (read_interp_segments(ph) != true)
+  // now read segments from interp (i.e ld.so or ld-linux.so or ld-elf.so)
+  if (read_interp_segments(ph) != true) {
     return false;
+  }
 
   // after adding interpreter (ld.so) mappings sort again
-  if (sort_map_array(ph) != true)
+  if (sort_map_array(ph) != true) {
     return false;
+  }
 
   print_debug("first link map is at 0x%lx\n", first_link_map_addr);
 
@@ -1380,8 +1408,9 @@ static bool read_shared_lib_info(struct ps_prochandle* ph) {
           add_lib_info_fd(ph, lib_name, lib_fd, lib_base);
           // Map info is added for the library (lib_name) so
           // we need to re-sort it before calling the p_pdread.
-          if (sort_map_array(ph) != true)
+          if (sort_map_array(ph) != true) {
             return false;
+          }
         } else {
           print_debug("can't read ELF header for shared object %s\n", lib_name);
           close(lib_fd);
@@ -1408,7 +1437,7 @@ struct ps_prochandle* Pgrab_core(const char* exec_file, const char* core_file) {
 
   struct ps_prochandle* ph = (struct ps_prochandle*) calloc(1, sizeof(struct ps_prochandle));
   if (ph == NULL) {
-    print_debug("cant allocate ps_prochandle\n");
+    print_debug("can't allocate ps_prochandle\n");
     return NULL;
   }
 
@@ -1449,33 +1478,40 @@ struct ps_prochandle* Pgrab_core(const char* exec_file, const char* core_file) {
   }
 
   // process core file segments
-  if (read_core_segments(ph, &core_ehdr) != true)
+  if (read_core_segments(ph, &core_ehdr) != true) {
     goto err;
+  }
 
   // process exec file segments
-  if (read_exec_segments(ph, &exec_ehdr) != true)
+  if (read_exec_segments(ph, &exec_ehdr) != true) {
     goto err;
+  }
 
   // exec file is also treated like a shared object for symbol search
   if (add_lib_info_fd(ph, exec_file, ph->core->exec_fd,
-                      (uintptr_t)0 + find_base_address(ph->core->exec_fd, &exec_ehdr)) == NULL)
+                      (uintptr_t)0 + find_base_address(ph->core->exec_fd, &exec_ehdr)) == NULL) {
     goto err;
+  }
 
   // allocate and sort maps into map_array, we need to do this
   // here because read_shared_lib_info needs to read from debuggee
   // address space
-  if (sort_map_array(ph) != true)
+  if (sort_map_array(ph) != true) {
     goto err;
+  }
 
-  if (read_shared_lib_info(ph) != true)
+  if (read_shared_lib_info(ph) != true) {
     goto err;
+  }
 
   // sort again because we have added more mappings from shared objects
-  if (sort_map_array(ph) != true)
+  if (sort_map_array(ph) != true) {
     goto err;
+  }
 
-  if (init_classsharing_workaround(ph) != true)
+  if (init_classsharing_workaround(ph) != true) {
     goto err;
+  }
 
   print_debug("Leave Pgrab_core\n");
   return ph;

agent/src/os/linux/ps_core.c

@@ -41,7 +41,7 @@
 // ps_prochandle cleanup helper functions
 
 // close all file descriptors
-static void close_elf_files(struct ps_prochandle* ph) {
+static void close_files(struct ps_prochandle* ph) {
   lib_info* lib = NULL;
 
   // close core file descriptor
@@ -64,7 +64,9 @@ static void close_elf_files(struct ps_prochandle* ph) {
   lib = ph->libs;
   while (lib) {
     int fd = lib->fd;
-      if (fd >= 0 && fd != ph->core->exec_fd) close(fd);
+    if (fd >= 0 && fd != ph->core->exec_fd) {
+      close(fd);
+    }
     lib = lib->next;
   }
 }
@@ -94,7 +96,7 @@ static void destroy_map_info(struct ps_prochandle* ph) {
 // ps_prochandle operations
 static void core_release(struct ps_prochandle* ph) {
   if (ph->core) {
-      close_elf_files(ph);
+    close_files(ph);
     destroy_map_info(ph);
     free(ph->core);
   }
@@ -132,40 +134,43 @@ static map_info* add_map_info(struct ps_prochandle* ph, int fd, off_t offset,
 }
 
 // Part of the class sharing workaround
-static void add_class_share_map_info(struct ps_prochandle* ph, off_t offset,
+static map_info* add_class_share_map_info(struct ps_prochandle* ph, off_t offset,
                              uintptr_t vaddr, size_t memsz) {
   map_info* map;
   if ((map = allocate_init_map(ph->core->classes_jsa_fd,
                                offset, vaddr, memsz)) == NULL) {
-      return;
+    return NULL;
   }
 
   map->next = ph->core->class_share_maps;
   ph->core->class_share_maps = map;
+  return map;
 }
 
 // Return the map_info for the given virtual address.  We keep a sorted
 // array of pointers in ph->map_array, so we can binary search.
-static map_info* core_lookup(struct ps_prochandle *ph, uintptr_t addr)
-{
+static map_info* core_lookup(struct ps_prochandle *ph, uintptr_t addr) {
   int mid, lo = 0, hi = ph->core->num_maps - 1;
   map_info *mp;
 
   while (hi - lo > 1) {
     mid = (lo + hi) / 2;
-      if (addr >= ph->core->map_array[mid]->vaddr)
+    if (addr >= ph->core->map_array[mid]->vaddr) {
       lo = mid;
-      else
+    } else {
       hi = mid;
     }
+  }
 
-   if (addr < ph->core->map_array[hi]->vaddr)
+  if (addr < ph->core->map_array[hi]->vaddr) {
     mp = ph->core->map_array[lo];
-   else
+  } else {
     mp = ph->core->map_array[hi];
+  }
 
-   if (addr >= mp->vaddr && addr < mp->vaddr + mp->memsz)
+  if (addr >= mp->vaddr && addr < mp->vaddr + mp->memsz) {
     return (mp);
+  }
 
 
   // Part of the class sharing workaround
@@ -176,13 +181,11 @@ static map_info* core_lookup(struct ps_prochandle *ph, uintptr_t addr)
   // want to prefer class sharing data to data from core.
   mp = ph->core->class_share_maps;
   if (mp) {
-      print_debug("can't locate map_info at 0x%lx, trying class share maps\n",
-             addr);
+    print_debug("can't locate map_info at 0x%lx, trying class share maps\n", addr);
   }
   while (mp) {
     if (addr >= mp->vaddr && addr < mp->vaddr + mp->memsz) {
-         print_debug("located map_info at 0x%lx from class share maps\n",
-                  addr);
+      print_debug("located map_info at 0x%lx from class share maps\n", addr);
       return (mp);
     }
     mp = mp->next;
@@ -226,9 +229,9 @@ struct FileMapHeader {
     size_t _used;            // for setting space top on read
 
     // 4991491 NOTICE These are C++ bool's in filemap.hpp and must match up with
-    // the C type matching the C++ bool type on any given platform. For
-    // Hotspot on Linux we assume the corresponding C type is char but
-    // licensees on Linux versions may need to adjust the type of these fields.
+    // the C type matching the C++ bool type on any given platform.
+    // We assume the corresponding C type is char but licensees
+    // may need to adjust the type of these fields.
     char   _read_only;       // read only space?
     char   _allow_exec;      // executable code in space?
 
@@ -249,7 +252,7 @@ static bool read_jboolean(struct ps_prochandle* ph, uintptr_t addr, jboolean* pv
 
 static bool read_pointer(struct ps_prochandle* ph, uintptr_t addr, uintptr_t* pvalue) {
   uintptr_t uip;
-   if (ps_pdread(ph, (psaddr_t) addr, &uip, sizeof(uip)) == PS_OK) {
+  if (ps_pdread(ph, (psaddr_t) addr, (char *)&uip, sizeof(uip)) == PS_OK) {
     *pvalue = uip;
     return true;
   } else {
@@ -263,12 +266,15 @@ static bool read_string(struct ps_prochandle* ph, uintptr_t addr, char* buf, siz
   char  c = ' ';
 
   while (c != '\0') {
-     if (ps_pdread(ph, (psaddr_t) addr, &c, sizeof(char)) != PS_OK)
+    if (ps_pdread(ph, (psaddr_t) addr, &c, sizeof(char)) != PS_OK) {
       return false;
-      if (i < size - 1)
+    }
+    if (i < size - 1) {
       buf[i] = c;
-      else // smaller buffer
+    } else {
+      // smaller buffer
       return false;
+    }
     i++; addr++;
   }
 
@@ -279,6 +285,7 @@ static bool read_string(struct ps_prochandle* ph, uintptr_t addr, char* buf, siz
 #define USE_SHARED_SPACES_SYM "UseSharedSpaces"
 // mangled name of Arguments::SharedArchivePath
 #define SHARED_ARCHIVE_PATH_SYM "_ZN9Arguments17SharedArchivePathE"
+#define LIBJVM_NAME "/libjvm.so"
 
 static bool init_classsharing_workaround(struct ps_prochandle* ph) {
   lib_info* lib = ph->libs;
@@ -286,11 +293,12 @@ static bool init_classsharing_workaround(struct ps_prochandle* ph) {
     // we are iterating over shared objects from the core dump. look for
     // libjvm.so.
     const char *jvm_name = 0;
-      if ((jvm_name = strstr(lib->name, "/libjvm.so")) != 0) {
+    if ((jvm_name = strstr(lib->name, LIBJVM_NAME)) != 0) {
       char classes_jsa[PATH_MAX];
       struct FileMapHeader header;
+      int fd = -1;
+      int m = 0;
       size_t n = 0;
-         int fd = -1, m = 0;
       uintptr_t base = 0, useSharedSpacesAddr = 0;
       uintptr_t sharedArchivePathAddrAddr = 0, sharedArchivePathAddr = 0;
       jboolean useSharedSpaces = 0;
@@ -399,8 +407,9 @@ static int core_cmp_mapping(const void *lhsp, const void *rhsp)
   const map_info *lhs = *((const map_info **)lhsp);
   const map_info *rhs = *((const map_info **)rhsp);
 
-   if (lhs->vaddr == rhs->vaddr)
+  if (lhs->vaddr == rhs->vaddr) {
     return (0);
+  }
 
   return (lhs->vaddr < rhs->vaddr ? -1 : 1);
 }
@@ -427,7 +436,10 @@ static bool sort_map_array(struct ps_prochandle* ph) {
   }
 
   // sort is called twice. If this is second time, clear map array
-   if (ph->core->map_array) free(ph->core->map_array);
+  if (ph->core->map_array) {
+    free(ph->core->map_array);
+  }
+
   ph->core->map_array = array;
   // sort the map_info array by base virtual address.
   qsort(ph->core->map_array, ph->core->num_maps, sizeof (map_info*),
@@ -460,16 +472,18 @@ static bool core_read_data(struct ps_prochandle* ph, uintptr_t addr, char *buf,
       off_t off;
       int fd;
 
-      if (mp == NULL)
+      if (mp == NULL) {
          break;  /* No mapping for this address */
+      }
 
       fd = mp->fd;
       mapoff = addr - mp->vaddr;
       len = MIN(resid, mp->memsz - mapoff);
       off = mp->offset + mapoff;
 
-      if ((len = pread(fd, buf, len, off)) <= 0)
+      if ((len = pread(fd, buf, len, off)) <= 0) {
          break;
+      }
 
       resid -= len;
       addr += len;
@@ -625,9 +639,10 @@ static bool core_handle_note(struct ps_prochandle* ph, ELF_PHDR* note_phdr) {
                                    notep->n_type, notep->n_descsz);
 
       if (notep->n_type == NT_PRSTATUS) {
-         if (core_handle_prstatus(ph, descdata, notep->n_descsz) != true)
+        if (core_handle_prstatus(ph, descdata, notep->n_descsz) != true) {
           return false;
         }
+      }
       p = descdata + ROUNDUP(notep->n_descsz, 4);
    }
 
@@ -654,7 +669,7 @@ static bool read_core_segments(struct ps_prochandle* ph, ELF_EHDR* core_ehdr) {
     * contains a set of saved /proc structures), and PT_LOAD (which
     * represents a memory mapping from the process's address space).
     *
-    * Difference b/w Solaris PT_NOTE and Linux PT_NOTE:
+    * Difference b/w Solaris PT_NOTE and Linux/BSD PT_NOTE:
     *
     *     In Solaris there are two PT_NOTE segments the first PT_NOTE (if present)
     *     contains /proc structs in the pre-2.6 unstructured /proc format. the last
@@ -674,7 +689,9 @@ static bool read_core_segments(struct ps_prochandle* ph, ELF_EHDR* core_ehdr) {
     for (core_php = phbuf, i = 0; i < core_ehdr->e_phnum; i++) {
       switch (core_php->p_type) {
          case PT_NOTE:
-            if (core_handle_note(ph, core_php) != true) goto err;
+            if (core_handle_note(ph, core_php) != true) {
+              goto err;
+            }
             break;
 
          case PT_LOAD: {
@@ -879,13 +896,15 @@ static bool read_shared_lib_info(struct ps_prochandle* ph) {
 
   print_debug("interpreter base address is 0x%lx\n", ld_base_addr);
 
-   // now read segments from interp (i.e ld.so or ld-linux.so)
-   if (read_interp_segments(ph) != true)
+  // now read segments from interp (i.e ld.so or ld-linux.so or ld-elf.so)
+  if (read_interp_segments(ph) != true) {
       return false;
+  }
 
   // after adding interpreter (ld.so) mappings sort again
-   if (sort_map_array(ph) != true)
+  if (sort_map_array(ph) != true) {
     return false;
+  }
 
    print_debug("first link map is at 0x%lx\n", first_link_map_addr);
 
@@ -1008,33 +1027,40 @@ struct ps_prochandle* Pgrab_core(const char* exec_file, const char* core_file) {
   }
 
   // process core file segments
-   if (read_core_segments(ph, &core_ehdr) != true)
+  if (read_core_segments(ph, &core_ehdr) != true) {
     goto err;
+  }
 
   // process exec file segments
-   if (read_exec_segments(ph, &exec_ehdr) != true)
+  if (read_exec_segments(ph, &exec_ehdr) != true) {
     goto err;
+  }
 
   // exec file is also treated like a shared object for symbol search
   if (add_lib_info_fd(ph, exec_file, ph->core->exec_fd,
-                       (uintptr_t)0 + find_base_address(ph->core->exec_fd, &exec_ehdr)) == NULL)
+                      (uintptr_t)0 + find_base_address(ph->core->exec_fd, &exec_ehdr)) == NULL) {
     goto err;
+  }
 
   // allocate and sort maps into map_array, we need to do this
   // here because read_shared_lib_info needs to read from debuggee
   // address space
-   if (sort_map_array(ph) != true)
+  if (sort_map_array(ph) != true) {
     goto err;
+  }
 
-   if (read_shared_lib_info(ph) != true)
+  if (read_shared_lib_info(ph) != true) {
     goto err;
+  }
 
   // sort again because we have added more mappings from shared objects
-   if (sort_map_array(ph) != true)
+  if (sort_map_array(ph) != true) {
     goto err;
+  }
 
-   if (init_classsharing_workaround(ph) != true)
+  if (init_classsharing_workaround(ph) != true) {
     goto err;
+  }
 
   return ph;