labath created this revision.
labath added reviewers: clayborg, jingham.
labath added a parent revision: D69105: minidump: Create memory regions from 
the sections of loaded modules.

The permissions in a memory region have ternary states (yes, no, don't
know), but the memory region command only prints in binary, treating
"don't know" as "yes", which is particularly confusing as for instance
the unwinder will treat an unknown value as "no".

This patch makes is so that we distinguish all three states when
printing the values, using "?" to indicate the lack of information. It
is implemented via a special argument to the format provider for the
OptionalBool enumeration.


https://reviews.llvm.org/D69106

Files:
  include/lldb/Target/MemoryRegionInfo.h
  source/Commands/CommandObjectMemory.cpp
  source/Target/MemoryRegionInfo.cpp
  test/Shell/Minidump/memory-region-from-module.yaml

Index: test/Shell/Minidump/memory-region-from-module.yaml
===================================================================
--- test/Shell/Minidump/memory-region-from-module.yaml
+++ test/Shell/Minidump/memory-region-from-module.yaml
@@ -19,8 +19,7 @@
 # ALL: [0x0000000000000000-0x0000000000004000) ---
 # ALL-LABEL: (lldb) memory region 0x4000
 # CHECK1: [0x0000000000004000-0x00000000000040b0) r-x {{.*}}memory-region-from-module.exe PT_LOAD[0]
-# TODO: This output does not give any indication that the region is only "potentially" writable.
-# CHECK2: [0x0000000000004000-0x0000000000004010) rwx
+# CHECK2: [0x0000000000004000-0x0000000000004010) r??
 # ALL-LABEL: (lldb) memory region 0x5000
 # ALL: [0x0000000000005000-0x000000000000505c) rw- {{.*}}memory-region-from-module.exe PT_LOAD[1]
 # ALL-LABEL: (lldb) memory region 0x6000
Index: source/Target/MemoryRegionInfo.cpp
===================================================================
--- source/Target/MemoryRegionInfo.cpp
+++ source/Target/MemoryRegionInfo.cpp
@@ -8,13 +8,33 @@
 
 #include "lldb/Target/MemoryRegionInfo.h"
 
+using namespace lldb_private;
+
 llvm::raw_ostream &lldb_private::operator<<(llvm::raw_ostream &OS,
                                             const MemoryRegionInfo &Info) {
-  return OS << llvm::formatv("MemoryRegionInfo([{0}, {1}), {2}, {3}, {4}, {5}, "
-                             "`{6}`, {7}, {8})",
+  return OS << llvm::formatv("MemoryRegionInfo([{0}, {1}), {2:r}{3:w}{4:x}, "
+                             "{5}, `{6}`, {7}, {8})",
                              Info.GetRange().GetRangeBase(),
                              Info.GetRange().GetRangeEnd(), Info.GetReadable(),
                              Info.GetWritable(), Info.GetExecutable(),
                              Info.GetMapped(), Info.GetName(), Info.GetFlash(),
                              Info.GetBlocksize());
 }
+
+void llvm::format_provider<MemoryRegionInfo::OptionalBool>::format(
+    const MemoryRegionInfo::OptionalBool &B, raw_ostream &OS,
+    StringRef Options) {
+  assert(Options.size() <= 1);
+  bool Empty = Options.empty();
+  switch (B) {
+  case lldb_private::MemoryRegionInfo::eNo:
+    OS << (Empty ? "no" : "-");
+    return;
+  case lldb_private::MemoryRegionInfo::eYes:
+    OS << (Empty ? "yes" : Options);
+    return;
+  case lldb_private::MemoryRegionInfo::eDontKnow:
+    OS << (Empty ? "don't know" : "?");
+    return;
+  }
+}
Index: source/Commands/CommandObjectMemory.cpp
===================================================================
--- source/Commands/CommandObjectMemory.cpp
+++ source/Commands/CommandObjectMemory.cpp
@@ -1730,15 +1730,12 @@
               section_name = section_sp->GetName();
             }
           }
-          result.AppendMessageWithFormat(
-              "[0x%16.16" PRIx64 "-0x%16.16" PRIx64 ") %c%c%c%s%s%s%s\n",
+          result.AppendMessageWithFormatv(
+              "[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}\n",
               range_info.GetRange().GetRangeBase(),
-              range_info.GetRange().GetRangeEnd(),
-              range_info.GetReadable() ? 'r' : '-',
-              range_info.GetWritable() ? 'w' : '-',
-              range_info.GetExecutable() ? 'x' : '-',
-              name ? " " : "", name.AsCString(""),
-              section_name ? " " : "", section_name.AsCString(""));
+              range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
+              range_info.GetWritable(), range_info.GetExecutable(),
+              name ? " " : "", name, section_name ? " " : "", section_name);
           m_prev_end_addr = range_info.GetRange().GetRangeEnd();
           result.SetStatus(eReturnStatusSuccessFinishResult);
         } else {
Index: include/lldb/Target/MemoryRegionInfo.h
===================================================================
--- include/lldb/Target/MemoryRegionInfo.h
+++ include/lldb/Target/MemoryRegionInfo.h
@@ -133,21 +133,13 @@
 
 namespace llvm {
 template <>
+/// If Options is empty, prints a textual representation of the value. If
+/// Options is a single character, it uses that character for the "yes" value,
+/// while "no" is printed as "-", and "don't know" as "?". This can be used to
+/// print the permissions in the traditional "rwx" form.
 struct format_provider<lldb_private::MemoryRegionInfo::OptionalBool> {
   static void format(const lldb_private::MemoryRegionInfo::OptionalBool &B,
-                     raw_ostream &OS, StringRef Options) {
-    switch(B) {
-    case lldb_private::MemoryRegionInfo::eNo:
-      OS << "no";
-      return;
-    case lldb_private::MemoryRegionInfo::eYes:
-      OS << "yes";
-      return;
-    case lldb_private::MemoryRegionInfo::eDontKnow:
-      OS << "don't know";
-      return;
-    }
-  }
+                     raw_ostream &OS, StringRef Options);
 };
 }
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to