From f5e6182ce6cd16080c62f6c8d599c09971904150 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Tue, 16 Mar 2021 04:03:45 -0700 Subject: [PATCH] [sanitizer][NFC] Remove InternalScopedString::size() size() is inconsistent with length(). In most size() use cases we can replace InternalScopedString with InternalMmapVector. Remove non-constant data() to avoid direct manipulations of internal buffer. append() should be enought to modify InternalScopedString. --- .../lib/sanitizer_common/sanitizer_common.h | 6 +--- .../sanitizer_common_libcdep.cpp | 5 ++- .../sanitizer_common/sanitizer_libignore.cpp | 2 +- .../sanitizer_linux_libcdep.cpp | 35 ++++++++++++------- .../lib/sanitizer_common/sanitizer_mac.cpp | 4 +-- .../lib/sanitizer_common/sanitizer_posix.cpp | 4 +-- .../lib/sanitizer_common/sanitizer_printf.cpp | 6 ++-- .../sanitizer_procmaps_common.cpp | 2 +- .../sanitizer_procmaps_mac.cpp | 4 +-- .../sanitizer_suppressions.cpp | 4 +-- compiler-rt/lib/tsan/rtl/tsan_rtl.cpp | 2 +- compiler-rt/lib/ubsan/ubsan_monitor.cpp | 4 +-- 12 files changed, 42 insertions(+), 36 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h index e8a15556d161..a9ecd2ad2da5 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h @@ -598,17 +598,13 @@ class InternalScopedString { : buffer_(max_length), length_(0) { buffer_[0] = '\0'; } - uptr size() const { return buffer_.size(); } uptr length() const { return length_; } void clear() { - (*this)[0] = '\0'; + buffer_[0] = '\0'; length_ = 0; } void append(const char *format, ...); - char *data() { return buffer_.data(); } const char *data() const { return buffer_.data(); } - char &operator[](uptr i) { return buffer_[i]; } - const char &operator[](uptr i) const { return buffer_[i]; } private: InternalMmapVector buffer_; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cpp index 047c5a17ea6e..9b3d05950c41 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cpp @@ -94,12 +94,11 @@ void *BackgroundThread(void *arg) { void WriteToSyslog(const char *msg) { InternalScopedString msg_copy(kErrorMessageBufferSize); msg_copy.append("%s", msg); - char *p = msg_copy.data(); - char *q; + const char *p = msg_copy.data(); // Print one line at a time. // syslog, at least on Android, has an implicit message length limit. - while ((q = internal_strchr(p, '\n'))) { + while (char* q = internal_strchr(p, '\n')) { *q = '\0'; WriteOneLineToSyslog(p); p = q + 1; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_libignore.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_libignore.cpp index 9ea19bc21fa3..a65d3d896e33 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_libignore.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_libignore.cpp @@ -38,7 +38,7 @@ void LibIgnore::AddIgnoredLibrary(const char *name_templ) { void LibIgnore::OnLibraryLoaded(const char *name) { BlockingMutexLock lock(&mutex_); // Try to match suppressions with symlink target. - InternalScopedString buf(kMaxPathLength); + InternalMmapVector buf(kMaxPathLength); if (name && internal_readlink(name, buf.data(), buf.size() - 1) > 0 && buf[0]) { for (uptr i = 0; i < count_; i++) { diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp index 2b1224babf18..b349d0d1243c 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp @@ -574,20 +574,12 @@ struct DlIteratePhdrData { bool first; }; -static int dl_iterate_phdr_cb(dl_phdr_info *info, size_t size, void *arg) { - DlIteratePhdrData *data = (DlIteratePhdrData*)arg; - InternalScopedString module_name(kMaxPathLength); - if (data->first) { - data->first = false; - // First module is the binary itself. - ReadBinaryNameCached(module_name.data(), module_name.size()); - } else if (info->dlpi_name) { - module_name.append("%s", info->dlpi_name); - } +static int AddModuleSegments(const char *module_name, dl_phdr_info *info, + InternalMmapVectorNoCtor *modules) { if (module_name[0] == '\0') return 0; LoadedModule cur_module; - cur_module.set(module_name.data(), info->dlpi_addr); + cur_module.set(module_name, info->dlpi_addr); for (int i = 0; i < (int)info->dlpi_phnum; i++) { const Elf_Phdr *phdr = &info->dlpi_phdr[i]; if (phdr->p_type == PT_LOAD) { @@ -599,7 +591,26 @@ static int dl_iterate_phdr_cb(dl_phdr_info *info, size_t size, void *arg) { writable); } } - data->modules->push_back(cur_module); + modules->push_back(cur_module); + return 0; +} + +static int dl_iterate_phdr_cb(dl_phdr_info *info, size_t size, void *arg) { + DlIteratePhdrData *data = (DlIteratePhdrData *)arg; + if (data->first) { + InternalMmapVector module_name(kMaxPathLength); + data->first = false; + // First module is the binary itself. + ReadBinaryNameCached(module_name.data(), module_name.size()); + return AddModuleSegments(module_name.data(), info, data->modules); + } + + if (info->dlpi_name) { + InternalScopedString module_name(kMaxPathLength); + module_name.append("%s", info->dlpi_name); + return AddModuleSegments(module_name.data(), info, data->modules); + } + return 0; } diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp index b0d7bcc645fa..535b8c218696 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp @@ -453,7 +453,7 @@ uptr ReadBinaryName(/*out*/char *buf, uptr buf_len) { // On OS X the executable path is saved to the stack by dyld. Reading it // from there is much faster than calling dladdr, especially for large // binaries with symbols. - InternalScopedString exe_path(kMaxPathLength); + InternalMmapVector exe_path(kMaxPathLength); uint32_t size = exe_path.size(); if (_NSGetExecutablePath(exe_path.data(), &size) == 0 && realpath(exe_path.data(), buf) != 0) { @@ -1019,7 +1019,7 @@ void MaybeReexec() { if (DyldNeedsEnvVariable() && !lib_is_in_env) { // DYLD_INSERT_LIBRARIES is not set or does not contain the runtime // library. - InternalScopedString program_name(1024); + InternalMmapVector program_name(1024); uint32_t buf_size = program_name.size(); _NSGetExecutablePath(program_name.data(), &buf_size); char *new_env = const_cast(info.dli_fname); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp index 2e080098283f..f8457a6aac41 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp @@ -275,8 +275,8 @@ void ReportFile::Write(const char *buffer, uptr length) { bool GetCodeRangeForFile(const char *module, uptr *start, uptr *end) { MemoryMappingLayout proc_maps(/*cache_enabled*/false); - InternalScopedString buff(kMaxPathLength); - MemoryMappedSegment segment(buff.data(), kMaxPathLength); + InternalMmapVector buff(kMaxPathLength); + MemoryMappedSegment segment(buff.data(), buff.size()); while (proc_maps.Next(&segment)) { if (segment.IsExecutable() && internal_strcmp(module, segment.filename) == 0) { diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp index a032787114bb..ae21f6cddc43 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp @@ -346,13 +346,13 @@ int internal_snprintf(char *buffer, uptr length, const char *format, ...) { FORMAT(2, 3) void InternalScopedString::append(const char *format, ...) { - CHECK_LT(length_, size()); + CHECK_LT(length_, buffer_.size()); va_list args; va_start(args, format); - VSNPrintf(data() + length_, size() - length_, format, args); + VSNPrintf(buffer_.data() + length_, buffer_.size() - length_, format, args); va_end(args); length_ += internal_strlen(data() + length_); - CHECK_LT(length_, size()); + CHECK_LT(length_, buffer_.size()); } } // namespace __sanitizer diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_common.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_common.cpp index f2cfcffaf476..1b7dd46d8de4 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_common.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_common.cpp @@ -120,7 +120,7 @@ void MemoryMappingLayout::LoadFromCache() { void MemoryMappingLayout::DumpListOfModules( InternalMmapVectorNoCtor *modules) { Reset(); - InternalScopedString module_name(kMaxPathLength); + InternalMmapVector module_name(kMaxPathLength); MemoryMappedSegment segment(module_name.data(), module_name.size()); for (uptr i = 0; Next(&segment); i++) { const char *cur_name = segment.filename; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp index d02afcfe87ae..1f53e3e46d8f 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp @@ -354,8 +354,8 @@ bool MemoryMappingLayout::Next(MemoryMappedSegment *segment) { void MemoryMappingLayout::DumpListOfModules( InternalMmapVectorNoCtor *modules) { Reset(); - InternalScopedString module_name(kMaxPathLength); - MemoryMappedSegment segment(module_name.data(), kMaxPathLength); + InternalMmapVector module_name(kMaxPathLength); + MemoryMappedSegment segment(module_name.data(), module_name.size()); MemoryMappedSegmentData data; segment.data_ = &data; while (Next(&segment)) { diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_suppressions.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_suppressions.cpp index 44c83a66c5fe..a674034b8e29 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_suppressions.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_suppressions.cpp @@ -34,7 +34,7 @@ SuppressionContext::SuppressionContext(const char *suppression_types[], static bool GetPathAssumingFileIsRelativeToExec(const char *file_path, /*out*/char *new_file_path, uptr new_file_path_size) { - InternalScopedString exec(kMaxPathLength); + InternalMmapVector exec(kMaxPathLength); if (ReadBinaryNameCached(exec.data(), exec.size())) { const char *file_name_pos = StripModuleName(exec.data()); uptr path_to_exec_len = file_name_pos - exec.data(); @@ -69,7 +69,7 @@ void SuppressionContext::ParseFromFile(const char *filename) { if (filename[0] == '\0') return; - InternalScopedString new_file_path(kMaxPathLength); + InternalMmapVector new_file_path(kMaxPathLength); filename = FindFile(filename, new_file_path.data(), new_file_path.size()); // Read the file. diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp index 4dda62054d8d..d0ba96b06ef6 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp @@ -172,7 +172,7 @@ static void *BackgroundThread(void *arg) { fd_t fd = OpenFile(filename.data(), WrOnly); if (fd == kInvalidFd) { Printf("ThreadSanitizer: failed to open memory profile file '%s'\n", - &filename[0]); + filename.data()); } else { mprof_fd = fd; } diff --git a/compiler-rt/lib/ubsan/ubsan_monitor.cpp b/compiler-rt/lib/ubsan/ubsan_monitor.cpp index d064e95f76f7..0b0ab50d6ecc 100644 --- a/compiler-rt/lib/ubsan/ubsan_monitor.cpp +++ b/compiler-rt/lib/ubsan/ubsan_monitor.cpp @@ -52,9 +52,9 @@ void __ubsan::__ubsan_get_current_report_data(const char **OutIssueKind, // Ensure that the first character of the diagnostic text can't start with a // lowercase letter. - char FirstChar = Buf.data()[0]; + char FirstChar = *Buf.data(); if (FirstChar >= 'a' && FirstChar <= 'z') - Buf.data()[0] = FirstChar - 'a' + 'A'; + *const_cast(Buf.data()) += 'A' - 'a'; *OutIssueKind = CurrentUBR->IssueKind; *OutMessage = Buf.data();