7017193: Small memory leak in get_stack_bounds os::create_stack_guard_pages

Getline() returns -1 but still allocate memory for str

Reviewed-by: dcubed, coleenp

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

@@ -2648,45 +2648,39 @@ bool os::uncommit_memory(char* addr, size_t size) {
 // writing thread stacks don't use growable mappings (i.e. those
 // creeated with MAP_GROWSDOWN), and aren't marked "[stack]", so this
 // only applies to the main thread.
-static bool
-get_stack_bounds(uintptr_t *bottom, uintptr_t *top)
-{
-  FILE *f = fopen("/proc/self/maps", "r");
-  if (f == NULL)
-    return false;
 
-  while (!feof(f)) {
+static
-    size_t dummy;
+bool get_stack_bounds(uintptr_t *bottom, uintptr_t *top) {
-    char *str = NULL;
+
-    ssize_t len = getline(&str, &dummy, f);
+  char buf[128];
-    if (len == -1) {
+  int fd, sz;
-      fclose(f);
+
+  if ((fd = ::open("/proc/self/maps", O_RDONLY)) < 0) {
     return false;
   }
 
-    if (len > 0 && str[len-1] == '\n') {
+  const char kw[] = "[stack]";
-      str[len-1] = 0;
+  const int kwlen = sizeof(kw)-1;
-      len--;
-    }
 
-    static const char *stack_str = "[stack]";
+  // Address part of /proc/self/maps couldn't be more than 128 bytes
-    if (len > (ssize_t)strlen(stack_str)
+  while ((sz = os::get_line_chars(fd, buf, sizeof(buf))) > 0) {
-       && (strcmp(str + len - strlen(stack_str), stack_str) == 0)) {
+     if (sz > kwlen && ::memcmp(buf+sz-kwlen, kw, kwlen) == 0) {
-      if (sscanf(str, "%" SCNxPTR "-%" SCNxPTR, bottom, top) == 2) {
+        // Extract addresses
+        if (sscanf(buf, "%" SCNxPTR "-%" SCNxPTR, bottom, top) == 2) {
            uintptr_t sp = (uintptr_t) __builtin_frame_address(0);
            if (sp >= *bottom && sp <= *top) {
-          free(str);
+              ::close(fd);
-          fclose(f);
               return true;
            }
         }
      }
-    free(str);
   }
-  fclose(f);
+
+ ::close(fd);
   return false;
 }
 
+
 // If the (growable) stack mapping already extends beyond the point
 // where we're going to put our guard pages, truncate the mapping at
 // that point by munmap()ping it.  This ensures that when we later

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

@@ -1291,3 +1291,41 @@ bool os::is_server_class_machine() {
   }
   return result;
 }
+
+// Read file line by line, if line is longer than bsize,
+// skip rest of line.
+int os::get_line_chars(int fd, char* buf, const size_t bsize){
+  size_t sz, i = 0;
+
+  // read until EOF, EOL or buf is full
+  while ((sz = (int) read(fd, &buf[i], 1)) == 1 && i < (bsize-1) && buf[i] != '\n') {
+     ++i;
+  }
+
+  if (buf[i] == '\n') {
+    // EOL reached so ignore EOL character and return
+
+    buf[i] = 0;
+    return (int) i;
+  }
+
+  buf[i+1] = 0;
+
+  if (sz != 1) {
+    // EOF reached. if we read chars before EOF return them and
+    // return EOF on next call otherwise return EOF
+
+    return (i == 0) ? -1 : (int) i;
+  }
+
+  // line is longer than size of buf, skip to EOL
+  int ch;
+  while (read(fd, &ch, 1) == 1 && ch != '\n') {
+    // Do nothing
+  }
+
+  // return initial part of line that fits in buf.
+  // If we reached EOF, it will be returned on next call.
+
+  return (int) i;
+}

hotspot/src/share/vm/runtime/os.hpp

@@ -658,6 +658,10 @@ class os: AllStatic {
   // Hook for os specific jvm options that we don't want to abort on seeing
   static bool obsolete_option(const JavaVMOption *option);
 
+  // Read file line by line. If line is longer than bsize,
+  // rest of line is skipped. Returns number of bytes read or -1 on EOF
+  static int get_line_chars(int fd, char *buf, const size_t bsize);
+
   // Platform dependent stuff
 #ifdef TARGET_OS_FAMILY_linux
 # include "os_linux.hpp"