8140482: Various minor code improvements (runtime)

Reviewed-by: dholmes, coleenp, sspitsyn, dsamersoff

hotspot/agent/src/os/linux/libproc_impl.c

@@ -38,6 +38,7 @@ int pathmap_open(const char* name) {
   int fd;
   char alt_path[PATH_MAX + 1], *alt_path_end;
   const char *s;
+  int free_space;
 
   if (!alt_root_initialized) {
     alt_root_initialized = -1;
@@ -48,14 +49,22 @@ int pathmap_open(const char* name) {
     return open(name, O_RDONLY);
   }
 
-  strcpy(alt_path, alt_root);
-  alt_path_end = alt_path + strlen(alt_path);
 
-  // Strip path items one by one and try to open file with alt_root prepended
+  if (strlen(alt_root) + strlen(name) < PATH_MAX) {
+    // Buffer too small.
+    return -1;
+  }
+
+  strncpy(alt_path, alt_root, PATH_MAX);
+  alt_path[PATH_MAX] = '\0';
+  alt_path_end = alt_path + strlen(alt_path);
+  free_space = PATH_MAX + 1 - (alt_path_end-alt_path);
+
+  // Strip path items one by one and try to open file with alt_root prepended.
   s = name;
   while (1) {
-    strcat(alt_path, s);
+    strncat(alt_path, s, free_space);
-    s += 1;
+    s += 1;  // Skip /.
 
     fd = open(alt_path, O_RDONLY);
     if (fd >= 0) {
@@ -70,7 +79,8 @@ int pathmap_open(const char* name) {
       break;
     }
 
-    *alt_path_end = 0;
+    // Cut off what we appended above.
+    *alt_path_end = '\0';
   }
 
   return -1;

hotspot/agent/src/os/linux/ps_core.c

@@ -774,72 +774,78 @@ err:
 
 // process segments from interpreter (ld.so or ld-linux.so)
 static bool read_interp_segments(struct ps_prochandle* ph) {
-   ELF_EHDR interp_ehdr;
+  ELF_EHDR interp_ehdr;
 
-   if (read_elf_header(ph->core->interp_fd, &interp_ehdr) != true) {
+  if (read_elf_header(ph->core->interp_fd, &interp_ehdr) != true) {
-       print_debug("interpreter is not a valid ELF file\n");
+    print_debug("interpreter is not a valid ELF file\n");
-       return false;
+    return false;
-   }
+  }
 
-   if (read_lib_segments(ph, ph->core->interp_fd, &interp_ehdr, ph->core->ld_base_addr) != true) {
+  if (read_lib_segments(ph, ph->core->interp_fd, &interp_ehdr, ph->core->ld_base_addr) != true) {
-       print_debug("can't read segments of interpreter\n");
+    print_debug("can't read segments of interpreter\n");
-       return false;
+    return false;
-   }
+  }
 
-   return true;
+  return true;
 }
 
 // process segments of a a.out
 static bool read_exec_segments(struct ps_prochandle* ph, ELF_EHDR* exec_ehdr) {
-   int i = 0;
+  int i = 0;
-   ELF_PHDR* phbuf = NULL;
+  ELF_PHDR* phbuf = NULL;
-   ELF_PHDR* exec_php = NULL;
+  ELF_PHDR* exec_php = NULL;
 
-   if ((phbuf = read_program_header_table(ph->core->exec_fd, exec_ehdr)) == NULL)
+  if ((phbuf = read_program_header_table(ph->core->exec_fd, exec_ehdr)) == NULL) {
-      return false;
+    return false;
+  }
 
-   for (exec_php = phbuf, i = 0; i < exec_ehdr->e_phnum; i++) {
+  for (exec_php = phbuf, i = 0; i < exec_ehdr->e_phnum; i++) {
-      switch (exec_php->p_type) {
+    switch (exec_php->p_type) {
 
-         // add mappings for PT_LOAD segments
+      // add mappings for PT_LOAD segments
-         case PT_LOAD: {
+    case PT_LOAD: {
-            // add only non-writable segments of non-zero filesz
+      // add only non-writable segments of non-zero filesz
-            if (!(exec_php->p_flags & PF_W) && exec_php->p_filesz != 0) {
+      if (!(exec_php->p_flags & PF_W) && exec_php->p_filesz != 0) {
-               if (add_map_info(ph, ph->core->exec_fd, exec_php->p_offset, exec_php->p_vaddr, exec_php->p_filesz) == NULL) goto err;
+        if (add_map_info(ph, ph->core->exec_fd, exec_php->p_offset, exec_php->p_vaddr, exec_php->p_filesz) == NULL) goto err;
-            }
+      }
-            break;
+      break;
-         }
+    }
 
-         // read the interpreter and it's segments
+    // read the interpreter and it's segments
-         case PT_INTERP: {
+    case PT_INTERP: {
-            char interp_name[BUF_SIZE];
+      char interp_name[BUF_SIZE + 1];
 
-            pread(ph->core->exec_fd, interp_name, MIN(exec_php->p_filesz, BUF_SIZE), exec_php->p_offset);
+      // BUF_SIZE is PATH_MAX + NAME_MAX + 1.
-            print_debug("ELF interpreter %s\n", interp_name);
+      if (exec_php->p_filesz > BUF_SIZE) {
-            // read interpreter segments as well
+        goto err;
-            if ((ph->core->interp_fd = pathmap_open(interp_name)) < 0) {
+      }
-               print_debug("can't open runtime loader\n");
+      pread(ph->core->exec_fd, interp_name, exec_php->p_filesz, exec_php->p_offset);
-               goto err;
+      interp_name[exec_php->p_filesz] = '\0';
-            }
+      print_debug("ELF interpreter %s\n", interp_name);
-            break;
+      // read interpreter segments as well
-         }
+      if ((ph->core->interp_fd = pathmap_open(interp_name)) < 0) {
+        print_debug("can't open runtime loader\n");
+        goto err;
+      }
+      break;
+    }
 
-         // from PT_DYNAMIC we want to read address of first link_map addr
+    // from PT_DYNAMIC we want to read address of first link_map addr
-         case PT_DYNAMIC: {
+    case PT_DYNAMIC: {
-            ph->core->dynamic_addr = exec_php->p_vaddr;
+      ph->core->dynamic_addr = exec_php->p_vaddr;
-            print_debug("address of _DYNAMIC is 0x%lx\n", ph->core->dynamic_addr);
+      print_debug("address of _DYNAMIC is 0x%lx\n", ph->core->dynamic_addr);
-            break;
+      break;
-         }
+    }
 
-      } // switch
+    } // switch
-      exec_php++;
+    exec_php++;
-   } // for
+  } // for
 
-   free(phbuf);
+  free(phbuf);
-   return true;
+  return true;
-err:
+ err:
-   free(phbuf);
+  free(phbuf);
-   return false;
+  return false;
 }
 
 

hotspot/src/cpu/x86/vm/stubRoutines_x86.cpp

@@ -147,7 +147,7 @@ uint32_t crc32c_multiply(uint32_t a, uint32_t b) {
   b_pow_x_table[0] = b;
   for (int k = 0; k < D; ++k) {
     // If "a" has non-zero coefficient at x**k,/ add ((b * x**k) mod P) to the result.
-    if ((a & (uint64_t)(1 << (D - 1 - k))) != 0) product ^= b_pow_x_table[k];
+    if ((a & (((uint32_t)1) << (D - 1 - k))) != 0) product ^= b_pow_x_table[k];
 
     // Compute b_pow_x_table[k+1] = (b ** x**(k+1)) mod P.
     if (b_pow_x_table[k] & 1) {

hotspot/src/cpu/x86/vm/templateTable_x86.cpp

@@ -1611,7 +1611,7 @@ static jlong double_signflip_pool[2*2];
 void TemplateTable::fneg() {
   transition(ftos, ftos);
   if (UseSSE >= 1) {
-    static jlong *float_signflip  = double_quadword(&float_signflip_pool[1], 0x8000000080000000, 0x8000000080000000);
+    static jlong *float_signflip  = double_quadword(&float_signflip_pool[1],  CONST64(0x8000000080000000),  CONST64(0x8000000080000000));
     __ xorps(xmm0, ExternalAddress((address) float_signflip));
   } else {
     LP64_ONLY(ShouldNotReachHere());
@@ -1622,7 +1622,8 @@ void TemplateTable::fneg() {
 void TemplateTable::dneg() {
   transition(dtos, dtos);
   if (UseSSE >= 2) {
-    static jlong *double_signflip  = double_quadword(&double_signflip_pool[1], 0x8000000000000000, 0x8000000000000000);
+    static jlong *double_signflip =
+      double_quadword(&double_signflip_pool[1], CONST64(0x8000000000000000), CONST64(0x8000000000000000));
     __ xorpd(xmm0, ExternalAddress((address) double_signflip));
   } else {
 #ifdef _LP64

hotspot/src/os/linux/vm/attachListener_linux.cpp

@@ -254,6 +254,8 @@ LinuxAttachOperation* LinuxAttachListener::read_request(int s) {
   do {
     int n;
     RESTARTABLE(read(s, buf+off, left), n);
+    assert(n <= left, "buffer was too small, impossible!");
+    buf[max_len - 1] = '\0';
     if (n == -1) {
       return NULL;      // reset by peer or other error
     }

hotspot/src/os/linux/vm/os_linux.cpp

@@ -4252,7 +4252,9 @@ int os::Linux::get_our_sigflags(int sig) {
 
 void os::Linux::set_our_sigflags(int sig, int flags) {
   assert(sig > 0 && sig < MAXSIGNUM, "vm signal out of expected range");
-  sigflags[sig] = flags;
+  if (sig > 0 && sig < MAXSIGNUM) {
+    sigflags[sig] = flags;
+  }
 }
 
 void os::Linux::set_signal_handler(int sig, bool set_installed) {
@@ -5927,22 +5929,20 @@ int os::get_core_path(char* buffer, size_t bufferSize) {
   char core_pattern[core_pattern_len] = {0};
 
   int core_pattern_file = ::open("/proc/sys/kernel/core_pattern", O_RDONLY);
-  if (core_pattern_file != -1) {
+  if (core_pattern_file == -1) {
-    ssize_t ret = ::read(core_pattern_file, core_pattern, core_pattern_len);
+    return -1;
-    ::close(core_pattern_file);
-
-    if (ret > 0) {
-      char *last_char = core_pattern + strlen(core_pattern) - 1;
-
-      if (*last_char == '\n') {
-        *last_char = '\0';
-      }
-    }
   }
 
-  if (strlen(core_pattern) == 0) {
+  ssize_t ret = ::read(core_pattern_file, core_pattern, core_pattern_len);
+  ::close(core_pattern_file);
+  if (ret <= 0 || ret >= core_pattern_len || core_pattern[0] == '\n') {
     return -1;
   }
+  if (core_pattern[ret-1] == '\n') {
+    core_pattern[ret-1] = '\0';
+  } else {
+    core_pattern[ret] = '\0';
+  }
 
   char *pid_pos = strstr(core_pattern, "%p");
   int written;

hotspot/src/os/windows/vm/attachListener_windows.cpp

@@ -191,7 +191,8 @@ int Win32AttachListener::enqueue(char* cmd, char* arg0, char* arg1, char* arg2,
   // check that all paramteres to the operation
   if (strlen(cmd) > AttachOperation::name_length_max) return ATTACH_ERROR_ILLEGALARG;
   if (strlen(arg0) > AttachOperation::arg_length_max) return ATTACH_ERROR_ILLEGALARG;
-  if (strlen(arg0) > AttachOperation::arg_length_max) return ATTACH_ERROR_ILLEGALARG;
+  if (strlen(arg1) > AttachOperation::arg_length_max) return ATTACH_ERROR_ILLEGALARG;
+  if (strlen(arg2) > AttachOperation::arg_length_max) return ATTACH_ERROR_ILLEGALARG;
   if (strlen(pipename) > Win32AttachOperation::pipe_name_max) return ATTACH_ERROR_ILLEGALARG;
 
   // check for a well-formed pipename

hotspot/src/share/vm/asm/codeBuffer.cpp

@@ -873,6 +873,7 @@ void CodeBuffer::expand(CodeSection* which_cs, csize_t amount) {
 
   // Figure new capacity for each section.
   csize_t new_capacity[SECT_LIMIT];
+  memset(new_capacity, 0, sizeof(csize_t) * SECT_LIMIT);
   csize_t new_total_cap
     = figure_expanded_capacities(which_cs, amount, new_capacity);
 

hotspot/src/share/vm/libadt/dict.cpp

@@ -126,37 +126,37 @@ void Dict::Clear() {
 void Dict::doubhash(void) {
   uint oldsize = _size;
   _size <<= 1;                  // Double in size
-  _bin = (bucket*)_arena->Arealloc( _bin, sizeof(bucket)*oldsize, sizeof(bucket)*_size );
+  _bin = (bucket*)_arena->Arealloc(_bin, sizeof(bucket) * oldsize, sizeof(bucket) * _size);
-  memset( &_bin[oldsize], 0, oldsize*sizeof(bucket) );
+  memset(&_bin[oldsize], 0, oldsize * sizeof(bucket));
   // Rehash things to spread into new table
-  for( uint i=0; i < oldsize; i++) { // For complete OLD table do
+  for (uint i = 0; i < oldsize; i++) { // For complete OLD table do
-    bucket *b = &_bin[i];       // Handy shortcut for _bin[i]
+    bucket *b = &_bin[i];              // Handy shortcut for _bin[i]
-    if( !b->_keyvals ) continue;        // Skip empties fast
+    if (!b->_keyvals) continue;        // Skip empties fast
 
-    bucket *nb = &_bin[i+oldsize];  // New bucket shortcut
+    bucket *nb = &_bin[i+oldsize];     // New bucket shortcut
-    uint j = b->_max;               // Trim new bucket to nearest power of 2
+    uint j = b->_max;                  // Trim new bucket to nearest power of 2
-    while( j > b->_cnt ) j >>= 1;   // above old bucket _cnt
+    while (j > b->_cnt) { j >>= 1; }   // above old bucket _cnt
-    if( !j ) j = 1;             // Handle zero-sized buckets
+    if (!j) { j = 1; }                 // Handle zero-sized buckets
-    nb->_max = j<<1;
+    nb->_max = j << 1;
     // Allocate worst case space for key-value pairs
-    nb->_keyvals = (void**)_arena->Amalloc_4( sizeof(void *)*nb->_max*2 );
+    nb->_keyvals = (void**)_arena->Amalloc_4(sizeof(void *) * nb->_max * 2);
     uint nbcnt = 0;
 
-    for( j=0; j<b->_cnt; j++ ) {  // Rehash all keys in this bucket
+    for (j = 0; j < b->_cnt; ) {           // Rehash all keys in this bucket
-      void *key = b->_keyvals[j+j];
+      void *key = b->_keyvals[j + j];
-      if( (_hash( key ) & (_size-1)) != i ) { // Moving to hi bucket?
+      if ((_hash(key) & (_size-1)) != i) { // Moving to hi bucket?
-        nb->_keyvals[nbcnt+nbcnt] = key;
+        nb->_keyvals[nbcnt + nbcnt] = key;
-        nb->_keyvals[nbcnt+nbcnt+1] = b->_keyvals[j+j+1];
+        nb->_keyvals[nbcnt + nbcnt + 1] = b->_keyvals[j + j + 1];
-        nb->_cnt = nbcnt = nbcnt+1;
+        nb->_cnt = nbcnt = nbcnt + 1;
-        b->_cnt--;              // Remove key/value from lo bucket
+        b->_cnt--;                         // Remove key/value from lo bucket
-        b->_keyvals[j+j  ] = b->_keyvals[b->_cnt+b->_cnt  ];
+        b->_keyvals[j + j] = b->_keyvals[b->_cnt + b->_cnt];
-        b->_keyvals[j+j+1] = b->_keyvals[b->_cnt+b->_cnt+1];
+        b->_keyvals[j + j + 1] = b->_keyvals[b->_cnt + b->_cnt + 1];
-        j--;                    // Hash compacted element also
+        // Don't increment j, hash compacted element also.
+      } else {
+        j++; // Iterate.
       }
     } // End of for all key-value pairs in bucket
   } // End of for all buckets
-
-
 }
 
 //------------------------------Dict-----------------------------------------

hotspot/src/share/vm/runtime/deoptimization.cpp

@@ -2062,6 +2062,7 @@ int Deoptimization::trap_state_set_recompiled(int trap_state, bool z) {
 // This is used for debugging and diagnostics, including LogFile output.
 const char* Deoptimization::format_trap_state(char* buf, size_t buflen,
                                               int trap_state) {
+  assert(buflen > 0, "sanity");
   DeoptReason reason      = trap_state_reason(trap_state);
   bool        recomp_flag = trap_state_is_recompiled(trap_state);
   // Re-encode the state from its decoded components.
@@ -2082,8 +2083,6 @@ const char* Deoptimization::format_trap_state(char* buf, size_t buflen,
                        trap_reason_name(reason),
                        recomp_flag ? " recompiled" : "");
   }
-  if (len >= buflen)
-    buf[buflen-1] = '\0';
   return buf;
 }
 
@@ -2178,8 +2177,6 @@ const char* Deoptimization::format_trap_request(char* buf, size_t buflen,
 #endif
                        );
   }
-  if (len >= buflen)
-    buf[buflen-1] = '\0';
   return buf;
 }
 

hotspot/src/share/vm/runtime/task.cpp

@@ -117,8 +117,9 @@ void PeriodicTask::enroll() {
 
   if (_num_tasks == PeriodicTask::max_tasks) {
     fatal("Overflow in PeriodicTask table");
+  } else {
+    _tasks[_num_tasks++] = this;
   }
-  _tasks[_num_tasks++] = this;
 
   WatcherThread* thread = WatcherThread::watcher_thread();
   if (thread != NULL) {

hotspot/src/share/vm/services/attachListener.hpp

@@ -29,6 +29,7 @@
 #include "utilities/debug.hpp"
 #include "utilities/ostream.hpp"
 #include "utilities/macros.hpp"
+#include "utilities/globalDefinitions.hpp"
 
 // The AttachListener thread services a queue of operations that are enqueued
 // by client tools. Each operation is identified by a name and has up to 3
@@ -121,8 +122,9 @@ class AttachOperation: public CHeapObj<mtInternal> {
 
   // set the operation name
   void set_name(char* name) {
-    assert(strlen(name) <= name_length_max, "exceeds maximum name length");
+    size_t len = strlen(name);
-    strcpy(_name, name);
+    assert(len <= name_length_max, "exceeds maximum name length");
+    memcpy(_name, name, MIN2(len + 1, (size_t)name_length_max));
   }
 
   // get an argument value
@@ -137,8 +139,9 @@ class AttachOperation: public CHeapObj<mtInternal> {
     if (arg == NULL) {
       _arg[i][0] = '\0';
     } else {
-      assert(strlen(arg) <= arg_length_max, "exceeds maximum argument length");
+      size_t len = strlen(arg);
-      strcpy(_arg[i], arg);
+      assert(len <= arg_length_max, "exceeds maximum argument length");
+      memcpy(_arg[i], arg, MIN2(len + 1, (size_t)arg_length_max));
     }
   }
 

hotspot/src/share/vm/services/heapDumper.cpp

@@ -1973,7 +1973,7 @@ void HeapDumper::dump_heap(bool oome) {
     if (HeapDumpPath == NULL || HeapDumpPath[0] == '\0') {
       // HeapDumpPath=<file> not specified
     } else {
-      strncpy(base_path, HeapDumpPath, sizeof(base_path));
+      strcpy(base_path, HeapDumpPath);
       // check if the path is a directory (must exist)
       DIR* dir = os::opendir(base_path);
       if (dir == NULL) {

hotspot/src/share/vm/services/memoryService.cpp

@@ -546,7 +546,7 @@ Handle MemoryService::create_MemoryUsage_obj(MemoryUsage usage, TRAPS) {
                           CHECK_NH);
   return obj;
 }
-//
+
 // GC manager type depends on the type of Generation. Depending on the space
 // availability and vm options the gc uses major gc manager or minor gc
 // manager or both. The type of gc manager depends on the generation kind.
@@ -559,21 +559,23 @@ TraceMemoryManagerStats::TraceMemoryManagerStats(Generation::Name kind, GCCause:
 #if INCLUDE_ALL_GCS
     case Generation::ParNew:
 #endif // INCLUDE_ALL_GCS
-      _fullGC=false;
+      _fullGC = false;
       break;
     case Generation::MarkSweepCompact:
 #if INCLUDE_ALL_GCS
     case Generation::ConcurrentMarkSweep:
 #endif // INCLUDE_ALL_GCS
-      _fullGC=true;
+      _fullGC = true;
       break;
     default:
+      _fullGC = false;
       assert(false, "Unrecognized gc generation kind.");
   }
   // this has to be called in a stop the world pause and represent
   // an entire gc pause, start to finish:
-  initialize(_fullGC, cause,true, true, true, true, true, true, true);
+  initialize(_fullGC, cause, true, true, true, true, true, true, true);
 }
+
 TraceMemoryManagerStats::TraceMemoryManagerStats(bool fullGC,
                                                  GCCause::Cause cause,
                                                  bool recordGCBeginTime,
@@ -583,7 +585,7 @@ TraceMemoryManagerStats::TraceMemoryManagerStats(bool fullGC,
                                                  bool recordAccumulatedGCTime,
                                                  bool recordGCEndTime,
                                                  bool countCollection) {
-    initialize(fullGC, cause, recordGCBeginTime, recordPreGCUsage, recordPeakUsage,
+  initialize(fullGC, cause, recordGCBeginTime, recordPreGCUsage, recordPeakUsage,
              recordPostGCUsage, recordAccumulatedGCTime, recordGCEndTime,
              countCollection);
 }

hotspot/src/share/vm/utilities/xmlstream.cpp

@@ -346,13 +346,16 @@ PRAGMA_FORMAT_NONLITERAL_IGNORED
 // ------------------------------------------------------------------
 void xmlStream::va_done(const char* format, va_list ap) {
   char buffer[200];
-  guarantee(strlen(format) + 10 < sizeof(buffer), "bigger format buffer");
+  size_t format_len = strlen(format);
+  guarantee(format_len + 10 < sizeof(buffer), "bigger format buffer");
   const char* kind = format;
   const char* kind_end = strchr(kind, ' ');
-  size_t kind_len = (kind_end != NULL) ? (kind_end - kind) : strlen(kind);
+  size_t kind_len = (kind_end != NULL) ? (kind_end - kind) : format_len;
   strncpy(buffer, kind, kind_len);
   strcpy(buffer + kind_len, "_done");
-  strcat(buffer, format + kind_len);
+  if (kind_end != NULL) {
+    strncat(buffer, format + kind_len, sizeof(buffer) - (kind_len + 5 /* _done */) - 1);
+  }
   // Output the trailing event with the timestamp.
   va_begin_elem(buffer, ap);
   stamp();