DavidSpickett updated this revision to Diff 296495.
DavidSpickett added a comment.

- reset flags in MemoryRegionInfo Clear()
- add Linux specific memory region flags test
- address comments from labath

(doing this before any change of approach,
to prevent any confusion)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87442/new/

https://reviews.llvm.org/D87442

Files:
  lldb/bindings/interface/SBMemoryRegionInfo.i
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/API/SBMemoryRegionInfo.h
  lldb/include/lldb/Target/MemoryRegionInfo.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/API/SBMemoryRegionInfo.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp
  lldb/source/Plugins/Process/Utility/LinuxProcMaps.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Target/MemoryRegionInfo.cpp
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
  lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===================================================================
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -374,20 +374,20 @@
 )"),
                     llvm::Succeeded());
 
-  EXPECT_THAT(
-      parser->BuildMemoryRegions(),
-      testing::Pair(testing::ElementsAre(
-                        MemoryRegionInfo({0x0, 0x10000}, no, no, no, no,
-                                         ConstString(), unknown, 0),
-                        MemoryRegionInfo({0x10000, 0x21000}, yes, yes, no, yes,
-                                         ConstString(), unknown, 0),
-                        MemoryRegionInfo({0x40000, 0x1000}, yes, no, no, yes,
-                                         ConstString(), unknown, 0),
-                        MemoryRegionInfo({0x7ffe0000, 0x1000}, yes, no, no, yes,
-                                         ConstString(), unknown, 0),
-                        MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, yes,
-                                         ConstString(), unknown, 0)),
-                    true));
+  EXPECT_THAT(parser->BuildMemoryRegions(),
+              testing::Pair(
+                  testing::ElementsAre(
+                      MemoryRegionInfo({0x0, 0x10000}, no, no, no, no,
+                                       ConstString(), unknown, 0, llvm::None),
+                      MemoryRegionInfo({0x10000, 0x21000}, yes, yes, no, yes,
+                                       ConstString(), unknown, 0, llvm::None),
+                      MemoryRegionInfo({0x40000, 0x1000}, yes, no, no, yes,
+                                       ConstString(), unknown, 0, llvm::None),
+                      MemoryRegionInfo({0x7ffe0000, 0x1000}, yes, no, no, yes,
+                                       ConstString(), unknown, 0, llvm::None),
+                      MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, yes,
+                                       ConstString(), unknown, 0, llvm::None)),
+                  true));
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoFromMemoryList) {
@@ -409,12 +409,13 @@
 
   EXPECT_THAT(
       parser->BuildMemoryRegions(),
-      testing::Pair(testing::ElementsAre(
-                        MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown,
-                                         yes, ConstString(), unknown, 0),
-                        MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown,
-                                         yes, ConstString(), unknown, 0)),
-                    false));
+      testing::Pair(
+          testing::ElementsAre(
+              MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+                               ConstString(), unknown, 0, llvm::None),
+              MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, yes,
+                               ConstString(), unknown, 0, llvm::None)),
+          false));
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoFromMemory64List) {
@@ -424,12 +425,13 @@
   // we don't have a MemoryInfoListStream.
   EXPECT_THAT(
       parser->BuildMemoryRegions(),
-      testing::Pair(testing::ElementsAre(
-                        MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown,
-                                         yes, ConstString(), unknown, 0),
-                        MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown,
-                                         yes, ConstString(), unknown, 0)),
-                    false));
+      testing::Pair(
+          testing::ElementsAre(
+              MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+                               ConstString(), unknown, 0, llvm::None),
+              MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, yes,
+                               ConstString(), unknown, 0, llvm::None)),
+          false));
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoLinuxMaps) {
@@ -453,22 +455,22 @@
   ConstString app_process("/system/bin/app_process");
   ConstString linker("/system/bin/linker");
   ConstString liblog("/system/lib/liblog.so");
-  EXPECT_THAT(
-      parser->BuildMemoryRegions(),
-      testing::Pair(testing::ElementsAre(
-                        MemoryRegionInfo({0x400d9000, 0x2000}, yes, no, yes,
-                                         yes, app_process, unknown, 0),
-                        MemoryRegionInfo({0x400db000, 0x1000}, yes, no, no, yes,
-                                         app_process, unknown, 0),
-                        MemoryRegionInfo({0x400dc000, 0x1000}, yes, yes, no,
-                                         yes, ConstString(), unknown, 0),
-                        MemoryRegionInfo({0x400ec000, 0x1000}, yes, no, no, yes,
-                                         ConstString(), unknown, 0),
-                        MemoryRegionInfo({0x400ee000, 0x1000}, yes, yes, no,
-                                         yes, linker, unknown, 0),
-                        MemoryRegionInfo({0x400fc000, 0x1000}, yes, yes, yes,
-                                         yes, liblog, unknown, 0)),
-                    true));
+  EXPECT_THAT(parser->BuildMemoryRegions(),
+              testing::Pair(
+                  testing::ElementsAre(
+                      MemoryRegionInfo({0x400d9000, 0x2000}, yes, no, yes, yes,
+                                       app_process, unknown, 0, llvm::None),
+                      MemoryRegionInfo({0x400db000, 0x1000}, yes, no, no, yes,
+                                       app_process, unknown, 0, llvm::None),
+                      MemoryRegionInfo({0x400dc000, 0x1000}, yes, yes, no, yes,
+                                       ConstString(), unknown, 0, llvm::None),
+                      MemoryRegionInfo({0x400ec000, 0x1000}, yes, no, no, yes,
+                                       ConstString(), unknown, 0, llvm::None),
+                      MemoryRegionInfo({0x400ee000, 0x1000}, yes, yes, no, yes,
+                                       linker, unknown, 0, llvm::None),
+                      MemoryRegionInfo({0x400fc000, 0x1000}, yes, yes, yes, yes,
+                                       liblog, unknown, 0, llvm::None)),
+                  true));
 }
 
 // Windows Minidump tests
Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===================================================================
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -343,6 +343,28 @@
   EXPECT_EQ(MemoryRegionInfo::eNo, region_info.GetWritable());
   EXPECT_EQ(MemoryRegionInfo::eYes, region_info.GetExecutable());
   EXPECT_EQ("/foo/bar.so", region_info.GetName().GetStringRef());
