[sanitizer][NFC] Remove InternalScopedString::size()
authorVitaly Buka <vitalybuka@google.com>
Tue, 16 Mar 2021 11:03:45 +0000 (04:03 -0700)
committerVitaly Buka <vitalybuka@google.com>
Tue, 16 Mar 2021 21:11:59 +0000 (14:11 -0700)
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.

12 files changed:
compiler-rt/lib/sanitizer_common/sanitizer_common.h
compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cpp
compiler-rt/lib/sanitizer_common/sanitizer_libignore.cpp
compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp
compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp
compiler-rt/lib/sanitizer_common/sanitizer_procmaps_common.cpp
compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp
compiler-rt/lib/sanitizer_common/sanitizer_suppressions.cpp
compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
compiler-rt/lib/ubsan/ubsan_monitor.cpp

index e8a1555..a9ecd2a 100644 (file)
@@ -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<char> buffer_;
index 047c5a1..9b3d059 100644 (file)
@@ -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;
index 9ea19bc..a65d3d8 100644 (file)
@@ -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<char> buf(kMaxPathLength);
   if (name && internal_readlink(name, buf.data(), buf.size() - 1) > 0 &&
       buf[0]) {
     for (uptr i = 0; i < count_; i++) {
index 2b1224b..b349d0d 100644 (file)
@@ -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<LoadedModule> *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<char> 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;
 }
 
index b0d7bcc..535b8c2 100644 (file)
@@ -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<char> 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<char> program_name(1024);
     uint32_t buf_size = program_name.size();
     _NSGetExecutablePath(program_name.data(), &buf_size);
     char *new_env = const_cast<char*>(info.dli_fname);
index 2e08009..f8457a6 100644 (file)
@@ -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<char> buff(kMaxPathLength);
+  MemoryMappedSegment segment(buff.data(), buff.size());
   while (proc_maps.Next(&segment)) {
     if (segment.IsExecutable() &&
         internal_strcmp(module, segment.filename) == 0) {
index a032787..ae21f6c 100644 (file)
@@ -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
index f2cfcff..1b7dd46 100644 (file)
@@ -120,7 +120,7 @@ void MemoryMappingLayout::LoadFromCache() {
 void MemoryMappingLayout::DumpListOfModules(
     InternalMmapVectorNoCtor<LoadedModule> *modules) {
   Reset();
-  InternalScopedString module_name(kMaxPathLength);
+  InternalMmapVector<char> module_name(kMaxPathLength);
   MemoryMappedSegment segment(module_name.data(), module_name.size());
   for (uptr i = 0; Next(&segment); i++) {
     const char *cur_name = segment.filename;
index d02afcf..1f53e3e 100644 (file)
@@ -354,8 +354,8 @@ bool MemoryMappingLayout::Next(MemoryMappedSegment *segment) {
 void MemoryMappingLayout::DumpListOfModules(
     InternalMmapVectorNoCtor<LoadedModule> *modules) {
   Reset();
-  InternalScopedString module_name(kMaxPathLength);
-  MemoryMappedSegment segment(module_name.data(), kMaxPathLength);
+  InternalMmapVector<char> module_name(kMaxPathLength);
+  MemoryMappedSegment segment(module_name.data(), module_name.size());
   MemoryMappedSegmentData data;
   segment.data_ = &data;
   while (Next(&segment)) {
index 44c83a6..a674034 100644 (file)
@@ -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<char> 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<char> new_file_path(kMaxPathLength);
   filename = FindFile(filename, new_file_path.data(), new_file_path.size());
 
   // Read the file.
index 4dda620..d0ba96b 100644 (file)
@@ -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;
       }
index d064e95..0b0ab50 100644 (file)
@@ -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<char *>(Buf.data()) += 'A' - 'a';
 
   *OutIssueKind = CurrentUBR->IssueKind;
   *OutMessage = Buf.data();