Implement /proc/self/maps/ parsing code.

base/debug/proc_maps.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.h"
+
+#if defined(OS_LINUX)
+#include <inttypes.h>
+#endif
+
+#if defined(OS_ANDROID)
+// Bionic's inttypes.h defines PRI/SCNxPTR as an unsigned long int, which
+// is incompatible with Bionic's stdint.h defining uintptr_t as a unsigned int:
+// https://code.google.com/p/android/issues/detail?id=57218
+#define SCNxPTR "x"
+#endif
+
+#include "base/file_util.h"
+#include "base/strings/string_split.h"
+
+namespace base {
+namespace debug {

[Mark Mentovai, 2013/07/02 19:36:57]

No more nested namespaces in new code.

+
+#if defined(OS_LINUX) || defined(OS_ANDROID)
+
+std::string ReadProcMaps() {

[Mark Mentovai, 2013/07/02 19:36:57]

For the sake of performance, this would likely be better if it accepted a
std::string* argument.

[scherkus (not reviewing), 2013/07/02 21:43:01]

Done.

+  std::string proc_maps;
+  FilePath proc_maps_path("/proc/self/maps");
+  if (!file_util::ReadFileToString(proc_maps_path, &proc_maps))
+    proc_maps.clear();
+  return proc_maps;
+}
+
+bool ParseProcMaps(const std::string& input,
+                   std::vector<MappedMemoryRegion>* regions_out) {
+  std::vector<MappedMemoryRegion> regions;
+
+  // This isn't async safe nor terribly efficient, but it doesn't need to be at
+  // this point in time.
+  std::vector<std::string> lines;
+  SplitString(input, '\n', &lines);
+
+  for (size_t i = 0; i < lines.size(); ++i) {
+    // Ignore empty lines produced by splitting on '\n'.

[Mark Mentovai, 2013/07/02 19:36:57]

This should only be the last line, right? Since you wrote this parser pretty
strictly, you might want to enforce that too.

[scherkus (not reviewing), 2013/07/02 21:43:01]

Gah! I had that code written but ditched it, because it felt a bit silly to be
so strict ... but you're right given how strict the rest of this parsing code
is.

Anyway, it's back w/ tests.

+    if (lines[i].empty())
+      continue;
+
+    const char* line = lines[i].c_str();
+    uintptr_t start = 0;
+    uintptr_t end = 0;
+    char permissions[4] = {'\0'};

[Mark Mentovai, 2013/07/02 19:36:57]

Recommend [5] so that you always have a NUL-terminated string, in case someone
ever wants to printf("%s", permissions) or something along those lines.

[scherkus (not reviewing), 2013/07/02 21:43:01]

Done.

+    size_t offset = 0;

[Mark Mentovai, 2013/07/02 19:36:57]

The kernel calls this an unsigned long long.

[scherkus (not reviewing), 2013/07/02 21:43:01]

Done.

+    uint8 dev_major = 0;
+    uint8 dev_minor = 0;
+    int inode = 0;

[Mark Mentovai, 2013/07/02 19:36:57]

The kernel calls this a long.

You can verify the other types used. fs/proc/task_mmu.c show_map_vma (or
fs/proc/task_nommu.c nommu_vma_show). For example, start and end are longs to
the kernel, but that should always be compatible with uintptr_t for the
architectures we’re working with.

[scherkus (not reviewing), 2013/07/02 21:43:01]

Done.

Thanks for the tip!

+    int path_index = 0;
+
+    // Sample format from man 5 proc:
+    //
+    // address           perms offset  dev   inode   pathname
+    // 08048000-08056000 r-xp 00000000 03:0c 64593   /usr/sbin/gpm
+    //
+    // The final %n term captures the offset in the input string, which is used
+    // to determine the path name.
+    if (sscanf(line, "%" SCNxPTR "-%" SCNxPTR " %4c %zx %hhx:%hhx %d %n",
+               &start, &end, permissions, &offset, &dev_major, &dev_minor,
+               &inode, &path_index) < 7) {

[jar (doing other things), 2013/07/02 19:58:57]

nit: I think you have 8 fields, rather than 7.  Maybe you're guessing that if
you get 7, you'll get the 8th (which consumes no data)... but it reads nicer
with an 8 here.
 

[scherkus (not reviewing), 2013/07/02 21:43:01]

man 3 sscanf has an interesting note about %n:

"""
The C standard says: "Execution of a %n directive does not increment the
assignment count returned at the completion of execution" but the Corrigendum
seems to contradict this.  Probably it is wise not to make any assumptions on
the effect of %n conversions on the return value.
"""

I confirmed that on Linux and Android we return 7 when we successfully parse
every field. Added a note to the comment.

+      return false;
+    }
+
+    MappedMemoryRegion region;
+    region.start = start;

[Mark Mentovai, 2013/07/02 19:36:57]

What’s with the copies? If it’s because there are data width conversions, then
it’s OK, but I don’t think there are. Why not declare the MemoryMappedRegion
above the sscanf, and where possible, give sscanf pointers to the
MemiryMappedRegion’s fields?

[scherkus (not reviewing), 2013/07/02 21:43:01]

Done.

+    region.end = end;
+    region.offset = offset;
+    region.permissions = 0;
+
+    if (permissions[0] == 'r')
+      region.permissions |= MappedMemoryRegion::READ;
+    else if (permissions[0] != '-')
+      return false;
+
+    if (permissions[1] == 'w')
+      region.permissions |= MappedMemoryRegion::WRITE;
+    else if (permissions[1] != '-')
+      return false;
+
+    if (permissions[2] == 'x')
+      region.permissions |= MappedMemoryRegion::EXECUTE;
+    else if (permissions[2] != '-')
+      return false;
+
+    if (permissions[3] == 'p')
+      region.permissions |= MappedMemoryRegion::PRIVATE;
+    else if (permissions[3] != 's')  // Shared memory.

[Mark Mentovai, 2013/07/02 19:36:57]

fs/proc/task_nommu.c nommu_vma_show permits 'S' too.

[scherkus (not reviewing), 2013/07/02 21:43:01]

Done.

+      return false;
+
+    region.path = FilePath(line + path_index);

[Mark Mentovai, 2013/07/02 19:36:57]

Since it might not be a path, maybe FilePath isn’t the right choice. Since you
know what OS you’re on and what paths are, maybe you should just use a
std::string.

[scherkus (not reviewing), 2013/07/02 21:43:01]

Done.

+    regions.push_back(region);
+  }
+
+  regions_out->swap(regions);
+  return true;
+}
+
+#endif
+
+}  // namespace debug
+}  // namespace base