+  EXPECT_FALSE(region_info.HasFlags());
+  EXPECT_EQ("?", region_info.GetFlags());
+
+  result = std::async(std::launch::async, [&] {
+    return client.GetMemoryRegionInfo(addr, region_info);
+  });
+
+  HandlePacket(server, "qMemoryRegionInfo:a000",
+               "start:a000;size:2000;flags:;");
+  EXPECT_TRUE(result.get().Success());
+  EXPECT_TRUE(region_info.HasFlags());
+  EXPECT_EQ("", region_info.GetFlags());
+
+  result = std::async(std::launch::async, [&] {
+    return client.GetMemoryRegionInfo(addr, region_info);
+  });
+
+  HandlePacket(server, "qMemoryRegionInfo:a000",
+               "start:a000;size:2000;flags: me  mr mw  ;");
+  EXPECT_TRUE(result.get().Success());
+  EXPECT_TRUE(region_info.HasFlags());
+  EXPECT_EQ("me mr mw", region_info.GetFlags());
 }
 
 TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfoInvalidResponse) {
Index: lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
===================================================================
--- /dev/null
+++ lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
@@ -0,0 +1,255 @@
+//===-- LinuxProcMapsTest.cpp ---------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
+#include "Plugins/Process/Utility/LinuxProcMaps.h"
+#include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Utility/Status.h"
+#include <tuple>
+
+using namespace lldb_private;
+
+typedef std::tuple<const char *, MemoryRegionInfos, const char *>
+    LinuxProcMapsTestParams;
+
+// Wrapper for convenience because Range is usually begin, size
+static MemoryRegionInfo::RangeType make_range(lldb::addr_t begin,
+                                              lldb::addr_t end) {
+  MemoryRegionInfo::RangeType range(begin, 0);
+  range.SetRangeEnd(end);
+  return range;
+}
+
+class LinuxProcMapsTestFixture
+    : public ::testing::TestWithParam<LinuxProcMapsTestParams> {
+protected:
+  Status error;
+  std::string err_str;
+  MemoryRegionInfos regions;
+  LinuxMapCallback callback;
+
+  void SetUp() override {
+    callback = [this](const MemoryRegionInfo &Info, const Status &ST) {
+      if (ST.Success()) {
+        err_str.clear();
+        regions.push_back(Info);
+        return true;
+      }
+      err_str = ST.AsCString();
+      return false;
+    };
+  }
+
+  void check_regions(LinuxProcMapsTestParams params) {
+    EXPECT_THAT(std::get<1>(params), testing::ContainerEq(regions));
+    ASSERT_EQ(std::get<2>(params), err_str);
+  }
+};
+
+TEST_P(LinuxProcMapsTestFixture, ParseMapRegions) {
+  auto params = GetParam();
+  ParseLinuxMapRegions(std::get<0>(params), callback);
+  check_regions(params);
+}
+
+// Note: ConstString("") != ConstString(nullptr)
+// When a region has no name, it will have the latter in the MemoryRegionInfo
+INSTANTIATE_TEST_CASE_P(
+    ProcMapTests, LinuxProcMapsTestFixture,
+    ::testing::Values(
+        // Nothing in nothing out
+        std::make_tuple("", MemoryRegionInfos{}, ""),
+        // Various formatting error conditions
+        std::make_tuple("55a4512f7000/55a451b68000 rw-p 00000000 00:00 0",
+                        MemoryRegionInfos{},
+                        "malformed /proc/{pid}/maps entry, missing dash "
+                        "between address range"),
+        std::make_tuple("0-0 rw", MemoryRegionInfos{},
+                        "malformed /proc/{pid}/maps entry, missing some "
+                        "portion of permissions"),
+        std::make_tuple("0-0 z--p 00000000 00:00 0", MemoryRegionInfos{},
+                        "unexpected /proc/{pid}/maps read permission char"),
+        std::make_tuple("0-0 rz-p 00000000 00:00 0", MemoryRegionInfos{},
+                        "unexpected /proc/{pid}/maps write permission char"),
+        std::make_tuple("0-0 rwzp 00000000 00:00 0", MemoryRegionInfos{},
+                        "unexpected /proc/{pid}/maps exec permission char"),
+        // Stops at first parsing error
+        std::make_tuple(
+            "0-1 rw-p 00000000 00:00 0 [abc]\n"
+            "0-0 rwzp 00000000 00:00 0\n"
+            "2-3 r-xp 00000000 00:00 0 [def]\n",
+            MemoryRegionInfos{
+                MemoryRegionInfo(make_range(0, 1), MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, ConstString("[abc]"),
+                                 MemoryRegionInfo::eDontKnow, 0, llvm::None),
+            },
+            "unexpected /proc/{pid}/maps exec permission char"),
+        // Single entry
+        std::make_tuple(
+            "55a4512f7000-55a451b68000 rw-p 00000000 00:00 0    [heap]",
+            MemoryRegionInfos{
+                MemoryRegionInfo(make_range(0x55a4512f7000, 0x55a451b68000),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 ConstString("[heap]"),
+                                 MemoryRegionInfo::eDontKnow, 0, llvm::None),
+            },
+            ""),
+        // Multiple entries
+        std::make_tuple(
+            "7fc090021000-7fc094000000 ---p 00000000 00:00 0\n"
+            "ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 "
+            "[vsyscall]",
+            MemoryRegionInfos{
+                MemoryRegionInfo(make_range(0x7fc090021000, 0x7fc094000000),
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 ConstString(nullptr),
+                                 MemoryRegionInfo::eDontKnow, 0, llvm::None),
+                MemoryRegionInfo(make_range(0xffffffffff600000,
+                                            0xffffffffff601000),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 ConstString("[vsyscall]"),
+                                 MemoryRegionInfo::eDontKnow, 0, llvm::None),
+            },
+            "")), );
+
+class LinuxProcSMapsTestFixture : public LinuxProcMapsTestFixture {};
+
+INSTANTIATE_TEST_CASE_P(
+    ProcSMapTests, LinuxProcSMapsTestFixture,
+    ::testing::Values(
+        // Nothing in nothing out
+        std::make_tuple("", MemoryRegionInfos{}, ""),
+        // Uses the same parsing for first line, so same errors but referring to
+        // smaps
+        std::make_tuple("0/0 rw-p 00000000 00:00 0", MemoryRegionInfos{},
+                        "malformed /proc/{pid}/smaps entry, missing dash "
+                        "between address range"),
+        // Stop parsing at first error
+        std::make_tuple(
+            "1111-2222 rw-p 00000000 00:00 0 [foo]\n"
+            "0/0 rw-p 00000000 00:00 0",
+            MemoryRegionInfos{
+                MemoryRegionInfo(make_range(0x1111, 0x2222),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 ConstString("[foo]"),
+                                 MemoryRegionInfo::eDontKnow, 0, llvm::None),
+            },
+            "malformed /proc/{pid}/smaps entry, missing dash between address "
+            "range"),
+        // Property line without a region is an error
+        std::make_tuple("Referenced:         2188 kB\n"
+                        "1111-2222 rw-p 00000000 00:00 0    [foo]\n"
+                        "3333-4444 rw-p 00000000 00:00 0    [bar]\n",
+                        MemoryRegionInfos{},
+                        "Found a property line without a corresponding mapping "
+                        "in /proc/{pid}/smaps"),
+        // Single region parses, has no flags
+        std::make_tuple(
+            "1111-2222 rw-p 00000000 00:00 0    [foo]",
+            MemoryRegionInfos{
+                MemoryRegionInfo(make_range(0x1111, 0x2222),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 ConstString("[foo]"),
+                                 MemoryRegionInfo::eDontKnow, 0, llvm::None),
+            },
+            ""),
+        // Single region with flags, other lines ignored
+        std::make_tuple("1111-2222 rw-p 00000000 00:00 0    [foo]\n"
+                        "Referenced:         2188 kB\n"
+                        "AnonHugePages:         0 kB\n"
+                        "VmFlags: rd wr ab cd",
+                        MemoryRegionInfos{
+                            MemoryRegionInfo(
+                                make_range(0x1111, 0x2222),
+                                MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                ConstString("[foo]"),
+                                MemoryRegionInfo::eDontKnow, 0,
+                                llvm::Optional<llvm::StringRef>("rd wr ab cd")),
+                        },
+                        ""),
+        // Whitespace in flags line ignored, any number of chars per flag
+        std::make_tuple("0-0 rw-p 00000000 00:00 0\n"
+                        "VmFlags: rd      abc x yz      ",
+                        MemoryRegionInfos{
+                            MemoryRegionInfo(
+                                make_range(0, 0), MemoryRegionInfo::eYes,
+                                MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                MemoryRegionInfo::eYes, ConstString(nullptr),
+                                MemoryRegionInfo::eDontKnow, 0,
+                                llvm::Optional<llvm::StringRef>("rd abc x yz")),
+                        },
+                        ""),
+        // VmFlags line means it has flag info, but nothing is set
+        std::make_tuple(
+            "0-0 rw-p 00000000 00:00 0\n"
+            "VmFlags:         ",
+            MemoryRegionInfos{
+                MemoryRegionInfo(make_range(0, 0), MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, ConstString(nullptr),
+                                 MemoryRegionInfo::eDontKnow, 0,
+                                 llvm::Optional<llvm::StringRef>("")),
+            },
+            ""),
+        // Handle some pages not having a flags line
+        std::make_tuple(
+            "1111-2222 rw-p 00000000 00:00 0    [foo]\n"
+            "Referenced:         2188 kB\n"
+            "AnonHugePages:         0 kB\n"
+            "3333-4444 r-xp 00000000 00:00 0    [bar]\n"
+            "VmFlags: rd wr ab cd",
+            MemoryRegionInfos{
+                MemoryRegionInfo(make_range(0x1111, 0x2222),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 ConstString("[foo]"),
+                                 MemoryRegionInfo::eDontKnow, 0, llvm::None),
+                MemoryRegionInfo(
+                    make_range(0x3333, 0x4444), MemoryRegionInfo::eYes,
+                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                    MemoryRegionInfo::eYes, ConstString("[bar]"),
+                    MemoryRegionInfo::eDontKnow, 0,
+                    llvm::Optional<llvm::StringRef>("rd wr ab cd")),
+            },
+            ""),
+        // Handle no pages having a flags line (older kernels)
+        std::make_tuple(
+            "1111-2222 rw-p 00000000 00:00 0\n"
+            "Referenced:         2188 kB\n"
+            "AnonHugePages:         0 kB\n"
+            "3333-4444 r-xp 00000000 00:00 0\n"
+            "KernelPageSize:        4 kB\n"
+            "MMUPageSize:           4 kB\n",
+            MemoryRegionInfos{
+                MemoryRegionInfo(make_range(0x1111, 0x2222),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 ConstString(nullptr),
+                                 MemoryRegionInfo::eDontKnow, 0, llvm::None),
+                MemoryRegionInfo(make_range(0x3333, 0x4444),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 ConstString(nullptr),
+                                 MemoryRegionInfo::eDontKnow, 0, llvm::None),
+            },
+            "")), );
+
+TEST_P(LinuxProcSMapsTestFixture, ParseSMapRegions) {
+  auto params = GetParam();
+  ParseLinuxSMapRegions(std::get<0>(params), callback);
+  check_regions(params);
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===================================================================
--- /dev/null
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -0,0 +1,6 @@
+add_lldb_unittest(LinuxProcMapsTests
+  LinuxProcMapsTest.cpp
+
+  LINK_LIBS
+     lldbPluginProcessLinux
+  )
Index: lldb/unittests/Process/CMakeLists.txt
===================================================================
--- lldb/unittests/Process/CMakeLists.txt
+++ lldb/unittests/Process/CMakeLists.txt
@@ -4,6 +4,7 @@
   add_subdirectory(POSIX)
 endif()
 add_subdirectory(minidump)
+add_subdirectory(Utility)
 
 add_lldb_unittest(ProcessEventDataTests
   ProcessEventDataTest.cpp
Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===================================================================
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -23,7 +23,6 @@
             'main.cpp',
             '// Run here before printing memory regions')
 
-    def test(self):
         self.build()
 
         # Set breakpoint in main and run
@@ -33,6 +32,11 @@
 
         self.runCmd("run", RUN_SUCCEEDED)
 
+        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+                    substrs=['stopped',
+                             'stop reason = breakpoint'])
+
+    def test(self):
         interp = self.dbg.GetCommandInterpreter()
         result = lldb.SBCommandReturnObject()
 
@@ -62,3 +66,15 @@
         interp.HandleCommand("memory region", result)
         self.assertFalse(result.Succeeded())
         self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+
+    @skipUnlessLinux
+    @skipUnlessHasProcSmapsVmFlags()
+    def test_flags(self):
+        """Test that a kernel with smaps/VmFlags shows emmory region flags."""
+        self.expect("memory region main",
+                    msg="Expected code address to be executable and readable!",
+                    patterns=[
+                        "\[0x[0-9A-Fa-f]+-0x[0-9A-Fa-f]+\)",
+                        # Accept either order of flags
+                        "flags:.*(ex|rd).*(ex|rd).*"
+                    ])
Index: lldb/source/Target/MemoryRegionInfo.cpp
===================================================================
--- lldb/source/Target/MemoryRegionInfo.cpp
+++ lldb/source/Target/MemoryRegionInfo.cpp
@@ -10,15 +10,37 @@
 
 using namespace lldb_private;
 
+void MemoryRegionInfo::SetFlags(llvm::StringRef flags) {
+  // Assuming a line of the format:
+  // <flag><spaces><flag><spaces>...
+  // E.g. "rd wr ex"
+  m_flags = std::set<std::string>();
+  llvm::StringRef flag;
+  while (flags.size()) {
+    flags = flags.ltrim();
+    std::tie(flag, flags) = flags.split(' ');
+    // Account for trailing whitespace
+    if (flag.size()) {
+      m_flags->insert(flag.str());
+    }
+  }
+}
+
+std::string MemoryRegionInfo::GetFlags() const {
+  if (HasFlags())
+    return llvm::join(m_flags.getValue(), " ");
+  return "?";
+}
+
 llvm::raw_ostream &lldb_private::operator<<(llvm::raw_ostream &OS,
                                             const MemoryRegionInfo &Info) {
   return OS << llvm::formatv("MemoryRegionInfo([{0}, {1}), {2:r}{3:w}{4:x}, "
-                             "{5}, `{6}`, {7}, {8})",
+                             "{5}, `{6}`, {7}, {8}, {9})",
                              Info.GetRange().GetRangeBase(),
                              Info.GetRange().GetRangeEnd(), Info.GetReadable(),
                              Info.GetWritable(), Info.GetExecutable(),
                              Info.GetMapped(), Info.GetName(), Info.GetFlash(),
-                             Info.GetBlocksize());
+                             Info.GetBlocksize(), Info.GetFlags());
 }
 
 void llvm::format_provider<MemoryRegionInfo::OptionalBool>::format(
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2491,6 +2491,14 @@
       response.PutChar(';');
     }
 
