[compiler-rt] r331701 - [sanitizer] Simplify ThreadLister interface
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Mon May 7 16:29:48 PDT 2018
Author: vitalybuka
Date: Mon May 7 16:29:48 2018
New Revision: 331701
URL: http://llvm.org/viewvc/llvm-project?rev=331701&view=rev
Log:
[sanitizer] Simplify ThreadLister interface
Reviewers: eugenis
Subscribers: kubamracek, llvm-commits
Differential Revision: https://reviews.llvm.org/D46516
Modified:
compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc
compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h
compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc?rev=331701&r1=331700&r2=331701&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc Mon May 7 16:29:48 2018
@@ -905,47 +905,40 @@ bool internal_sigismember(__sanitizer_si
#endif // !SANITIZER_SOLARIS
// ThreadLister implementation.
-ThreadLister::ThreadLister(int pid)
- : pid_(pid),
- descriptor_(-1),
- buffer_(4096),
- error_(true),
- entry_((struct linux_dirent *)buffer_.data()),
- bytes_read_(0) {
+ThreadLister::ThreadLister(int pid) : pid_(pid), buffer_(4096) {
+ buffer_.resize(buffer_.capacity());
char task_directory_path[80];
internal_snprintf(task_directory_path, sizeof(task_directory_path),
"/proc/%d/task/", pid);
- uptr openrv = internal_open(task_directory_path, O_RDONLY | O_DIRECTORY);
- if (internal_iserror(openrv)) {
- error_ = true;
+ descriptor_ = internal_open(task_directory_path, O_RDONLY | O_DIRECTORY);
+ if (internal_iserror(descriptor_)) {
Report("Can't open /proc/%d/task for reading.\n", pid);
- } else {
- error_ = false;
- descriptor_ = openrv;
}
}
-int ThreadLister::GetNextTID() {
- int tid = -1;
- do {
- if (error_)
- return -1;
- if ((char *)entry_ >= &buffer_[bytes_read_] && !GetDirectoryEntries())
- return -1;
- if (entry_->d_ino != 0 && entry_->d_name[0] >= '0' &&
- entry_->d_name[0] <= '9') {
- // Found a valid tid.
- tid = (int)internal_atoll(entry_->d_name);
+bool ThreadLister::ListThreads(InternalMmapVector<int> *threads) {
+ if (!descriptor_) return false;
+ internal_lseek(descriptor_, 0, SEEK_SET);
+ threads->clear();
+
+ while (uptr read = internal_getdents(descriptor_,
+ (struct linux_dirent *)buffer_.data(),
+ buffer_.size())) {
+ if (internal_iserror(read)) {
+ Report("Can't read directory entries from /proc/%d/task.\n", pid_);
+ return false;
}
- entry_ = (struct linux_dirent *)(((char *)entry_) + entry_->d_reclen);
- } while (tid < 0);
- return tid;
-}
-void ThreadLister::Reset() {
- if (error_ || descriptor_ < 0)
- return;
- internal_lseek(descriptor_, 0, SEEK_SET);
+ for (uptr begin = (uptr)buffer_.data(), end = begin + read; begin < end;) {
+ struct linux_dirent *entry = (struct linux_dirent *)begin;
+ begin += entry->d_reclen;
+ if (entry->d_ino && *entry->d_name >= '0' && *entry->d_name <= '9') {
+ // Found a valid tid.
+ threads->push_back(internal_atoll(entry->d_name));
+ }
+ }
+ }
+ return true;
}
ThreadLister::~ThreadLister() {
@@ -953,24 +946,6 @@ ThreadLister::~ThreadLister() {
internal_close(descriptor_);
}
-bool ThreadLister::error() { return error_; }
-
-bool ThreadLister::GetDirectoryEntries() {
- CHECK_GE(descriptor_, 0);
- CHECK_NE(error_, true);
- bytes_read_ = internal_getdents(
- descriptor_, (struct linux_dirent *)buffer_.data(), buffer_.size());
- if (internal_iserror(bytes_read_)) {
- Report("Can't read directory entries from /proc/%d/task.\n", pid_);
- error_ = true;
- return false;
- } else if (bytes_read_ == 0) {
- return false;
- }
- entry_ = (struct linux_dirent *)buffer_.data();
- return true;
-}
-
#if SANITIZER_WORDSIZE == 32
// Take care of unusable kernel area in top gigabyte.
static uptr GetKernelAreaSize() {
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h?rev=331701&r1=331700&r2=331701&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h Mon May 7 16:29:48 2018
@@ -76,21 +76,12 @@ class ThreadLister {
public:
explicit ThreadLister(int pid);
~ThreadLister();
- // GetNextTID returns -1 if the list of threads is exhausted, or if there has
- // been an error.
- int GetNextTID();
- void Reset();
- bool error();
+ bool ListThreads(InternalMmapVector<int> *threads);
private:
- bool GetDirectoryEntries();
-
int pid_;
- int descriptor_;
+ int descriptor_ = -1;
InternalMmapVector<char> buffer_;
- bool error_;
- struct linux_dirent* entry_;
- int bytes_read_;
};
// Exposed for testing.
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc?rev=331701&r1=331700&r2=331701&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc Mon May 7 16:29:48 2018
@@ -211,21 +211,23 @@ bool ThreadSuspender::SuspendAllThreads(
ThreadLister thread_lister(pid_);
bool added_threads;
bool first_iteration = true;
+ InternalMmapVector<int> threads;
+ threads.reserve(128);
do {
// Run through the directory entries once.
added_threads = false;
- pid_t tid = thread_lister.GetNextTID();
- while (tid >= 0) {
+ if (!thread_lister.ListThreads(&threads)) {
+ ResumeAllThreads();
+ return false;
+ }
+ for (int tid : threads)
if (SuspendThread(tid))
added_threads = true;
- tid = thread_lister.GetNextTID();
- }
- if (thread_lister.error() || (first_iteration && !added_threads)) {
+ if (first_iteration && !added_threads) {
// Detach threads and fail.
ResumeAllThreads();
return false;
}
- thread_lister.Reset();
first_iteration = false;
} while (added_threads);
return true;
Modified: compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc?rev=331701&r1=331700&r2=331701&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc Mon May 7 16:29:48 2018
@@ -123,11 +123,9 @@ void ThreadListerTest::SpawnTidReporter(
static std::vector<pid_t> ReadTidsToVector(ThreadLister *thread_lister) {
std::vector<pid_t> listed_tids;
- pid_t tid;
- while ((tid = thread_lister->GetNextTID()) >= 0)
- listed_tids.push_back(tid);
- EXPECT_FALSE(thread_lister->error());
- return listed_tids;
+ InternalMmapVector<int> threads(128);
+ EXPECT_TRUE(thread_lister->ListThreads(&threads));
+ return std::vector<pid_t>(threads.begin(), threads.end());
}
static bool Includes(std::vector<pid_t> first, std::vector<pid_t> second) {
@@ -151,23 +149,20 @@ TEST_F(ThreadListerTest, ThreadListerSee
ASSERT_TRUE(Includes(listed_tids, tids_));
}
-// Calling Reset() should not cause ThreadLister to forget any threads it's
-// supposed to know about.
-TEST_F(ThreadListerTest, ResetDoesNotForgetThreads) {
+TEST_F(ThreadListerTest, DoNotForgetThreads) {
ThreadLister thread_lister(getpid());
- // Run the loop body twice, because Reset() might behave differently if called
- // on a freshly created object.
+ // Run the loop body twice, because ThreadLister might behave differently if
+ // called on a freshly created object.
for (uptr i = 0; i < 2; i++) {
- thread_lister.Reset();
std::vector<pid_t> listed_tids = ReadTidsToVector(&thread_lister);
ASSERT_TRUE(Includes(listed_tids, tids_));
}
}
// If new threads have spawned during ThreadLister object's lifetime, calling
-// Reset() should cause ThreadLister to recognize their existence.
-TEST_F(ThreadListerTest, ResetMakesNewThreadsKnown) {
+// relisting should cause ThreadLister to recognize their existence.
+TEST_F(ThreadListerTest, NewThreads) {
ThreadLister thread_lister(getpid());
std::vector<pid_t> threads_before_extra = ReadTidsToVector(&thread_lister);
@@ -182,8 +177,6 @@ TEST_F(ThreadListerTest, ResetMakesNewTh
// so better check for that.
ASSERT_FALSE(HasElement(threads_before_extra, extra_tid));
- thread_lister.Reset();
-
std::vector<pid_t> threads_after_extra = ReadTidsToVector(&thread_lister);
ASSERT_TRUE(HasElement(threads_after_extra, extra_tid));
}
More information about the llvm-commits
mailing list