Update ReadProcMaps() to reflect lack of atomicity when reading /proc/self/maps.

base/debug/proc_maps_linux.cc

Index: base/debug/proc_maps_linux.cc
diff --git a/base/debug/proc_maps_linux.cc b/base/debug/proc_maps_linux.cc
index 9557feb025994b1e88763b3c52f855127eaeda81..562189ff72b9c90470a343d06b4626bda1476a30 100644
--- a/base/debug/proc_maps_linux.cc
+++ b/base/debug/proc_maps_linux.cc
@@ -22,7 +22,20 @@
 namespace base {
 namespace debug {
 
+// Local testing revealed that the size of /proc/<pid>/maps for an official
+// release build of chrome hovered around the ~45 KB mark with some processes
+// hitting 80-90 KB. We'll play it safe and use 256 KB.
+enum { kLargeProcMapsSize = 256 * 1024 };

[Mark Mentovai, 2013/07/11 03:47:50]

Why an enum? Why not a const?

And why is it out here? You can put it in ReadProcMaps, since that’s the only
place it’s used.

+
 bool ReadProcMaps(std::string* proc_maps) {
+  // It's possible for /proc/self/maps to change during reading (e.g., when
+  // running under ThreadSanitizer). Resize |proc_maps| to be large enough
+  // to read without causing a reallocation.
+  //
+  // Refer to http://crbug.com/258451 for details.
+  proc_maps->clear();
+  proc_maps->reserve(kLargeProcMapsSize);

[Mark Mentovai, 2013/07/11 03:47:50]

How is this supposed to help…?

+
   FilePath proc_maps_path("/proc/self/maps");
   return file_util::ReadFileToString(proc_maps_path, proc_maps);

[Mark Mentovai, 2013/07/11 03:47:50]

…file_util::ReadFileToString still reads chunks of at most 64kB at a time. It
doesn’t care about the size of the “contents” buffer. All the “reserve” does is
make sure that the std::string::append calls don’t result in internal
reallocations until at least 256kB has been read.

It’s not enough to avoid reallocations, you really need to gobble up the entire
contents of the file in a single read operation.

[Alexander Potapenko, 2013/07/11 10:28:54]

Maybe base::MemoryMappedFile could be used instead?
This is still non-atomic, because the size of /proc/self/maps may change between
the fstat() and mmap() calls, so we may need to pass some default size to
MapFileToMemoryInternal.

[Mark Mentovai, 2013/07/11 14:20:40]

Alexander Potapenko wrote:
/proc is not a regular filesystem. There are two problems with this approach.

1. You can’t use stat (or fstat) to figure out how big the file is, because
there is no file. The kernel won’t assemble any data until you try to read the
file, so it doesn’t even know the answer. st_size will always be 0 for this
file, and for most or all other files in /proc.

2. You can’t mmap the file, because it doesn’t implement mmap.
(fs/proc/task_mmu.c proc_tid_maps_operations). You’ll get ENODEV. I don’t know
what semantics you would expect of mmap, anyway, since this isn’t a real file.

 }