+    // Flags
+    if (region_info.HasFlags()) {
+      std::string flags_str = region_info.GetFlags();
+      response.PutCString("flags:");
+      response.PutCString(flags_str);
+      response.PutChar(';');
+    }
+
     // Name
     ConstString name = region_info.GetName();
     if (name) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1529,6 +1529,8 @@
           std::string name;
           name_extractor.GetHexByteString(name);
           region_info.SetName(name.c_str());
+        } else if (name.equals("flags")) {
+          region_info.SetFlags(value);
         } else if (name.equals("error")) {
           StringExtractorGDBRemote error_extractor(value);
           std::string error_string;
Index: lldb/source/Plugins/Process/Utility/LinuxProcMaps.h
===================================================================
--- lldb/source/Plugins/Process/Utility/LinuxProcMaps.h
+++ lldb/source/Plugins/Process/Utility/LinuxProcMaps.h
@@ -21,6 +21,8 @@
 
 void ParseLinuxMapRegions(llvm::StringRef linux_map,
                           LinuxMapCallback const &callback);
+void ParseLinuxSMapRegions(llvm::StringRef linux_smap,
+                           LinuxMapCallback const &callback);
 
 } // namespace lldb_private
 
Index: lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp
===================================================================
--- lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp
+++ lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp
@@ -7,16 +7,24 @@
 //===----------------------------------------------------------------------===//
 
 #include "LinuxProcMaps.h"
