[compiler-rt] r334130 - [sanitizer] Cleanup ReadFileToVector and ReadFileToBuffer
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 6 13:53:43 PDT 2018
Author: vitalybuka
Date: Wed Jun 6 13:53:43 2018
New Revision: 334130
URL: http://llvm.org/viewvc/llvm-project?rev=334130&view=rev
Log:
[sanitizer] Cleanup ReadFileToVector and ReadFileToBuffer
Summary:
Added unit-test.
Fixed behavior of max_len argument.
Call read syscall with all available buffer, not just a page.
Reviewers: eugenis
Subscribers: kubamracek, mgorny, llvm-commits
Differential Revision: https://reviews.llvm.org/D46618
Modified:
compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
compiler-rt/trunk/lib/sanitizer_common/sanitizer_file.cc
compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc
compiler-rt/trunk/lib/sanitizer_common/tests/CMakeLists.txt
compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_libc_test.cc
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h?rev=334130&r1=334129&r2=334130&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h Wed Jun 6 13:53:43 2018
@@ -639,18 +639,22 @@ enum ModuleArch {
// Opens the file 'file_name" and reads up to 'max_len' bytes.
// The resulting buffer is mmaped and stored in '*buff'.
+// Returns true if file was successfully opened and read.
+bool ReadFileToVector(const char *file_name,
+ InternalMmapVectorNoCtor<char> *buff,
+ uptr max_len = 1 << 26, error_t *errno_p = nullptr);
+
+// Opens the file 'file_name" and reads up to 'max_len' bytes.
+// This function is less I/O efficient than ReadFileToVector as it may reread
+// file multiple times to avoid mmap during read attempts. It's used to read
+// procmap, so short reads with mmap in between can produce inconsistent result.
+// The resulting buffer is mmaped and stored in '*buff'.
// The size of the mmaped region is stored in '*buff_size'.
// The total number of read bytes is stored in '*read_len'.
// Returns true if file was successfully opened and read.
bool ReadFileToBuffer(const char *file_name, char **buff, uptr *buff_size,
uptr *read_len, uptr max_len = 1 << 26,
error_t *errno_p = nullptr);
-// Opens the file 'file_name" and reads up to 'max_len' bytes.
-// The resulting buffer is mmaped and stored in '*buff'.
-// Returns true if file was successfully opened and read.
-bool ReadFileToBuffer(const char *file_name,
- InternalMmapVectorNoCtor<char> *buff,
- uptr max_len = 1 << 26, error_t *errno_p = nullptr);
// When adding a new architecture, don't forget to also update
// script/asan_symbolize.py and sanitizer_symbolizer_libcdep.cc.
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_file.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_file.cc?rev=334130&r1=334129&r2=334130&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_file.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_file.cc Wed Jun 6 13:53:43 2018
@@ -95,34 +95,40 @@ void ReportFile::SetReportPath(const cha
bool ReadFileToBuffer(const char *file_name, char **buff, uptr *buff_size,
uptr *read_len, uptr max_len, error_t *errno_p) {
- uptr PageSize = GetPageSizeCached();
- uptr kMinFileLen = PageSize;
*buff = nullptr;
*buff_size = 0;
*read_len = 0;
+ if (!max_len)
+ return true;
+ uptr PageSize = GetPageSizeCached();
+ uptr kMinFileLen = Min(PageSize, max_len);
+
// The files we usually open are not seekable, so try different buffer sizes.
- for (uptr size = kMinFileLen; size <= max_len; size *= 2) {
+ for (uptr size = kMinFileLen;; size = Min(size * 2, max_len)) {
UnmapOrDie(*buff, *buff_size);
*buff = (char*)MmapOrDie(size, __func__);
*buff_size = size;
fd_t fd = OpenFile(file_name, RdOnly, errno_p);
- if (fd == kInvalidFd)
+ if (fd == kInvalidFd) {
+ UnmapOrDie(*buff, *buff_size);
return false;
+ }
*read_len = 0;
// Read up to one page at a time.
bool reached_eof = false;
- while (*read_len + PageSize <= size) {
+ while (*read_len < size) {
uptr just_read;
- if (!ReadFromFile(fd, *buff + *read_len, PageSize, &just_read, errno_p)) {
+ if (!ReadFromFile(fd, *buff + *read_len, size - *read_len, &just_read,
+ errno_p)) {
UnmapOrDie(*buff, *buff_size);
CloseFile(fd);
return false;
}
- if (just_read == 0) {
+ *read_len += just_read;
+ if (just_read == 0 || *read_len == max_len) {
reached_eof = true;
break;
}
- *read_len += just_read;
}
CloseFile(fd);
if (reached_eof) // We've read the whole file.
@@ -131,40 +137,34 @@ bool ReadFileToBuffer(const char *file_n
return true;
}
-bool ReadFileToBuffer(const char *file_name,
+bool ReadFileToVector(const char *file_name,
InternalMmapVectorNoCtor<char> *buff, uptr max_len,
error_t *errno_p) {
- uptr PageSize = GetPageSizeCached();
buff->clear();
- // The files we usually open are not seekable, so try different buffer sizes.
- for (uptr size = Max(PageSize, buff->capacity()); size <= max_len;
- size *= 2) {
- buff->resize(size);
- fd_t fd = OpenFile(file_name, RdOnly, errno_p);
- if (fd == kInvalidFd)
+ if (!max_len)
+ return true;
+ uptr PageSize = GetPageSizeCached();
+ fd_t fd = OpenFile(file_name, RdOnly, errno_p);
+ if (fd == kInvalidFd)
+ return false;
+ uptr read_len = 0;
+ while (read_len < max_len) {
+ if (read_len >= buff->size())
+ buff->resize(Min(Max(PageSize, read_len * 2), max_len));
+ CHECK_LT(read_len, buff->size());
+ CHECK_LE(buff->size(), max_len);
+ uptr just_read;
+ if (!ReadFromFile(fd, buff->data() + read_len, buff->size() - read_len,
+ &just_read, errno_p)) {
+ CloseFile(fd);
return false;
- uptr read_len = 0;
- // Read up to one page at a time.
- bool reached_eof = false;
- while (read_len + PageSize <= buff->size()) {
- uptr just_read;
- if (!ReadFromFile(fd, buff->data() + read_len, PageSize, &just_read,
- errno_p)) {
- CloseFile(fd);
- return false;
- }
- if (!just_read) {
- reached_eof = true;
- break;
- }
- read_len += just_read;
}
- CloseFile(fd);
- if (reached_eof) { // We've read the whole file.
- buff->resize(read_len);
+ read_len += just_read;
+ if (!just_read)
break;
- }
}
+ CloseFile(fd);
+ buff->resize(read_len);
return true;
}
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=334130&r1=334129&r2=334130&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc Wed Jun 6 13:53:43 2018
@@ -975,7 +975,7 @@ bool ThreadLister::IsAlive(int tid) {
// proc_task_readdir. See task_state implementation in Linux.
char path[80];
internal_snprintf(path, sizeof(path), "/proc/%d/task/%d/status", pid_, tid);
- if (!ReadFileToBuffer(path, &buffer_) || buffer_.empty())
+ if (!ReadFileToVector(path, &buffer_) || buffer_.empty())
return false;
buffer_.push_back(0);
static const char kPrefix[] = "\nPPid:";
Modified: compiler-rt/trunk/lib/sanitizer_common/tests/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/tests/CMakeLists.txt?rev=334130&r1=334129&r2=334130&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/tests/CMakeLists.txt (original)
+++ compiler-rt/trunk/lib/sanitizer_common/tests/CMakeLists.txt Wed Jun 6 13:53:43 2018
@@ -54,7 +54,8 @@ set(SANITIZER_TEST_CFLAGS_COMMON
-fno-rtti
-O2
-Werror=sign-compare
- -Wno-non-virtual-dtor)
+ -Wno-non-virtual-dtor
+ -Wno-gnu-zero-variadic-macro-arguments)
if(MSVC)
# Disable exceptions on Windows until they work reliably.
Modified: compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_libc_test.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_libc_test.cc?rev=334130&r1=334129&r2=334130&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_libc_test.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_libc_test.cc Wed Jun 6 13:53:43 2018
@@ -9,6 +9,7 @@
// Tests for sanitizer_libc.h.
//===----------------------------------------------------------------------===//
#include <algorithm>
+#include <fstream>
#include "sanitizer_common/sanitizer_common.h"
#include "sanitizer_common/sanitizer_file.h"
@@ -82,8 +83,8 @@ static void temp_file_name(char *buf, si
// on Android already.
tmpdir = GetEnv("EXTERNAL_STORAGE");
#endif
- u32 uid = GetUid();
- internal_snprintf(buf, bufsize, "%s/%s%d", tmpdir, prefix, uid);
+ internal_snprintf(buf, bufsize, "%s/%sXXXXXX", tmpdir, prefix);
+ ASSERT_TRUE(mktemp(buf));
#endif
}
@@ -151,6 +152,64 @@ TEST(SanitizerCommon, FileOps) {
#endif
}
+class SanitizerCommonFileTest : public ::testing::TestWithParam<uptr> {
+ void SetUp() override {
+ data_.resize(GetParam());
+ std::generate(data_.begin(), data_.end(), [] {
+ return rand() % 256; // NOLINT
+ });
+
+ temp_file_name(file_name_, sizeof(file_name_),
+ "sanitizer_common.ReadFile.tmp.");
+
+ std::ofstream f(file_name_, std::ios::out | std::ios::binary);
+ if (!data_.empty())
+ f.write(data_.data(), data_.size());
+ }
+
+ void TearDown() override { internal_unlink(file_name_); }
+
+ protected:
+ char file_name_[256];
+ std::vector<char> data_;
+};
+
+TEST_P(SanitizerCommonFileTest, ReadFileToBuffer) {
+ char *buff;
+ uptr size;
+ uptr len;
+ EXPECT_TRUE(ReadFileToBuffer(file_name_, &buff, &len, &size));
+ EXPECT_EQ(data_, std::vector<char>(buff, buff + size));
+ UnmapOrDie(buff, len);
+}
+
+TEST_P(SanitizerCommonFileTest, ReadFileToBufferHalf) {
+ char *buff;
+ uptr size;
+ uptr len;
+ data_.resize(data_.size() / 2);
+ EXPECT_TRUE(ReadFileToBuffer(file_name_, &buff, &len, &size, data_.size()));
+ EXPECT_EQ(data_, std::vector<char>(buff, buff + size));
+ UnmapOrDie(buff, len);
+}
+
+TEST_P(SanitizerCommonFileTest, ReadFileToVector) {
+ InternalMmapVector<char> buff;
+ EXPECT_TRUE(ReadFileToVector(file_name_, &buff));
+ EXPECT_EQ(data_, std::vector<char>(buff.begin(), buff.end()));
+}
+
+TEST_P(SanitizerCommonFileTest, ReadFileToVectorHalf) {
+ InternalMmapVector<char> buff;
+ data_.resize(data_.size() / 2);
+ EXPECT_TRUE(ReadFileToVector(file_name_, &buff, data_.size()));
+ EXPECT_EQ(data_, std::vector<char>(buff.begin(), buff.end()));
+}
+
+INSTANTIATE_TEST_CASE_P(FileSizes, SanitizerCommonFileTest,
+ ::testing::Values(0, 1, 7, 13, 32, 4096, 4097, 1048575,
+ 1048576, 1048577));
+
static const size_t kStrlcpyBufSize = 8;
void test_internal_strlcpy(char *dbuf, const char *sbuf) {
uptr retval = 0;
More information about the llvm-commits
mailing list