OSDN Git Service

release-request-60f55d52-4657-4714-89dc-b6443475d767-for-git_oc-release-4022373 snap...
authorandroid-build-team Robot <android-build-team-robot@google.com>
Thu, 18 May 2017 07:14:12 +0000 (07:14 +0000)
committerandroid-build-team Robot <android-build-team-robot@google.com>
Thu, 18 May 2017 07:14:12 +0000 (07:14 +0000)
Change-Id: I42201fff3f019509c3c9278d76755edfccd0df49

17 files changed:
runtime/fault_handler.cc
runtime/jit/profile_compilation_info.cc
runtime/jit/profile_compilation_info.h
runtime/jit/profile_compilation_info_test.cc
runtime/jit/profile_saver.cc
runtime/jit/profile_saver.h
runtime/native_bridge_art_interface.cc
runtime/runtime.cc
sigchainlib/sigchain.cc
sigchainlib/sigchain.h
sigchainlib/sigchain_dummy.cc
sigchainlib/version-script32.txt
sigchainlib/version-script64.txt
test/115-native-bridge/expected.txt
test/115-native-bridge/nativebridge.cc
test/115-native-bridge/run
test/115-native-bridge/src/NativeBridgeMain.java

index 7f738bf..5594f4d 100644 (file)
@@ -129,7 +129,21 @@ FaultManager::~FaultManager() {
 
 void FaultManager::Init() {
   CHECK(!initialized_);
-  AddSpecialSignalHandlerFn(SIGSEGV, art_fault_handler);
+  sigset_t mask;
+  sigfillset(&mask);
+  sigdelset(&mask, SIGABRT);
+  sigdelset(&mask, SIGBUS);
+  sigdelset(&mask, SIGFPE);
+  sigdelset(&mask, SIGILL);
+  sigdelset(&mask, SIGSEGV);
+
+  SigchainAction sa = {
+    .sc_sigaction = art_fault_handler,
+    .sc_mask = mask,
+    .sc_flags = 0UL,
+  };
+
+  AddSpecialSignalHandlerFn(SIGSEGV, &sa);
   initialized_ = true;
 }
 
index 0acce1e..3a2b6ef 100644 (file)
@@ -109,9 +109,7 @@ bool ProfileCompilationInfo::AddMethodsAndClasses(
   return true;
 }
 
-bool ProfileCompilationInfo::MergeAndSave(const std::string& filename,
-                                          uint64_t* bytes_written,
-                                          bool force) {
+bool ProfileCompilationInfo::Load(const std::string& filename, bool clear_if_invalid) {
   ScopedTrace trace(__PRETTY_FUNCTION__);
   ScopedFlock flock;
   std::string error;
@@ -126,36 +124,42 @@ bool ProfileCompilationInfo::MergeAndSave(const std::string& filename,
 
   int fd = flock.GetFile()->Fd();
 
-  // Load the file but keep a copy around to be able to infer if the content has changed.
-  ProfileCompilationInfo fileInfo;
-  ProfileLoadSatus status = fileInfo.LoadInternal(fd, &error);
+  ProfileLoadSatus status = LoadInternal(fd, &error);
   if (status == kProfileLoadSuccess) {
-    // Merge the content of file into the current object.
-    if (MergeWith(fileInfo)) {
-      // If after the merge we have the same data as what is the file there's no point
-      // in actually doing the write. The file will be exactly the same as before.
-      if (Equals(fileInfo)) {
-        if (bytes_written != nullptr) {
-          *bytes_written = 0;
-        }
-        return true;
-      }
+    return true;
+  }
+
+  if (clear_if_invalid &&
+      ((status == kProfileLoadVersionMismatch) || (status == kProfileLoadBadData))) {
+    LOG(WARNING) << "Clearing bad or obsolete profile data from file "
+                 << filename << ": " << error;
+    if (flock.GetFile()->ClearContent()) {
+      return true;
     } else {
-      LOG(WARNING) << "Could not merge previous profile data from file " << filename;
-      if (!force) {
-        return false;
-      }
+      PLOG(WARNING) << "Could not clear profile file: " << filename;
+      return false;
     }
-  } else if (force &&
-        ((status == kProfileLoadVersionMismatch) || (status == kProfileLoadBadData))) {
-      // Log a warning but don't return false. We will clear the profile anyway.
-      LOG(WARNING) << "Clearing bad or obsolete profile data from file "
-          << filename << ": " << error;
-  } else {
-    LOG(WARNING) << "Could not load profile data from file " << filename << ": " << error;
+  }
+
+  LOG(WARNING) << "Could not load profile data from file " << filename << ": " << error;
+  return false;
+}
+
+bool ProfileCompilationInfo::Save(const std::string& filename, uint64_t* bytes_written) {
+  ScopedTrace trace(__PRETTY_FUNCTION__);
+  ScopedFlock flock;
+  std::string error;
+  int flags = O_WRONLY | O_NOFOLLOW | O_CLOEXEC;
+  // There's no need to fsync profile data right away. We get many chances
+  // to write it again in case something goes wrong. We can rely on a simple
+  // close(), no sync, and let to the kernel decide when to write to disk.
+  if (!flock.Init(filename.c_str(), flags, /*block*/false, /*flush_on_close*/false, &error)) {
+    LOG(WARNING) << "Couldn't lock the profile file " << filename << ": " << error;
     return false;
   }
 
+  int fd = flock.GetFile()->Fd();
+
   // We need to clear the data because we don't support appending to the profiles yet.
   if (!flock.GetFile()->ClearContent()) {
     PLOG(WARNING) << "Could not clear profile file: " << filename;
@@ -166,10 +170,14 @@ bool ProfileCompilationInfo::MergeAndSave(const std::string& filename,
   // access and fail immediately if we can't.
   bool result = Save(fd);
   if (result) {
-    VLOG(profiler) << "Successfully saved profile info to " << filename
-        << " Size: " << GetFileSizeBytes(filename);
-    if (bytes_written != nullptr) {
-      *bytes_written = GetFileSizeBytes(filename);
+    int64_t size = GetFileSizeBytes(filename);
+    if (size != -1) {
+      VLOG(profiler)
+        << "Successfully saved profile info to " << filename << " Size: "
+        << size;
+      if (bytes_written != nullptr) {
+        *bytes_written = static_cast<uint64_t>(size);
+      }
     }
   } else {
     VLOG(profiler) << "Failed to save profile info to " << filename;
@@ -800,6 +808,7 @@ bool ProfileCompilationInfo::Load(int fd) {
   }
 }
 
+// TODO(calin): fail fast if the dex checksums don't match.
 ProfileCompilationInfo::ProfileLoadSatus ProfileCompilationInfo::LoadInternal(
       int fd, std::string* error) {
   ScopedTrace trace(__PRETTY_FUNCTION__);
index f68ed5d..60e1d53 100644 (file)
@@ -196,18 +196,20 @@ class ProfileCompilationInfo {
   // If the current profile is non-empty the load will fail.
   bool Load(int fd);
 
+  // Load profile information from the given file
+  // If the current profile is non-empty the load will fail.
+  // If clear_if_invalid is true and the file is invalid the method clears the
+  // the file and returns true.
+  bool Load(const std::string& filename, bool clear_if_invalid);
+
   // Merge the data from another ProfileCompilationInfo into the current object.
   bool MergeWith(const ProfileCompilationInfo& info);
 
   // Save the profile data to the given file descriptor.
   bool Save(int fd);
 
-  // Load and merge profile information from the given file into the current
-  // object and tries to save it back to disk.
-  // If `force` is true then the save will go through even if the given file
-  // has bad data or its version does not match. In this cases the profile content
-  // is ignored.
-  bool MergeAndSave(const std::string& filename, uint64_t* bytes_written, bool force);
+  // Save the current profile into the given file. The file will be cleared before saving.
+  bool Save(const std::string& filename, uint64_t* bytes_written);
 
   // Return the number of methods that were profiled.
   uint32_t GetNumberOfMethods() const;
index c9f2d0e..e8f4ce2 100644 (file)
@@ -92,7 +92,15 @@ class ProfileCompilationInfoTest : public CommonRuntimeTest {
     if (info.GetNumberOfMethods() != profile_methods.size()) {
       return false;
     }
-    return info.MergeAndSave(filename, nullptr, false);
+    ProfileCompilationInfo file_profile;
+    if (!file_profile.Load(filename, false)) {
+      return false;
+    }
+    if (!info.MergeWith(file_profile)) {
+      return false;
+    }
+
+    return info.Save(filename, nullptr);
   }
 
   // Saves the given art methods to a profile backed by 'filename' and adds
@@ -145,7 +153,7 @@ class ProfileCompilationInfoTest : public CommonRuntimeTest {
     if (info.GetNumberOfMethods() != profile_methods.size()) {
       return false;
     }
-    return info.MergeAndSave(filename, nullptr, false);
+    return info.Save(filename, nullptr);
   }
 
   ProfileCompilationInfo::OfflineProfileMethodInfo ConvertProfileMethodInfo(
index 1441987..602ece0 100644 (file)
@@ -171,14 +171,6 @@ void ProfileSaver::NotifyJitActivityInternal() {
   }
 }
 
-ProfileSaver::ProfileInfoCache* ProfileSaver::GetCachedProfiledInfo(const std::string& filename) {
-  auto info_it = profile_cache_.find(filename);
-  if (info_it == profile_cache_.end()) {
-    info_it = profile_cache_.Put(filename, ProfileInfoCache());
-  }
-  return &info_it->second;
-}
-
 // Get resolved methods that have a profile info or more than kStartupMethodSamples samples.
 // Excludes native methods and classes in the boot image.
 class GetMethodsVisitor : public ClassVisitor {
@@ -252,9 +244,11 @@ void ProfileSaver::FetchAndCacheResolvedClassesAndMethods() {
                        << " (" << classes.GetDexLocation() << ")";
       }
     }
-    ProfileInfoCache* cached_info = GetCachedProfiledInfo(filename);
-    cached_info->profile.AddMethodsAndClasses(profile_methods_for_location,
-                                              resolved_classes_for_location);
+    auto info_it = profile_cache_.Put(filename, ProfileCompilationInfo());
+
+    ProfileCompilationInfo* cached_info = &(info_it->second);
+    cached_info->AddMethodsAndClasses(profile_methods_for_location,
+                                      resolved_classes_for_location);
     total_number_of_profile_entries_cached += resolved_classes_for_location.size();
   }
   max_number_of_profile_entries_cached_ = std::max(
@@ -297,16 +291,22 @@ bool ProfileSaver::ProcessProfilingInfo(bool force_save, /*out*/uint16_t* number
       jit_code_cache_->GetProfiledMethods(locations, profile_methods);
       total_number_of_code_cache_queries_++;
     }
+    ProfileCompilationInfo info;
+    if (!info.Load(filename, /*clear_if_invalid*/ true)) {
+      LOG(ERROR) << "Could not forcefully load profile " << filename;
+      continue;
+    }
+    uint64_t last_save_number_of_methods = info.GetNumberOfMethods();
+    uint64_t last_save_number_of_classes = info.GetNumberOfResolvedClasses();
+
+    info.AddMethodsAndClasses(profile_methods, std::set<DexCacheResolvedClasses>());
+    auto profile_cache_it = profile_cache_.find(filename);
+    if (profile_cache_it != profile_cache_.end()) {
+      info.MergeWith(profile_cache_it->second);
+    }
 
-    ProfileInfoCache* cached_info = GetCachedProfiledInfo(filename);
-    ProfileCompilationInfo* cached_profile = &cached_info->profile;
-    cached_profile->AddMethodsAndClasses(profile_methods, std::set<DexCacheResolvedClasses>());
-    int64_t delta_number_of_methods =
-        cached_profile->GetNumberOfMethods() -
-        static_cast<int64_t>(cached_info->last_save_number_of_methods);
-    int64_t delta_number_of_classes =
-        cached_profile->GetNumberOfResolvedClasses() -
-        static_cast<int64_t>(cached_info->last_save_number_of_classes);
+    int64_t delta_number_of_methods = info.GetNumberOfMethods() - last_save_number_of_methods;
+    int64_t delta_number_of_classes = info.GetNumberOfResolvedClasses() - last_save_number_of_classes;
 
     if (!force_save &&
         delta_number_of_methods < options_.GetMinMethodsToSave() &&
@@ -324,12 +324,12 @@ bool ProfileSaver::ProcessProfilingInfo(bool force_save, /*out*/uint16_t* number
     uint64_t bytes_written;
     // Force the save. In case the profile data is corrupted or the the profile
     // has the wrong version this will "fix" the file to the correct format.
-    if (cached_profile->MergeAndSave(filename, &bytes_written, /*force*/ true)) {
-      cached_info->last_save_number_of_methods = cached_profile->GetNumberOfMethods();
-      cached_info->last_save_number_of_classes = cached_profile->GetNumberOfResolvedClasses();
-      // Clear resolved classes. No need to store them around as
-      // they don't change after the first write.
-      cached_profile->ClearResolvedClasses();
+    if (info.Save(filename, &bytes_written)) {
+      // We managed to save the profile. Clear the cache stored during startup.
+      if (profile_cache_it != profile_cache_.end()) {
+        profile_cache_.erase(profile_cache_it);
+        total_number_of_profile_entries_cached = 0;
+      }
       if (bytes_written > 0) {
         total_number_of_writes_++;
         total_bytes_written_ += bytes_written;
@@ -345,13 +345,8 @@ bool ProfileSaver::ProcessProfilingInfo(bool force_save, /*out*/uint16_t* number
       LOG(WARNING) << "Could not save profiling info to " << filename;
       total_number_of_failed_writes_++;
     }
-    total_number_of_profile_entries_cached +=
-        cached_profile->GetNumberOfMethods() +
-        cached_profile->GetNumberOfResolvedClasses();
   }
-  max_number_of_profile_entries_cached_ = std::max(
-      max_number_of_profile_entries_cached_,
-      total_number_of_profile_entries_cached);
+
   return profile_file_saved;
 }
 
@@ -575,7 +570,10 @@ bool ProfileSaver::HasSeenMethod(const std::string& profile,
                                  uint16_t method_idx) {
   MutexLock mu(Thread::Current(), *Locks::profiler_lock_);
   if (instance_ != nullptr) {
-    const ProfileCompilationInfo& info = instance_->GetCachedProfiledInfo(profile)->profile;
+    ProfileCompilationInfo info;
+    if (!info.Load(profile, /*clear_if_invalid*/false)) {
+      return false;
+    }
     return info.ContainsMethod(MethodReference(dex_file, method_idx));
   }
   return false;
index bd539a4..60c9cc6 100644 (file)
@@ -61,14 +61,6 @@ class ProfileSaver {
                             uint16_t method_idx);
 
  private:
-  // A cache structure which keeps track of the data saved to disk.
-  // It is used to reduce the number of disk read/writes.
-  struct ProfileInfoCache {
-    ProfileCompilationInfo profile;
-    uint32_t last_save_number_of_methods = 0;
-    uint32_t last_save_number_of_classes = 0;
-  };
-
   ProfileSaver(const ProfileSaverOptions& options,
                const std::string& output_filename,
                jit::JitCodeCache* jit_code_cache,
@@ -102,10 +94,6 @@ class ProfileSaver {
                            const std::vector<std::string>& code_paths)
       REQUIRES(Locks::profiler_lock_);
 
-  // Retrieves the cached profile compilation info for the given profile file.
-  // If no entry exists, a new empty one will be created, added to the cache and
-  // then returned.
-  ProfileInfoCache* GetCachedProfiledInfo(const std::string& filename);
   // Fetches the current resolved classes and methods from the ClassLinker and stores them in the
   // profile_cache_ for later save.
   void FetchAndCacheResolvedClassesAndMethods();
@@ -139,10 +127,11 @@ class ProfileSaver {
   uint32_t jit_activity_notifications_;
 
   // A local cache for the profile information. Maps each tracked file to its
-  // profile information. The size of this cache is usually very small and tops
+  // profile information. This is used to cache the startup classes so that
+  // we don't hammer the disk to save them right away.
+  // The size of this cache is usually very small and tops
   // to just a few hundreds entries in the ProfileCompilationInfo objects.
-  // It helps avoiding unnecessary writes to disk.
-  SafeMap<std::string, ProfileInfoCache> profile_cache_;
+  SafeMap<std::string, ProfileCompilationInfo> profile_cache_;
 
   // Save period condition support.
   Mutex wait_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
index d77cfa1..cd8315c 100644 (file)
@@ -118,7 +118,15 @@ void InitializeNativeBridge(JNIEnv* env, const char* instruction_set) {
       for (int signal = 0; signal < _NSIG; ++signal) {
         android::NativeBridgeSignalHandlerFn fn = android::NativeBridgeGetSignalHandler(signal);
         if (fn != nullptr) {
-          AddSpecialSignalHandlerFn(signal, fn);
+          sigset_t mask;
+          sigfillset(&mask);
+          SigchainAction sa = {
+            .sc_sigaction = fn,
+            .sc_mask = mask,
+            // The native bridge signal might not return back to sigchain's handler.
+            .sc_flags = SIGCHAIN_ALLOW_NORETURN,
+          };
+          AddSpecialSignalHandlerFn(signal, &sa);
         }
       }
 #endif
index 093a399..8667310 100644 (file)
@@ -1180,12 +1180,6 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) {
 
   if (!no_sig_chain_) {
     // Dex2Oat's Runtime does not need the signal chain or the fault handler.
-
-    // Initialize the signal chain so that any calls to sigaction get
-    // correctly routed to the next in the chain regardless of whether we
-    // have claimed the signal or not.
-    InitializeSignalChain();
-
     if (implicit_null_checks_ || implicit_so_checks_ || implicit_suspend_checks_) {
       fault_manager.Init();
 
index f4799d2..df4372f 100644 (file)
@@ -93,6 +93,35 @@ namespace art {
 static decltype(&sigaction) linked_sigaction;
 static decltype(&sigprocmask) linked_sigprocmask;
 
+__attribute__((constructor)) static void InitializeSignalChain() {
+  static std::once_flag once;
+  std::call_once(once, []() {
+    void* linked_sigaction_sym = dlsym(RTLD_NEXT, "sigaction");
+    if (linked_sigaction_sym == nullptr) {
+      linked_sigaction_sym = dlsym(RTLD_DEFAULT, "sigaction");
+      if (linked_sigaction_sym == nullptr ||
+          linked_sigaction_sym == reinterpret_cast<void*>(sigaction)) {
+        fatal("Unable to find next sigaction in signal chain");
+      }
+    }
+
+    void* linked_sigprocmask_sym = dlsym(RTLD_NEXT, "sigprocmask");
+    if (linked_sigprocmask_sym == nullptr) {
+      linked_sigprocmask_sym = dlsym(RTLD_DEFAULT, "sigprocmask");
+      if (linked_sigprocmask_sym == nullptr ||
+          linked_sigprocmask_sym == reinterpret_cast<void*>(sigprocmask)) {
+        fatal("Unable to find next sigprocmask in signal chain");
+      }
+    }
+
+    linked_sigaction =
+        reinterpret_cast<decltype(linked_sigaction)>(linked_sigaction_sym);
+    linked_sigprocmask =
+        reinterpret_cast<decltype(linked_sigprocmask)>(linked_sigprocmask_sym);
+  });
+}
+
+
 static pthread_key_t GetHandlingSignalKey() {
   static pthread_key_t key;
   static std::once_flag once;
@@ -161,10 +190,10 @@ class SignalChain {
     return action_;
   }
 
-  void AddSpecialHandler(SpecialSignalHandlerFn fn) {
-    for (SpecialSignalHandlerFn& slot : special_handlers_) {
-      if (slot == nullptr) {
-        slot = fn;
+  void AddSpecialHandler(SigchainAction* sa) {
+    for (SigchainAction& slot : special_handlers_) {
+      if (slot.sc_sigaction == nullptr) {
+        slot = *sa;
         return;
       }
     }
@@ -172,15 +201,15 @@ class SignalChain {
     fatal("too many special signal handlers");
   }
 
-  void RemoveSpecialHandler(SpecialSignalHandlerFn fn) {
+  void RemoveSpecialHandler(bool (*fn)(int, siginfo_t*, void*)) {
     // This isn't thread safe, but it's unlikely to be a real problem.
     size_t len = sizeof(special_handlers_)/sizeof(*special_handlers_);
     for (size_t i = 0; i < len; ++i) {
-      if (special_handlers_[i] == fn) {
+      if (special_handlers_[i].sc_sigaction == fn) {
         for (size_t j = i; j < len - 1; ++j) {
           special_handlers_[j] = special_handlers_[j + 1];
         }
-        special_handlers_[len - 1] = nullptr;
+        special_handlers_[len - 1].sc_sigaction = nullptr;
         return;
       }
     }
@@ -194,47 +223,37 @@ class SignalChain {
  private:
   bool claimed_;
   struct sigaction action_;
-  SpecialSignalHandlerFn special_handlers_[2];
+  SigchainAction special_handlers_[2];
 };
 
 static SignalChain chains[_NSIG];
 
-class ScopedSignalUnblocker {
- public:
-  explicit ScopedSignalUnblocker(const std::initializer_list<int>& signals) {
-    sigset_t new_mask;
-    sigemptyset(&new_mask);
-    for (int signal : signals) {
-      sigaddset(&new_mask, signal);
-    }
-    if (sigprocmask(SIG_UNBLOCK, &new_mask, &previous_mask_) != 0) {
-      fatal("failed to unblock signals: %s", strerror(errno));
-    }
-  }
-
-  ~ScopedSignalUnblocker() {
-    if (sigprocmask(SIG_SETMASK, &previous_mask_, nullptr) != 0) {
-      fatal("failed to unblock signals: %s", strerror(errno));
-    }
-  }
-
- private:
-  sigset_t previous_mask_;
-};
-
 void SignalChain::Handler(int signo, siginfo_t* siginfo, void* ucontext_raw) {
-  ScopedHandlingSignal handling_signal;
-
   // Try the special handlers first.
   // If one of them crashes, we'll reenter this handler and pass that crash onto the user handler.
   if (!GetHandlingSignal()) {
-    ScopedSignalUnblocker unblocked { SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGSEGV }; // NOLINT
-    SetHandlingSignal(true);
-
     for (const auto& handler : chains[signo].special_handlers_) {
-      if (handler != nullptr && handler(signo, siginfo, ucontext_raw)) {
+      if (handler.sc_sigaction == nullptr) {
+        break;
+      }
+
+      // The native bridge signal handler might not return.
+      // Avoid setting the thread local flag in this case, since we'll never
+      // get a chance to restore it.
+      bool handler_noreturn = (handler.sc_flags & SIGCHAIN_ALLOW_NORETURN);
+      sigset_t previous_mask;
+      linked_sigprocmask(SIG_SETMASK, &handler.sc_mask, &previous_mask);
+
+      ScopedHandlingSignal restorer;
+      if (!handler_noreturn) {
+        SetHandlingSignal(true);
+      }
+
+      if (handler.sc_sigaction(signo, siginfo, ucontext_raw)) {
         return;
       }
+
+      linked_sigprocmask(SIG_SETMASK, &previous_mask, nullptr);
     }
   }
 
@@ -246,7 +265,7 @@ void SignalChain::Handler(int signo, siginfo_t* siginfo, void* ucontext_raw) {
   if ((handler_flags & SA_NODEFER)) {
     sigdelset(&mask, signo);
   }
-  sigprocmask(SIG_SETMASK, &mask, nullptr);
+  linked_sigprocmask(SIG_SETMASK, &mask, nullptr);
 
   if ((handler_flags & SA_SIGINFO)) {
     chains[signo].action_.sa_sigaction(signo, siginfo, ucontext_raw);
@@ -263,6 +282,8 @@ void SignalChain::Handler(int signo, siginfo_t* siginfo, void* ucontext_raw) {
 }
 
 extern "C" int sigaction(int signal, const struct sigaction* new_action, struct sigaction* old_action) {
+  InitializeSignalChain();
+
   // If this signal has been claimed as a signal chain, record the user's
   // action but don't pass it on to the kernel.
   // Note that we check that the signal number is in range here.  An out of range signal
@@ -285,11 +306,12 @@ extern "C" int sigaction(int signal, const struct sigaction* new_action, struct
 
   // Will only get here if the signal chain has not been claimed.  We want
   // to pass the sigaction on to the kernel via the real sigaction in libc.
-  InitializeSignalChain();
   return linked_sigaction(signal, new_action, old_action);
 }
 
 extern "C" sighandler_t signal(int signo, sighandler_t handler) {
+  InitializeSignalChain();
+
   if (signo < 0 || signo > _NSIG) {
     errno = EINVAL;
     return SIG_ERR;
@@ -311,7 +333,6 @@ extern "C" sighandler_t signal(int signo, sighandler_t handler) {
 
   // Will only get here if the signal chain has not been claimed.  We want
   // to pass the sigaction on to the kernel via the real sigaction in libc.
-  InitializeSignalChain();
   if (linked_sigaction(signo, &sa, &sa) == -1) {
     return SIG_ERR;
   }
@@ -321,11 +342,15 @@ extern "C" sighandler_t signal(int signo, sighandler_t handler) {
 
 #if !defined(__LP64__)
 extern "C" sighandler_t bsd_signal(int signo, sighandler_t handler) {
+  InitializeSignalChain();
+
   return signal(signo, handler);
 }
 #endif
 
 extern "C" int sigprocmask(int how, const sigset_t* bionic_new_set, sigset_t* bionic_old_set) {
+  InitializeSignalChain();
+
   // When inside a signal handler, forward directly to the actual sigprocmask.
   if (GetHandlingSignal()) {
     return linked_sigprocmask(how, bionic_new_set, bionic_old_set);
@@ -348,57 +373,24 @@ extern "C" int sigprocmask(int how, const sigset_t* bionic_new_set, sigset_t* bi
     new_set_ptr = &tmpset;
   }
 
-  InitializeSignalChain();
   return linked_sigprocmask(how, new_set_ptr, bionic_old_set);
 }
 
-extern "C" void InitializeSignalChain() {
-  // Warning.
-  // Don't call this from within a signal context as it makes calls to
-  // dlsym.  Calling into the dynamic linker will result in locks being
-  // taken and if it so happens that a signal occurs while one of these
-  // locks is already taken, dlsym will block trying to reenter a
-  // mutex and we will never get out of it.
-  static bool initialized = false;
-  if (initialized) {
-    // Don't initialize twice.
-    return;
-  }
-
-  void* linked_sigaction_sym = dlsym(RTLD_NEXT, "sigaction");
-  if (linked_sigaction_sym == nullptr) {
-    linked_sigaction_sym = dlsym(RTLD_DEFAULT, "sigaction");
-    if (linked_sigaction_sym == nullptr ||
-        linked_sigaction_sym == reinterpret_cast<void*>(sigaction)) {
-      fatal("Unable to find next sigaction in signal chain");
-    }
-  }
-
-  void* linked_sigprocmask_sym = dlsym(RTLD_NEXT, "sigprocmask");
-  if (linked_sigprocmask_sym == nullptr) {
-    linked_sigprocmask_sym = dlsym(RTLD_DEFAULT, "sigprocmask");
-    if (linked_sigprocmask_sym == nullptr ||
-        linked_sigprocmask_sym == reinterpret_cast<void*>(sigprocmask)) {
-      fatal("Unable to find next sigprocmask in signal chain");
-    }
-  }
-
-  linked_sigaction = reinterpret_cast<decltype(linked_sigaction)>(linked_sigaction_sym);
-  linked_sigprocmask = reinterpret_cast<decltype(linked_sigprocmask)>(linked_sigprocmask_sym);
-  initialized = true;
-}
+extern "C" void AddSpecialSignalHandlerFn(int signal, SigchainAction* sa) {
+  InitializeSignalChain();
 
-extern "C" void AddSpecialSignalHandlerFn(int signal, SpecialSignalHandlerFn fn) {
   if (signal <= 0 || signal >= _NSIG) {
     fatal("Invalid signal %d", signal);
   }
 
   // Set the managed_handler.
-  chains[signal].AddSpecialHandler(fn);
+  chains[signal].AddSpecialHandler(sa);
   chains[signal].Claim(signal);
 }
 
-extern "C" void RemoveSpecialSignalHandlerFn(int signal, SpecialSignalHandlerFn fn) {
+extern "C" void RemoveSpecialSignalHandlerFn(int signal, bool (*fn)(int, siginfo_t*, void*)) {
+  InitializeSignalChain();
+
   if (signal <= 0 || signal >= _NSIG) {
     fatal("Invalid signal %d", signal);
   }
@@ -407,14 +399,16 @@ extern "C" void RemoveSpecialSignalHandlerFn(int signal, SpecialSignalHandlerFn
 }
 
 extern "C" void EnsureFrontOfChain(int signal) {
+  InitializeSignalChain();
+
   if (signal <= 0 || signal >= _NSIG) {
     fatal("Invalid signal %d", signal);
   }
 
   // Read the current action without looking at the chain, it should be the expected action.
   struct sigaction current_action;
-  InitializeSignalChain();
   linked_sigaction(signal, nullptr, &current_action);
+
   // If the sigactions don't match then we put the current action on the chain and make ourself as
   // the main action.
   if (current_action.sa_sigaction != SignalChain::Handler) {
index 960d221..23fba03 100644 (file)
 #define ART_SIGCHAINLIB_SIGCHAIN_H_
 
 #include <signal.h>
+#include <stdint.h>
 
 namespace art {
 
-extern "C" void InitializeSignalChain();
+// Handlers that exit without returning to their caller (e.g. via siglongjmp) must pass this flag.
+static constexpr uint64_t SIGCHAIN_ALLOW_NORETURN = 0x1UL;
 
-typedef bool (*SpecialSignalHandlerFn)(int, siginfo_t*, void*);
-extern "C" void AddSpecialSignalHandlerFn(int signal, SpecialSignalHandlerFn fn);
-extern "C" void RemoveSpecialSignalHandlerFn(int signal, SpecialSignalHandlerFn fn);
+struct SigchainAction {
+  bool (*sc_sigaction)(int, siginfo_t*, void*);
+  sigset_t sc_mask;
+  uint64_t sc_flags;
+};
+
+extern "C" void AddSpecialSignalHandlerFn(int signal, SigchainAction* sa);
+extern "C" void RemoveSpecialSignalHandlerFn(int signal, bool (*fn)(int, siginfo_t*, void*));
 
 extern "C" void EnsureFrontOfChain(int signal);
 
index d6a5e12..edce965 100644 (file)
@@ -48,24 +48,19 @@ static void log(const char* format, ...) {
 
 namespace art {
 
-extern "C" void InitializeSignalChain() {
-  log("InitializeSignalChain is not exported by the main executable.");
-  abort();
-}
-
 extern "C" void EnsureFrontOfChain(int signal ATTRIBUTE_UNUSED) {
   log("EnsureFrontOfChain is not exported by the main executable.");
   abort();
 }
 
 extern "C" void AddSpecialSignalHandlerFn(int signal ATTRIBUTE_UNUSED,
-                                          SpecialSignalHandlerFn fn ATTRIBUTE_UNUSED) {
+                                          SigchainAction* sa ATTRIBUTE_UNUSED) {
   log("SetSpecialSignalHandlerFn is not exported by the main executable.");
   abort();
 }
 
 extern "C" void RemoveSpecialSignalHandlerFn(int signal ATTRIBUTE_UNUSED,
-                                          SpecialSignalHandlerFn fn ATTRIBUTE_UNUSED) {
+                                             bool (*fn)(int, siginfo_t*, void*) ATTRIBUTE_UNUSED) {
   log("SetSpecialSignalHandlerFn is not exported by the main executable.");
   abort();
 }
index f360efa..2340785 100644 (file)
@@ -1,6 +1,5 @@
 {
 global:
-  InitializeSignalChain;
   EnsureFrontOfChain;
   AddSpecialSignalHandlerFn;
   RemoveSpecialSignalHandlerFn;
index 319d1c6..acf3630 100644 (file)
@@ -1,6 +1,5 @@
 {
 global:
-  InitializeSignalChain;
   EnsureFrontOfChain;
   AddSpecialSignalHandlerFn;
   RemoveSpecialSignalHandlerFn;
index 852ec2e..343a762 100644 (file)
@@ -3,7 +3,7 @@ Checking for getEnvValues.
 Ready for native bridge tests.
 Checking for support.
 Getting trampoline for JNI_OnLoad with shorty (null).
-Test ART callbacks: all JNI function number is 11.
+Test ART callbacks: all JNI function number is 12.
     name:booleanMethod, signature:(ZZZZZZZZZZ)Z, shorty:ZZZZZZZZZZZ.
     name:byteMethod, signature:(BBBBBBBBBB)B, shorty:BBBBBBBBBBB.
     name:charMethod, signature:(CCCCCCCCCC)C, shorty:CCCCCCCCCCC.
@@ -14,6 +14,7 @@ Test ART callbacks: all JNI function number is 11.
     name:testGetMirandaMethodNative, signature:()Ljava/lang/reflect/Method;, shorty:L.
     name:testNewStringObject, signature:()V, shorty:V.
     name:testSignal, signature:()I, shorty:I.
+    name:testSignalHandlerNotReturn, signature:()V, shorty:V.
     name:testZeroLengthByteBuffers, signature:()V, shorty:V.
 trampoline_JNI_OnLoad called!
 JNI_OnLoad called
@@ -62,3 +63,18 @@ trampoline_Java_Main_testNewStringObject called!
 Getting trampoline for Java_Main_testSignal with shorty I.
 NB signal handler with signal 11.
 NB signal handler with signal 4.
+Loading invalid library 'libinvalid.so' from Java, which will fail.
+Checking for support.
+Was to load 'libinvalid.so', force fail.
+getError() in native bridge.
+Catch UnsatisfiedLinkError exception as expected.
+Getting trampoline for Java_Main_testSignalHandlerNotReturn with shorty V.
+start testSignalHandlerNotReturn
+raising first SIGSEGV
+NB signal handler with signal 11.
+handling first SIGSEGV, will raise another
+unblock SIGSEGV in handler
+raising second SIGSEGV
+NB signal handler with signal 11.
+handling second SIGSEGV, will jump back to test function
+back to test from signal handler via siglongjmp(), and done!
index 87287f8..307fd9b 100644 (file)
@@ -26,6 +26,7 @@
 #include "stdio.h"
 #include "unistd.h"
 #include "sys/stat.h"
+#include "setjmp.h"
 
 #include "base/macros.h"
 #include "nativebridge/native_bridge.h"
@@ -191,6 +192,19 @@ char *go_away_compiler = nullptr;
   abort();
 }
 
+static void raise_sigsegv() {
+#if defined(__arm__) || defined(__i386__) || defined(__aarch64__)
+  *go_away_compiler = 'a';
+#elif defined(__x86_64__)
+  // Cause a SEGV using an instruction known to be 2 bytes long to account for hardcoded jump
+  // in the signal handler
+  asm volatile("movl $0, %%eax;" "movb %%ah, (%%rax);" : : : "%eax");
+#else
+  // On other architectures we simulate SEGV.
+  kill(getpid(), SIGSEGV);
+#endif
+}
+
 static jint trampoline_Java_Main_testSignal(JNIEnv*, jclass) {
   // Install the sigaction handler above, which should *not* be reached as the native-bridge
   // handler should be called first. Note: we won't chain at all, if we ever get here, we'll die.
@@ -203,16 +217,7 @@ static jint trampoline_Java_Main_testSignal(JNIEnv*, jclass) {
 
   // Test segv
   sigaction(SIGSEGV, &tmp, nullptr);
-#if defined(__arm__) || defined(__i386__) || defined(__aarch64__)
-  *go_away_compiler = 'a';
-#elif defined(__x86_64__)
-  // Cause a SEGV using an instruction known to be 2 bytes long to account for hardcoded jump
-  // in the signal handler
-  asm volatile("movl $0, %%eax;" "movb %%ah, (%%rax);" : : : "%eax");
-#else
-  // On other architectures we simulate SEGV.
-  kill(getpid(), SIGSEGV);
-#endif
+  raise_sigsegv();
 
   // Test sigill
   sigaction(SIGILL, &tmp, nullptr);
@@ -221,6 +226,135 @@ static jint trampoline_Java_Main_testSignal(JNIEnv*, jclass) {
   return 1234;
 }
 
+// Status of the tricky control path of testSignalHandlerNotReturn.
+//
+// "kNone" is the default status except testSignalHandlerNotReturn,
+// others are used by testSignalHandlerNotReturn.
+enum class TestStatus {
+  kNone,
+  kRaiseFirst,
+  kHandleFirst,
+  kRaiseSecond,
+  kHandleSecond,
+};
+
+// State transition helper for testSignalHandlerNotReturn.
+class SignalHandlerTestStatus {
+ public:
+  SignalHandlerTestStatus() : state_(TestStatus::kNone) {
+  }
+
+  TestStatus Get() {
+    return state_;
+  }
+
+  void Reset() {
+    Set(TestStatus::kNone);
+  }
+
+  void Set(TestStatus state) {
+    switch (state) {
+      case TestStatus::kNone:
+        AssertState(TestStatus::kHandleSecond);
+        break;
+
+      case TestStatus::kRaiseFirst:
+        AssertState(TestStatus::kNone);
+        break;
+
+      case TestStatus::kHandleFirst:
+        AssertState(TestStatus::kRaiseFirst);
+        break;
+
+      case TestStatus::kRaiseSecond:
+        AssertState(TestStatus::kHandleFirst);
+        break;
+
+      case TestStatus::kHandleSecond:
+        AssertState(TestStatus::kRaiseSecond);
+        break;
+
+      default:
+        printf("ERROR: unknown state\n");
+        abort();
+    }
+
+    state_ = state;
+  }
+
+ private:
+  TestStatus state_;
+
+  void AssertState(TestStatus expected) {
+    if (state_ != expected) {
+      printf("ERROR: unexpected state, was %d, expected %d\n", state_, expected);
+    }
+  }
+};
+
+static SignalHandlerTestStatus gSignalTestStatus;
+// The context is used to jump out from signal handler.
+static sigjmp_buf gSignalTestJmpBuf;
+
+// Test whether NativeBridge can receive future signal when its handler doesn't return.
+//
+// Control path:
+//  1. Raise first SIGSEGV in test function.
+//  2. Raise another SIGSEGV in NativeBridge's signal handler which is handling
+//     the first SIGSEGV.
+//  3. Expect that NativeBridge's signal handler invokes again. And jump back
+//     to test function in when handling second SIGSEGV.
+//  4. Exit test.
+//
+// NOTE: sigchain should be aware that "special signal handler" may not return.
+//       Pay attention if this case fails.
+static void trampoline_Java_Main_testSignalHandlerNotReturn(JNIEnv*, jclass) {
+  if (gSignalTestStatus.Get() != TestStatus::kNone) {
+    printf("ERROR: test already started?\n");
+    return;
+  }
+  printf("start testSignalHandlerNotReturn\n");
+
+  if (sigsetjmp(gSignalTestJmpBuf, 1) == 0) {
+    gSignalTestStatus.Set(TestStatus::kRaiseFirst);
+    printf("raising first SIGSEGV\n");
+    raise_sigsegv();
+  } else {
+    // jump to here from signal handler when handling second SIGSEGV.
+    if (gSignalTestStatus.Get() != TestStatus::kHandleSecond) {
+      printf("ERROR: not jump from second SIGSEGV?\n");
+      return;
+    }
+    gSignalTestStatus.Reset();
+    printf("back to test from signal handler via siglongjmp(), and done!\n");
+  }
+}
+
+// Signal handler for testSignalHandlerNotReturn.
+// This handler won't return.
+static bool NotReturnSignalHandler() {
+  if (gSignalTestStatus.Get() == TestStatus::kRaiseFirst) {
+    // handling first SIGSEGV
+    gSignalTestStatus.Set(TestStatus::kHandleFirst);
+    printf("handling first SIGSEGV, will raise another\n");
+    sigset_t set;
+    sigemptyset(&set);
+    sigaddset(&set, SIGSEGV);
+    printf("unblock SIGSEGV in handler\n");
+    sigprocmask(SIG_UNBLOCK, &set, nullptr);
+    gSignalTestStatus.Set(TestStatus::kRaiseSecond);
+    printf("raising second SIGSEGV\n");
+    raise_sigsegv();    // raise second SIGSEGV
+  } else if (gSignalTestStatus.Get() == TestStatus::kRaiseSecond) {
+    // handling second SIGSEGV
+    gSignalTestStatus.Set(TestStatus::kHandleSecond);
+    printf("handling second SIGSEGV, will jump back to test function\n");
+    siglongjmp(gSignalTestJmpBuf, 1);
+  }
+  printf("ERROR: should not reach here!\n");
+  return false;
+}
+
 NativeBridgeMethod gNativeBridgeMethods[] = {
   { "JNI_OnLoad", "", true, nullptr,
     reinterpret_cast<void*>(trampoline_JNI_OnLoad) },
@@ -246,6 +380,8 @@ NativeBridgeMethod gNativeBridgeMethods[] = {
     reinterpret_cast<void*>(trampoline_Java_Main_testZeroLengthByteBuffers) },
   { "testSignal", "()I", true, nullptr,
     reinterpret_cast<void*>(trampoline_Java_Main_testSignal) },
+  { "testSignalHandlerNotReturn", "()V", true, nullptr,
+    reinterpret_cast<void*>(trampoline_Java_Main_testSignalHandlerNotReturn) },
 };
 
 static NativeBridgeMethod* find_native_bridge_method(const char *name) {
@@ -285,6 +421,10 @@ extern "C" bool native_bridge_initialize(const android::NativeBridgeRuntimeCallb
 }
 
 extern "C" void* native_bridge_loadLibrary(const char* libpath, int flag) {
+  if (strstr(libpath, "libinvalid.so") != nullptr) {
+    printf("Was to load 'libinvalid.so', force fail.\n");
+    return nullptr;
+  }
   size_t len = strlen(libpath);
   char* tmp = new char[len + 10];
   strncpy(tmp, libpath, len);
@@ -300,7 +440,7 @@ extern "C" void* native_bridge_loadLibrary(const char* libpath, int flag) {
     printf("Handle = nullptr!\n");
     printf("Was looking for %s.\n", libpath);
     printf("Error = %s.\n", dlerror());
-    char cwd[1024];
+    char cwd[1024] = {'\0'};
     if (getcwd(cwd, sizeof(cwd)) != nullptr) {
       printf("Current working dir: %s\n", cwd);
     }
@@ -395,10 +535,8 @@ extern "C" bool native_bridge_isCompatibleWith(uint32_t bridge_version ATTRIBUTE
 #endif
 #endif
 
-// A dummy special handler, continueing after the faulting location. This code comes from
-// 004-SignalTest.
-static bool nb_signalhandler(int sig, siginfo_t* info ATTRIBUTE_UNUSED, void* context) {
-  printf("NB signal handler with signal %d.\n", sig);
+static bool StandardSignalHandler(int sig, siginfo_t* info ATTRIBUTE_UNUSED,
+                                     void* context) {
   if (sig == SIGSEGV) {
 #if defined(__arm__)
     struct ucontext *uc = reinterpret_cast<struct ucontext*>(context);
@@ -423,6 +561,21 @@ static bool nb_signalhandler(int sig, siginfo_t* info ATTRIBUTE_UNUSED, void* co
   return true;
 }
 
+// A dummy special handler, continueing after the faulting location. This code comes from
+// 004-SignalTest.
+static bool nb_signalhandler(int sig, siginfo_t* info, void* context) {
+  printf("NB signal handler with signal %d.\n", sig);
+
+  if (gSignalTestStatus.Get() == TestStatus::kNone) {
+    return StandardSignalHandler(sig, info, context);
+  } else if (sig == SIGSEGV) {
+    return NotReturnSignalHandler();
+  } else {
+    printf("ERROR: should not reach here!\n");
+    return false;
+  }
+}
+
 static ::android::NativeBridgeSignalHandlerFn native_bridge_getSignalHandler(int signal) {
   // Test segv for already claimed signal, and sigill for not claimed signal
   if ((signal == SIGSEGV) || (signal == SIGILL)) {
@@ -437,8 +590,8 @@ extern "C" int native_bridge_unloadLibrary(void* handle ATTRIBUTE_UNUSED) {
 }
 
 extern "C" const char* native_bridge_getError() {
-  printf("dlerror() in native bridge.\n");
-  return nullptr;
+  printf("getError() in native bridge.\n");
+  return "";
 }
 
 extern "C" bool native_bridge_isPathSupported(const char* library_path ATTRIBUTE_UNUSED) {
index 9290dd3..22f5c67 100644 (file)
@@ -23,6 +23,7 @@ LIBPATH=${LIBPATH##*:}
 ln -sf ${LIBPATH}/libnativebridgetest.so .
 touch libarttest.so
 touch libarttestd.so
+touch libinvalid.so
 ln -sf ${LIBPATH}/libarttest.so libarttest2.so
 ln -sf ${LIBPATH}/libarttestd.so libarttestd2.so
 
index c298b1b..11eaa84 100644 (file)
@@ -16,6 +16,7 @@
 
 import java.lang.reflect.Method;
 import java.lang.System;
+import java.lang.Exception;
 
 // This is named Main as it is a copy of JniTest, so that we can re-use the native implementations
 // from libarttest.
@@ -33,6 +34,8 @@ class Main {
         testEnvironment();
         testNewStringObject();
         testSignalHandler();
+        testGetErrorByLoadInvalidLibrary();
+        testSignalHandlerNotReturn();
     }
 
     public static native void testFindClassOnAttachedNativeThread();
@@ -183,6 +186,22 @@ class Main {
     }
 
     private static native int testSignal();
+
+    // Test the path from Java to getError() of NativeBridge.
+    //
+    // Load invalid library 'libinvalid.so' from Java. Library loading will fail since it's
+    // invalid (empty file). ART, NativeLoader actually, calls getError() to dump error message.
+    // After that in Java, catch UnsatisfiedLinkError exception to confirm.
+    private static void testGetErrorByLoadInvalidLibrary() {
+        System.out.println("Loading invalid library 'libinvalid.so' from Java, which will fail.");
+        try {
+            System.loadLibrary("invalid");
+        } catch (java.lang.UnsatisfiedLinkError e){
+            System.out.println("Catch UnsatisfiedLinkError exception as expected.");
+        }
+    }
+
+    private static native void testSignalHandlerNotReturn();
 }
 
 public class NativeBridgeMain {