-#include "llvm/ADT/StringRef.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StringExtractor.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 
 using namespace lldb_private;
 
+enum class MapsKind { Maps, SMaps };
+
+static Status ProcMapError(const char *msg, MapsKind kind) {
+  return Status(msg, kind == MapsKind::Maps ? "maps" : "smaps");
+}
+
 static Status
 ParseMemoryRegionInfoFromProcMapsLine(llvm::StringRef maps_line,
-                                      MemoryRegionInfo &memory_region_info) {
+                                      MemoryRegionInfo &memory_region_info,
+                                      MapsKind maps_kind) {
   memory_region_info.Clear();
   
   StringExtractor line_extractor(maps_line);
@@ -30,30 +38,33 @@
   
   // Parse out hyphen separating start and end address from range.
   if (!line_extractor.GetBytesLeft() || (line_extractor.GetChar() != '-'))
-    return Status(
-        "malformed /proc/{pid}/maps entry, missing dash between address range");
-  
+    return ProcMapError(
+        "malformed /proc/{pid}/%s entry, missing dash between address range",
+        maps_kind);
+
   // Parse out the ending address
   lldb::addr_t end_address = line_extractor.GetHexMaxU64(false, start_address);
   
   // Parse out the space after the address.
   if (!line_extractor.GetBytesLeft() || (line_extractor.GetChar() != ' '))
-    return Status(
-        "malformed /proc/{pid}/maps entry, missing space after range");
-  
+    return ProcMapError(
+        "malformed /proc/{pid}/%s entry, missing space after range", maps_kind);
+
   // Save the range.
   memory_region_info.GetRange().SetRangeBase(start_address);
   memory_region_info.GetRange().SetRangeEnd(end_address);
-  
-  // Any memory region in /proc/{pid}/maps is by definition mapped into the
-  // process.
+
+  // Any memory region in /proc/{pid}/(maps|smaps) is by definition mapped
+  // into the process.
   memory_region_info.SetMapped(MemoryRegionInfo::OptionalBool::eYes);
   
   // Parse out each permission entry.
   if (line_extractor.GetBytesLeft() < 4)
-    return Status("malformed /proc/{pid}/maps entry, missing some portion of "
-                  "permissions");
-  
+    return ProcMapError(
+        "malformed /proc/{pid}/%s entry, missing some portion of "
+        "permissions",
+        maps_kind);
+
   // Handle read permission.
   const char read_perm_char = line_extractor.GetChar();
   if (read_perm_char == 'r')
@@ -61,8 +72,9 @@
   else if (read_perm_char == '-')
     memory_region_info.SetReadable(MemoryRegionInfo::OptionalBool::eNo);
   else
-    return Status("unexpected /proc/{pid}/maps read permission char");
-  
+    return ProcMapError("unexpected /proc/{pid}/%s read permission char",
+                        maps_kind);
+
   // Handle write permission.
   const char write_perm_char = line_extractor.GetChar();
   if (write_perm_char == 'w')
@@ -70,8 +82,9 @@
   else if (write_perm_char == '-')
     memory_region_info.SetWritable(MemoryRegionInfo::OptionalBool::eNo);
   else
-    return Status("unexpected /proc/{pid}/maps write permission char");
-  
+    return ProcMapError("unexpected /proc/{pid}/%s write permission char",
+                        maps_kind);
+
   // Handle execute permission.
   const char exec_perm_char = line_extractor.GetChar();
   if (exec_perm_char == 'x')
@@ -79,8 +92,9 @@
   else if (exec_perm_char == '-')
     memory_region_info.SetExecutable(MemoryRegionInfo::OptionalBool::eNo);
   else
-    return Status("unexpected /proc/{pid}/maps exec permission char");
-  
+    return ProcMapError("unexpected /proc/{pid}/%s exec permission char",
+                        maps_kind);
+
   line_extractor.GetChar();              // Read the private bit
   line_extractor.SkipSpaces();           // Skip the separator
   line_extractor.GetHexMaxU64(false, 0); // Read the offset
@@ -105,8 +119,83 @@
   while (!lines.empty()) {
     std::tie(line, lines) = lines.split('\n');
     MemoryRegionInfo region;
-    Status error = ParseMemoryRegionInfoFromProcMapsLine(line, region);
+    Status error =
+        ParseMemoryRegionInfoFromProcMapsLine(line, region, MapsKind::Maps);
     if (!callback(region, error))
       break;
   }
 }
