Coding style. Minor bugfixes.

SAP · Oct 8, 2017 · b7c1c46 · b7c1c46
1 parent be6c503
commit b7c1c46
Show file tree

Hide file tree

Showing 12 changed files with 138 additions and 153 deletions.
diff --git a/Makefile b/Makefile
@@ -21,6 +21,7 @@ build/$(JATTACH): src/jattach.c
 
 test: all
 	test/smoke-test.sh
+	test/alloc-smoke-test.sh
 
 clean:
 	rm -rf build
diff --git a/profiler.sh b/profiler.sh
@@ -31,18 +31,20 @@ show_agent_output() {
     fi
 }
 
+function abspath() {
+    UNAME_S=$(uname -s)
+    if [ "$UNAME_S" == "Darwin" ]; then
+        perl -MCwd -e 'print Cwd::abs_path shift' $1
+    else
+        readlink -f $1
+    fi
+}
+
 
 OPTIND=1
 SCRIPT_DIR=$(dirname $0)
 JATTACH=$SCRIPT_DIR/build/jattach
-UNAME_S=$(uname -s)
-if [ "$UNAME_S" == "Darwin" ]; then
-    PROFILER=$SCRIPT_DIR/build/libasyncProfiler.so
-    PROFILER=$(perl -MCwd -e 'print Cwd::abs_path shift' $PROFILER)
-else
-    PROFILER=$(readlink -f $SCRIPT_DIR/build/libasyncProfiler.so)
-fi
-
+PROFILER=$(abspath $SCRIPT_DIR/build/libasyncProfiler.so)
 ACTION="collect"
 MODE="cpu"
 DURATION="60"

diff --git a/src/allocTracer.cpp b/src/allocTracer.cpp
@@ -45,17 +45,6 @@ void Trap::uninstall() {
 }
 
 
