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

base/debug/proc_maps_linux_unittest.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/debug/proc_maps_linux.h"
 #include "base/files/file_path.h"
 #include "base/path_service.h"
 #include "base/strings/stringprintf.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ skipping 162 matching lines @@
 
     std::vector<MappedMemoryRegion> regions;
     EXPECT_TRUE(ParseProcMaps(kTestCases[i].input, &regions));
     EXPECT_EQ(1u, regions.size());
     if (regions.empty())
       continue;
     EXPECT_EQ(kTestCases[i].permissions, regions[0].permissions);
   }
 }
 
-// ProcMapsTest.ReadProcMaps fails under TSan on Linux,
-// see http://crbug.com/258451.
-#if defined(THREAD_SANITIZER)
-#define MAYBE_ReadProcMaps DISABLED_ReadProcMaps
-#else
-#define MAYBE_ReadProcMaps ReadProcMaps
-#endif
-TEST(ProcMapsTest, MAYBE_ReadProcMaps) {
+TEST(ProcMapsTest, ReadProcMaps) {
   std::string proc_maps;
   ASSERT_TRUE(ReadProcMaps(&proc_maps));
 
   std::vector<MappedMemoryRegion> regions;
   ASSERT_TRUE(ParseProcMaps(proc_maps, &regions));
   ASSERT_FALSE(regions.empty());
 
   // We should be able to find both the current executable as well as the stack
   // mapped into memory. Use the address of |proc_maps| as a way of finding the
   // stack.
@@ skipping 30 matching lines @@
       EXPECT_FALSE(found_address) << "Found same address in multiple regions";
       found_address = true;
     }
   }
 
   EXPECT_TRUE(found_exe);
   EXPECT_TRUE(found_stack);
   EXPECT_TRUE(found_address);
 }
 
+TEST(ProcMapsTest, ReadProcMapsNonEmptyString) {
+  std::string old_string("I forgot to clear the string");
+  std::string proc_maps(old_string);
+  ASSERT_TRUE(ReadProcMaps(&proc_maps));
+  EXPECT_EQ(std::string::npos, proc_maps.find(old_string));
+}
+

[Mark Mentovai, 2013/09/05 19:25:50]

It’d be good if there was a test that ensured that ReadProcMaps was able to
properly read maps larger than a page so that you know your loop gets exercised
in testing and you can validate that things from the first [and middle] and last
reads show up in the output. I can’t come up with a great way to write that test
case, though. I guess you could mmap a few hundred files and make sure their
paths all show up in ReadProcMaps’ output.

I’m not actually asking you to write this test if you don’t want to, it’s
probably not that important and I’d like to see this code get checked in after
what you’ve been through for it.

[scherkus (not reviewing), 2013/09/05 19:54:51]

Yeah ... I'm inclined to pass as the ReadProcMaps test above should be
sufficient for now.

Debug base_unittests has a ~35KB /proc/self/maps so the loop is being covered.
Also while I was iterating on this CL, the ReadProcMaps test was catching
instances where I screwed up the string sizes 'n offsets.

 TEST(ProcMapsTest, MissingFields) {
   static const char* kTestCases[] = {
     "00400000\n",                               // Missing end + beyond.
     "00400000-0040b000\n",                      // Missing perms + beyond.
     "00400000-0040b000 r-xp\n",                 // Missing offset + beyond.
     "00400000-0040b000 r-xp 00000000\n",        // Missing device + beyond.
     "00400000-0040b000 r-xp 00000000 fc:00\n",  // Missing inode + beyond.
     "00400000-0040b000 00000000 fc:00 794418 /bin/cat\n",  // Missing perms.
     "00400000-0040b000 r-xp fc:00 794418 /bin/cat\n",      // Missing offset.
     "00400000-0040b000 r-xp 00000000 fc:00 /bin/cat\n",    // Missing inode.
@@ skipping 21 matching lines @@
 
   for (size_t i = 0; i < arraysize(kTestCases); ++i) {
     SCOPED_TRACE(base::StringPrintf("kTestCases[%zu] = %s", i, kTestCases[i]));
     std::vector<MappedMemoryRegion> regions;
     EXPECT_FALSE(ParseProcMaps(kTestCases[i], &regions));
   }
 }
 
 }  // namespace debug
 }  // namespace base