+
+void lldb_private::ParseLinuxSMapRegions(llvm::StringRef linux_smap,
+                                         LinuxMapCallback const &callback) {
+  // Entries in /smaps look like:
+  // 00400000-0048a000 r-xp 00000000 fd:03 960637
+  // Size:                552 kB
+  // Rss:                 460 kB
+  // <...>
+  // VmFlags: rd ex mr mw me dw
+  // 00500000-0058a000 rwxp 00000000 fd:03 960637
+  // <...>
+  //
+  // Where the first line is identical to the /maps format
+  // And VmFlags is only printed for kernels >= 3.8
+
+  llvm::StringRef lines(linux_smap);
+  llvm::StringRef line;
+  llvm::Optional<MemoryRegionInfo> region;
+  Status error;
+
+  while (!lines.empty()) {
+    std::tie(line, lines) = lines.split('\n');
+
+    // A property line looks like:
+    // <word>: <value>
+    // (no spaces on the left hand side)
+    // A header will have a ':' but the LHS will contain spaces
+    llvm::StringRef name;
+    llvm::StringRef value;
+    std::tie(name, value) = line.split(':');
+
+    // If this line is a property line
+    if (name.find(' ') == llvm::StringRef::npos) {
+      if (region) {
+        if (name == "VmFlags") {
+          // An empty value is ok, although unlikely to happen
+          region->SetFlags(value);
+        }
+        // Ignore anything else
+      } else {
+        // Orphaned settings line
+        error = Status("Found a property line without a corresponding mapping "
+                       "in /proc/{pid}/smaps");
+        region = MemoryRegionInfo();
+        callback(*region, error);
+        return;
+      }
+    } else {
+      // Must be a new region header
+      if (region) {
+        // Save current region (error will always be Success here)
+        callback(*region, error);
+        region.reset();
+      }
+
+      // Try to start a new region
+      MemoryRegionInfo new_region;
+      error = ParseMemoryRegionInfoFromProcMapsLine(line, new_region,
+                                                    MapsKind::SMaps);
+      if (error.Fail()) {
+        // Stop at first invalid region header
+        callback(*region, error);
+        return;
+      } else {
+        region = new_region;
+      }
+    }
+  }
+
+  // Catch last region (error will always be Success here)
+  if (region) {
+    callback(*region, error);
+  }
+}
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1297,28 +1297,42 @@
     return Status();
   }
 