-bool AllocTracer::checkTracerSymbols() {
-    if (_in_new_tlab._entry == NULL || _outside_tlab._entry == NULL) {
-        NativeCodeCache* libjvm = Profiler::_instance.jvmLibrary();
-        if (libjvm != NULL) {
-            _in_new_tlab._entry = (instruction_t*)libjvm->findSymbol(_in_new_tlab._func_name);
-            _outside_tlab._entry = (instruction_t*)libjvm->findSymbol(_outside_tlab._func_name);
-        }
-    }
-    return _in_new_tlab._entry != NULL && _outside_tlab._entry != NULL;
-}
-
 void AllocTracer::installSignalHandler() {
     struct sigaction sa;
     sigemptyset(&sa.sa_mask);
@@ -93,16 +82,26 @@ void AllocTracer::signalHandler(int signo, siginfo_t* siginfo, void* ucontext) {
 }
 
 bool AllocTracer::start() {
-    if (!VMStructs::available()) {
-        std::cerr << "VMStructs unavailable. Unsupported JVM?" << std::endl;
+    NativeCodeCache* libjvm = Profiler::_instance.jvmLibrary();
+    if (libjvm == NULL) {
+        std::cerr << "libjvm not found among loaded libraries" << std::endl;
         return false;
     }
 
-    if (!checkTracerSymbols()) {
-        std::cerr << "No AllocTracer symbols found. Are JDK debug symbols installed?" << std::endl;
+    if (!VMStructs::init(libjvm)) {
+        std::cerr << "VMStructs unavailable. Unsupported JVM?" << std::endl;
         return false;
     }
 
+    if (_in_new_tlab._entry == NULL || _outside_tlab._entry == NULL) {
+        _in_new_tlab._entry = (instruction_t*)libjvm->findSymbol(_in_new_tlab._func_name);
+        _outside_tlab._entry = (instruction_t*)libjvm->findSymbol(_outside_tlab._func_name);
+        if (_in_new_tlab._entry == NULL || _outside_tlab._entry == NULL) {
+            std::cerr << "No AllocTracer symbols found. Are JDK debug symbols installed?" << std::endl;
+            return false;
+        }
+    }
+
     installSignalHandler();
 
     _in_new_tlab.install();

diff --git a/src/allocTracer.h b/src/allocTracer.h
@@ -44,7 +44,6 @@ class AllocTracer {
     static Trap _in_new_tlab;
     static Trap _outside_tlab;
 
-    static bool checkTracerSymbols();
     static void installSignalHandler();
     static void signalHandler(int signo, siginfo_t* siginfo, void* ucontext);
 

diff --git a/src/jattach.c b/src/jattach.c
@@ -27,37 +27,32 @@
 #include <time.h>
 #include <unistd.h>
 
-#ifdef __linux__
-#define _GNU_SOURCE
-#include <sched.h>
-#endif
-
 #define PATH_MAX 1024
 
-// See hotspot/src/os/bsd/vm/os_bsd.cpp
-// This must be hard coded because it's the system's temporary
-// directory not the java application's temp directory, ala java.io.tmpdir.
-#ifdef __APPLE__
-// macosx has a secure per-user temporary directory
 
-char temp_path_storage[PATH_MAX];
+#ifdef __APPLE__
 
+// macOS has a secure per-user temporary directory
 const char* get_temp_directory() {
-    static char *temp_path = NULL;
-    if (temp_path == NULL) {
-        int pathSize = confstr(_CS_DARWIN_USER_TEMP_DIR, temp_path_storage, PATH_MAX);
-        if (pathSize == 0 || pathSize > PATH_MAX) {
-            strlcpy(temp_path_storage, "/tmp", sizeof (temp_path_storage));
+    static char temp_path_storage[PATH_MAX] = {0};
+
+    if (temp_path_storage[0] == 0) {
+        int path_size = confstr(_CS_DARWIN_USER_TEMP_DIR, temp_path_storage, PATH_MAX);
+        if (path_size == 0 || path_size > PATH_MAX) {
+            strcpy(temp_path_storage, "/tmp");
         }
-        temp_path = temp_path_storage;
     }
-    return temp_path;
+    return temp_path_storage;
 }
+
 #else // __APPLE__
 
 const char* get_temp_directory() {
     return "/tmp";
 }
+
+int setns(int fd, int nstype);
+
 #endif // __APPLE__
 
 // Check if remote JVM has already opened socket for Dynamic Attach

diff --git a/src/stackFrame_aarch64.cpp b/src/stackFrame_aarch64.cpp
@@ -21,7 +21,6 @@
 
 #include "stackFrame.h"
 
-#include <stdio.h>
 
 #define REG_FP 29
 #define REG_LR 30
@@ -56,16 +55,15 @@ void StackFrame::ret() {
 
 bool StackFrame::pop() {
     if (fp() == sp()) {
-      // Expected frame layout:
-      // sp   000000nnnnnnnnnn  [stack]
-      // sp+8 000000nnnnnnnnnn  [link]
-      fp() = stackAt(0);
-      pc() = stackAt(1);
-      sp() += 16;
-    }
-    else {
-      // Hope LR holds correct value
-      pc() = _ucontext->uc_mcontext.regs[REG_LR];
+        // Expected frame layout:
+        // sp   000000nnnnnnnnnn  [stack]
+        // sp+8 000000nnnnnnnnnn  [link]
+        fp() = stackAt(0);
+        pc() = stackAt(1);
+        sp() += 16;
+    } else {
+        // Hope LR holds correct value
+        pc() = _ucontext->uc_mcontext.regs[REG_LR];
     }
     return true;
 }

diff --git a/src/symbols_macos.cpp b/src/symbols_macos.cpp
@@ -16,68 +16,60 @@
 
 #ifdef __APPLE__
 
+#include <string.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <mach-o/dyld_images.h>
 #include <mach-o/loader.h>
-#include <cstring>
 #include <mach-o/nlist.h>
 #include "symbols.h"
-#include <sys/mman.h>
-#include <mach-o/dyld_images.h>
-#include <fcntl.h>
 
 // Workaround for newer macosx versions: since 10.12.x _dyld_get_all_image_infos exists, but is not exported
 extern "C" const struct dyld_all_image_infos* _dyld_get_all_image_infos();
 
+
 class MachOParser {
   private:
     NativeCodeCache* _cc;
-    const uintptr_t _base;
-    const void* _header;
+    const char* _base;
+    const char* _header;
 
     load_command* findCommand(uint32_t command) {
         mach_header_64* header = (mach_header_64*)_header;
-        load_command* result = (load_command*)((uint64_t)header + sizeof(mach_header_64));
-
-        if (command == 0) {
-            return result;
-        }
+        load_command* result = (load_command*)(_header + sizeof(mach_header_64));
 
         for (uint32_t i = 0; i < header->ncmds; i++) {
             if (result->cmd == command) {
-                break;
+                return result;
             }
-
-            result = (load_command*)((uint64_t)result + result->cmdsize);
+            result = (load_command*)((uintptr_t)result + result->cmdsize);
         }
-        return result;
-    }
 
+        return NULL;
+    }
 
     void loadSymbols() {
-        load_command* command = findCommand(LC_SYMTAB);
-        symtab_command* symtab = (symtab_command*)command;
-        nlist_64* symbol_table = (nlist_64*)((char*)_header + symtab->symoff);
-        const char* str_table = (char*)_header + symtab->stroff;
-
-        for (uint32_t sym_n = 0; sym_n < symtab->nsyms; ++sym_n) {
-            nlist_64 descr = symbol_table[sym_n];
-            uint32_t str_offset = descr.n_un.n_strx;
-            uintptr_t offset = descr.n_value;
-            if (offset != 0) {
-                const char* symbol_name = &str_table[str_offset];
-                if (symbol_name != NULL && (descr.n_type)) {
-                    _cc->add((void*)(_base + offset), sizeof(uintptr_t), symbol_name + 1);
-                }
-            }
+        symtab_command* symtab = (symtab_command*)findCommand(LC_SYMTAB);
+        if (symtab == NULL) {
+            return;
         }
 
-        _cc->sort();
+        nlist_64* symbol_table = (nlist_64*)(_header + symtab->symoff);
+        const char* str_table = _header + symtab->stroff;
+
+        for (uint32_t i = 0; i < symtab->nsyms; i++) {
+            nlist_64 sym = symbol_table[i];
+            if ((sym.n_type & 0xee) == 0x0e && sym.n_value != 0) {
+                _cc->add(_base + sym.n_value, 0, str_table + sym.n_un.n_strx + 1);
+            }
+        }
     }
 
   public:
-    MachOParser(NativeCodeCache* cc, uintptr_t base, const char* header) : _cc(cc), _base(base), _header(header) {
+    MachOParser(NativeCodeCache* cc, const char* base, const char* header) : _cc(cc), _base(base), _header(header) {
     }
 
-    static void parseFile(NativeCodeCache* cc, uintptr_t base, const char* file_name) {
+    static void parseFile(NativeCodeCache* cc, const char* base, const char* file_name) {
         int fd = open(file_name, O_RDONLY);
         if (fd == -1) {
             return;
@@ -88,38 +80,37 @@ class MachOParser {
         close(fd);
 
         if (addr != NULL) {
-            MachOParser parser(cc, base, (char*)addr);
+            MachOParser parser(cc, base, (const char*)addr);
             parser.loadSymbols();
             munmap(addr, length);
         }
     }
 };
 
+
 void Symbols::parseKernelSymbols(NativeCodeCache* cc) {
 }
 
 int Symbols::parseMaps(NativeCodeCache** array, int size) {
+    int count = 0;
+    const dyld_all_image_infos* all_images = _dyld_get_all_image_infos();
+
+    for (int i = 0; i < all_images->infoArrayCount && count < size; i++) {
+        const char* path = all_images->infoArray[i].imageFilePath;
+        const char* base = (const char*)all_images->infoArray[i].imageLoadAddress;
 
-    const dyld_all_image_infos* dyld_all_image_infos = _dyld_get_all_image_infos();
-    const char* lib_path = NULL;
-    uintptr_t loaded_lib_addr = 0;
-    for (int i = 0; i < dyld_all_image_infos->infoArrayCount; i++) {
-        const char* path = dyld_all_image_infos->infoArray[i].imageFilePath;
+        // For now load only libjvm symbols. As soon as native stack traces
+        // are supported on macOS, we'll take care about other native libraries
         size_t length = strlen(path);
-        if (strcmp(path + length - 12, "libjvm.dylib") == 0) {
-            lib_path = path;
-            loaded_lib_addr = (uintptr_t)dyld_all_image_infos->infoArray[i].imageLoadAddress;
+        if (length >= 12 && strcmp(path + length - 12, "libjvm.dylib") == 0) {
+            NativeCodeCache* cc = new NativeCodeCache(path);
+            MachOParser::parseFile(cc, base, path);
+            cc->sort();
+            array[count++] = cc;
         }
     }
 
-    if (lib_path == NULL) {
-        return 0;
-    }
-
-    NativeCodeCache* cc = new NativeCodeCache(lib_path);
-    MachOParser::parseFile(cc, loaded_lib_addr, lib_path);
-    array[0] = cc;
-    return 1;
+    return count;
 }
 
 #endif // __APPLE__
diff --git a/src/vmEntry.cpp b/src/vmEntry.cpp
@@ -20,7 +20,6 @@
 #include "arguments.h"
 #include "profiler.h"
 #include "perfEvents.h"
-#include "vmStructs.h"
 
 
 JavaVM* VM::_vm;
@@ -64,6 +63,7 @@ bool VM::init(JavaVM* vm) {
     _jvmti->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_DYNAMIC_CODE_GENERATED, NULL);
 
     PerfEvents::init();
+
     _asyncGetCallTrace = (AsyncGetCallTrace)dlsym(RTLD_DEFAULT, "AsyncGetCallTrace");
     if (_asyncGetCallTrace == NULL) {
         std::cerr << "Could not find AsyncGetCallTrace function" << std::endl;