-  auto BufferOrError = getProcFile(GetID(), "maps");
-  if (!BufferOrError) {
-    m_supports_mem_region = LazyBool::eLazyBoolNo;
-    return BufferOrError.getError();
-  }
   Status Result;
-  ParseLinuxMapRegions(BufferOrError.get()->getBuffer(),
-                       [&](const MemoryRegionInfo &Info, const Status &ST) {
-                         if (ST.Success()) {
-                           FileSpec file_spec(Info.GetName().GetCString());
-                           FileSystem::Instance().Resolve(file_spec);
-                           m_mem_region_cache.emplace_back(Info, file_spec);
-                           return true;
-                         } else {
-                           m_supports_mem_region = LazyBool::eLazyBoolNo;
-                           LLDB_LOG(log, "failed to parse proc maps: {0}", ST);
-                           Result = ST;
-                           return false;
-                         }
-                       });
-  if (Result.Fail())
-    return Result;
+  LinuxMapCallback callback = [&](const MemoryRegionInfo &Info,
+                                  const Status &ST) {
+    if (ST.Success()) {
+      FileSpec file_spec(Info.GetName().GetCString());
+      FileSystem::Instance().Resolve(file_spec);
+      m_mem_region_cache.emplace_back(Info, file_spec);
+      return true;
+    } else {
+      m_supports_mem_region = LazyBool::eLazyBoolNo;
+      LLDB_LOG(log, "failed to parse proc maps: {0}", ST);
+      Result = ST;
+      return false;
+    }
+  };
+
+  // Linux kernel since 2.6.14 has /proc/{pid}/smaps
+  // if CONFIG_PROC_PAGE_MONITOR is enabled
+  auto BufferOrError = getProcFile(GetID(), "smaps");
+  if (BufferOrError) {
+    ParseLinuxSMapRegions(BufferOrError.get()->getBuffer(), callback);
+    if (Result.Fail()) {
+      return Result;
+    }
+  } else {
+    BufferOrError = getProcFile(GetID(), "maps");
+    if (!BufferOrError) {
+      m_supports_mem_region = LazyBool::eLazyBoolNo;
+      return BufferOrError.getError();
+    }
+
+    ParseLinuxMapRegions(BufferOrError.get()->getBuffer(), callback);
+    if (Result.Fail()) {
+      return Result;
+    }
+  }
 
   if (m_mem_region_cache.empty()) {
     // No entries after attempting to read them.  This shouldn't happen if
Index: lldb/source/Commands/CommandObjectMemory.cpp
===================================================================
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -1733,12 +1733,18 @@
           section_name = section_sp->GetName();
         }
       }
+
       result.AppendMessageWithFormatv(
-          "[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}\n",
+          "[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}",
           range_info.GetRange().GetRangeBase(),
           range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
           range_info.GetWritable(), range_info.GetExecutable(), name ? " " : "",
           name, section_name ? " " : "", section_name);
+      if (range_info.HasFlags()) {
+        result.AppendMessageWithFormatv("flags: {0}",
+                                        range_info.GetFlags());
+      }
+
       m_prev_end_addr = range_info.GetRange().GetRangeEnd();
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return true;
Index: lldb/source/API/SBMemoryRegionInfo.cpp
===================================================================
--- lldb/source/API/SBMemoryRegionInfo.cpp
+++ lldb/source/API/SBMemoryRegionInfo.cpp
@@ -116,6 +116,19 @@
   return m_opaque_up->GetName().AsCString();
 }
 
+bool SBMemoryRegionInfo::GetFlags(SBStream &flags) {
+  LLDB_RECORD_METHOD(bool, SBMemoryRegionInfo, GetFlags, (lldb::SBStream &),
+                     flags);
+
+  if (!m_opaque_up->HasFlags())
+    return false;
+
+  Stream &strm = flags.ref();
+  strm << m_opaque_up->GetFlags();
+
+  return true;
+}
+
 bool SBMemoryRegionInfo::GetDescription(SBStream &description) {
   LLDB_RECORD_METHOD(bool, SBMemoryRegionInfo, GetDescription,
                      (lldb::SBStream &), description);
@@ -128,6 +141,8 @@
   strm.Printf(m_opaque_up->GetReadable() ? "R" : "-");
   strm.Printf(m_opaque_up->GetWritable() ? "W" : "-");
   strm.Printf(m_opaque_up->GetExecutable() ? "X" : "-");
+  if (m_opaque_up->HasFlags())
+    strm.Printf(" flags: %s", m_opaque_up->GetFlags().c_str());
   strm.Printf("]");
 
   return true;
@@ -158,6 +173,7 @@
   LLDB_REGISTER_METHOD(bool, SBMemoryRegionInfo, IsExecutable, ());
   LLDB_REGISTER_METHOD(bool, SBMemoryRegionInfo, IsMapped, ());
   LLDB_REGISTER_METHOD(const char *, SBMemoryRegionInfo, GetName, ());
+  LLDB_REGISTER_METHOD(bool, SBMemoryRegionInfo, GetFlags, (lldb::SBStream &));
   LLDB_REGISTER_METHOD(bool, SBMemoryRegionInfo, GetDescription,
                        (lldb::SBStream &));
 }
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -806,8 +806,9 @@
                     "start",
                     "size",
                     "permissions",
+                    "flags",
                     "name",
-                    "error"])
+                    "error"], "Unexpected key \"%s\"" % key)
             self.assertIsNotNone(val)
 
         mem_region_dict["name"] = seven.unhexlify(mem_region_dict.get("name", ""))
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -587,6 +587,10 @@
     """Decorate the item to skip tests that should be skipped on Linux."""
     return skipIfPlatform(["linux"])(func)
 
+def skipUnlessLinux(func):
+    """Decorate the item to skip tests that should be skipped on any non-Linux platform."""
+    return skipUnlessPlatform(["linux"])(func)
+
 
 def skipIfWindows(func):
     """Decorate the item to skip tests that should be skipped on Windows."""
@@ -864,3 +868,21 @@
     return unittest2.skipIf(
         configuration.capture_path or configuration.replay_path,
         "reproducers unsupported")(func)
+
+def skipUnlessHasProcSmapsVmFlags():
+    # smaps is available on kernels >= 2.6.14
+    # VmFlags is present if CONFIG_PROC_PAGE_MONITOR is enabled
+    # We're assuming that if this Python process has /smaps/ then
+    # any debug target we launch would have it too.
+    smaps_path = os.path.join(os.sep, "proc", str(os.getpid()), "smaps")
+    has_vmflags = False
+
+    try:
+        with open(smaps_path, 'r') as f:
+            if "VmFlags" in f.read():
+                has_vmflags = True
+    # Would use FileNotFoundError but that is py3 only
+    except IOError:
+        pass
+
+    return unittest2.skipUnless(has_vmflags, "requires /proc/{pid}/smaps with VmFlags")
Index: lldb/include/lldb/Target/MemoryRegionInfo.h
===================================================================
--- lldb/include/lldb/Target/MemoryRegionInfo.h
+++ lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -12,7 +12,9 @@
 
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/RangeMap.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatProviders.h"
+#include <set>
 
 namespace lldb_private {
 class MemoryRegionInfo {
@@ -24,15 +26,20 @@
   MemoryRegionInfo() = default;
   MemoryRegionInfo(RangeType range, OptionalBool read, OptionalBool write,
                    OptionalBool execute, OptionalBool mapped, ConstString name,
-                   OptionalBool flash, lldb::offset_t blocksize)
+                   OptionalBool flash, lldb::offset_t blocksize,
+                   llvm::Optional<llvm::StringRef> flags)
       : m_range(range), m_read(read), m_write(write), m_execute(execute),
         m_mapped(mapped), m_name(name), m_flash(flash), m_blocksize(blocksize) {
+    if (flags) {
+      SetFlags(*flags);
+    }
   }
 
   RangeType &GetRange() { return m_range; }
 
   void Clear() {
     m_range.Clear();
+    m_flags.reset();
     m_read = m_write = m_execute = eDontKnow;
   }
 
@@ -48,6 +55,11 @@
 
   ConstString GetName() const { return m_name; }
 
+  bool HasFlags() const { return m_flags.hasValue(); }
+
+  // Get space joined string of flags
+  std::string GetFlags() const;
+
   void SetReadable(OptionalBool val) { m_read = val; }
 
   void SetWritable(OptionalBool val) { m_write = val; }
@@ -66,6 +78,9 @@
 
   void SetBlocksize(lldb::offset_t blocksize) { m_blocksize = blocksize; }
 
+  // Init from space seperated list of flags
+  void SetFlags(llvm::StringRef flags);
+
   // Get permissions as a uint32_t that is a mask of one or more bits from the
   // lldb::Permissions
   uint32_t GetLLDBPermissions() const {
@@ -91,7 +106,8 @@
     return m_range == rhs.m_range && m_read == rhs.m_read &&
            m_write == rhs.m_write && m_execute == rhs.m_execute &&
            m_mapped == rhs.m_mapped && m_name == rhs.m_name &&
-           m_flash == rhs.m_flash && m_blocksize == rhs.m_blocksize;
+           m_flash == rhs.m_flash && m_blocksize == rhs.m_blocksize &&
+           m_flags == rhs.m_flags;
   }
 
   bool operator!=(const MemoryRegionInfo &rhs) const { return !(*this == rhs); }
@@ -105,6 +121,7 @@
   ConstString m_name;
   OptionalBool m_flash = eDontKnow;
   lldb::offset_t m_blocksize = 0;
+  llvm::Optional<std::set<std::string>> m_flags;
 };
   
 inline bool operator<(const MemoryRegionInfo &lhs,
Index: lldb/include/lldb/API/SBMemoryRegionInfo.h
===================================================================
--- lldb/include/lldb/API/SBMemoryRegionInfo.h
+++ lldb/include/lldb/API/SBMemoryRegionInfo.h
@@ -73,6 +73,16 @@
   ///     region. If no name can be determined the returns nullptr.
   const char *GetName();
 
+  /// If the region has flag information, get its flags as a space
+  /// seperated string.
+  ///
+  /// \param[out] flags
+  ///    Stream to write the flags string to.
+  ///
+  /// \return
+  ///    True if flags were written to \p flags, False otherwise.
+  bool GetFlags(lldb::SBStream &flags);
+
   bool operator==(const lldb::SBMemoryRegionInfo &rhs) const;
 
   bool operator!=(const lldb::SBMemoryRegionInfo &rhs) const;
Index: lldb/docs/lldb-gdb-remote.txt
===================================================================
--- lldb/docs/lldb-gdb-remote.txt
+++ lldb/docs/lldb-gdb-remote.txt
@@ -1086,6 +1086,10 @@
                  // the file while for anonymous regions it have to be the name
                  // associated to the region if that is available.
 
+    flags:<flags-string>; // where <flags-string> is a space seperated string
+                          // of flag names. These names are platform specific
+                          // e.g. on Linux it might be "rd ex mr".
+
     error:<ascii-byte-error-string>; // where <ascii-byte-error-string> is
                                      // a hex encoded string value that
                                      // contains an error string
Index: lldb/bindings/interface/SBMemoryRegionInfo.i
===================================================================
--- lldb/bindings/interface/SBMemoryRegionInfo.i
+++ lldb/bindings/interface/SBMemoryRegionInfo.i
@@ -47,6 +47,9 @@
     GetName ();
 
     bool
+    GetFlags (lldb::SBStream &flags);
+
+    bool
     operator == (const lldb::SBMemoryRegionInfo &rhs) const;
 
     bool
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to