Skip to content

[lldb] Reduce the number of arguments to MemoryRegionInfo constructor - #188755

Closed
DavidSpickett wants to merge 4 commits into
llvm:mainfrom
DavidSpickett:lldb-memory-region-info
Closed

[lldb] Reduce the number of arguments to MemoryRegionInfo constructor#188755
DavidSpickett wants to merge 4 commits into
llvm:mainfrom
DavidSpickett:lldb-memory-region-info

Conversation

@DavidSpickett

@DavidSpickett DavidSpickett commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

This was requested on #182246 where I am adding yet another memory region property.

This PR contains several individual commits that I intend to push separately if they pass review. I have uploaded them as one PR so reviewers can be confident with the direction I'm going in.

I suggest you look at the complete changes to see the end goal, and the individual commits for anything more detailed.

The two types of change are:

  • Removing arguments that were only ever set to default values.
  • Anything that was sometimes set to a non-default value, I have changed the setter into a builder function.

That is, a method that returns a reference to self.

I am doing this because it was pointed out to me that MemoryRegionInfo
has lots of construction paramaters, and most of the time, we want the
default value for most of the things.

So now we can do:
MemoryRegionInfo(...); // Shadow stack is "don't know".
MemoryRegionInfo(...).SetIsShadowStack(eNo) // Shadow stack is "no".

Which removes one parameter from every use of the constructor.

Along the way I realised that the shadow stack "ss" flag is only
tested by the Guarded Control Stack tests, which only run on
specific Arm hardware. I've added a new "ss" test to LinuxProcMapsTest,
which will run on any system.
Nothing passes anything but the default value here. Code that
wants to set it is starting with a default constructed info,
and filling in the fields one by one (the gdb-remote client
for example).
By turning SetMemoryTagged into a builder method (returns a
reference to self). Then only using that in the tests that
need to change the default of "don't know".
These are only set to non-default values after calling a constructor.
Removing them removes noise from many tests that make MemoryRegionInfos.
@llvmbot

llvmbot commented Mar 26, 2026

Copy link
Copy Markdown
Member

@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)

Changes

This was requested on #182246 where I am adding yet another memory region property.

This PR contains several individual commits that I intend to push separately if they pass review. I have uploaded them as one PR so reviewers can be confident with the direction I'm going in.

I suggest you look at the complete changes to see the end goal, and the individual commits for anything more detailed.

The two types of change are:

  • Removing arguments that were only ever set to default values.
  • Anything that was sometimes set to a non-default value, I have changed the setter into a builder function.

Patch is 25.46 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/188755.diff

4 Files Affected:

  • (modified) lldb/include/lldb/Target/MemoryRegionInfo.h (+10-8)
  • (modified) lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp (+84-108)
  • (modified) lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp (+5-7)
  • (modified) lldb/unittests/Process/minidump/MinidumpParserTest.cpp (+25-41)
diff --git a/lldb/include/lldb/Target/MemoryRegionInfo.h b/lldb/include/lldb/Target/MemoryRegionInfo.h
index dc37a7dbeda52..527461dd91e91 100644
--- a/lldb/include/lldb/Target/MemoryRegionInfo.h
+++ b/lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -27,13 +27,9 @@ class MemoryRegionInfo {
   MemoryRegionInfo() = default;
   MemoryRegionInfo(RangeType range, OptionalBool read, OptionalBool write,
                    OptionalBool execute, OptionalBool shared,
-                   OptionalBool mapped, ConstString name, OptionalBool flash,
-                   lldb::offset_t blocksize, OptionalBool memory_tagged,
-                   OptionalBool stack_memory, OptionalBool shadow_stack)
+                   OptionalBool mapped, ConstString name)
       : m_range(range), m_read(read), m_write(write), m_execute(execute),
-        m_shared(shared), m_mapped(mapped), m_name(name), m_flash(flash),
-        m_blocksize(blocksize), m_memory_tagged(memory_tagged),
-        m_is_stack_memory(stack_memory), m_is_shadow_stack(shadow_stack) {}
+        m_shared(shared), m_mapped(mapped), m_name(name) {}
 
   RangeType &GetRange() { return m_range; }
 
@@ -77,9 +73,15 @@ class MemoryRegionInfo {
 
   void SetBlocksize(lldb::offset_t blocksize) { m_blocksize = blocksize; }
 
-  void SetMemoryTagged(OptionalBool val) { m_memory_tagged = val; }
+  MemoryRegionInfo &SetMemoryTagged(OptionalBool val) {
+    m_memory_tagged = val;
+    return *this;
+  }
 
-  void SetIsShadowStack(OptionalBool val) { m_is_shadow_stack = val; }
+  MemoryRegionInfo &SetIsShadowStack(OptionalBool val) {
+    m_is_shadow_stack = val;
+    return *this;
+  }
 
   // Get permissions as a uint32_t that is a mask of one or more bits from the
   // lldb::Permissions
diff --git a/lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp b/lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
index d94bb4f4db982..9c99117f1fe7c 100644
--- a/lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
+++ b/lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
@@ -87,26 +87,20 @@ INSTANTIATE_TEST_SUITE_P(
             "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::eNo, MemoryRegionInfo::eYes,
-                    ConstString("[abc]"), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eDontKnow),
+                MemoryRegionInfo(make_range(0, 1), MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 ConstString("[abc]")),
             },
             "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::eNo,
-                    MemoryRegionInfo::eYes, ConstString("[heap]"),
-                    MemoryRegionInfo::eDontKnow, 0, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow),
+                MemoryRegionInfo(make_range(0x55a4512f7000, 0x55a451b68000),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, ConstString("[heap]")),
             },
             ""),
         // Multiple entries
@@ -116,27 +110,19 @@ INSTANTIATE_TEST_SUITE_P(
             "ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 "
             "[vsyscall]",
             MemoryRegionInfos{
-                MemoryRegionInfo(
-                    make_range(0x7fc090021000, 0x7fc094000000),
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
-                    MemoryRegionInfo::eYes, ConstString(nullptr),
-                    MemoryRegionInfo::eDontKnow, 0, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow),
-                MemoryRegionInfo(
-                    make_range(0x7fc094000000, 0x7fc094a00000),
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eYes, ConstString(nullptr),
-                    MemoryRegionInfo::eDontKnow, 0, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow),
+                MemoryRegionInfo(make_range(0x7fc090021000, 0x7fc094000000),
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, ConstString(nullptr)),
+                MemoryRegionInfo(make_range(0x7fc094000000, 0x7fc094a00000),
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eYes, ConstString(nullptr)),
                 MemoryRegionInfo(
                     make_range(0xffffffffff600000, 0xffffffffff601000),
                     MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
                     MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
-                    MemoryRegionInfo::eYes, ConstString("[vsyscall]"),
-                    MemoryRegionInfo::eDontKnow, 0, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow),
+                    MemoryRegionInfo::eYes, ConstString("[vsyscall]")),
             },
             "")));
 
@@ -157,13 +143,10 @@ INSTANTIATE_TEST_SUITE_P(
             "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::eNo, MemoryRegionInfo::eYes,
-                    ConstString("[foo]"), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eDontKnow),
+                MemoryRegionInfo(make_range(0x1111, 0x2222),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, ConstString("[foo]")),
             },
             "malformed /proc/{pid}/smaps entry, missing dash between address "
             "range"),
@@ -178,26 +161,20 @@ INSTANTIATE_TEST_SUITE_P(
         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::eNo, MemoryRegionInfo::eYes,
-                    ConstString("[foo]"), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eDontKnow),
+                MemoryRegionInfo(make_range(0x1111, 0x2222),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, ConstString("[foo]")),
             },
             ""),
         // Single shared region parses, has no flags
         std::make_tuple(
             "1111-2222 rw-s 00000000 00:00 0    [foo]",
             MemoryRegionInfos{
-                MemoryRegionInfo(
-                    make_range(0x1111, 0x2222), MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
-                    MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
-                    ConstString("[foo]"), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eDontKnow),
+                MemoryRegionInfo(make_range(0x1111, 0x2222),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eYes, ConstString("[foo]")),
             },
             ""),
         // Single region with flags, other lines ignored
@@ -207,13 +184,12 @@ INSTANTIATE_TEST_SUITE_P(
             "AnonHugePages:         0 kB\n"
             "VmFlags: mt",
             MemoryRegionInfos{
-                MemoryRegionInfo(
-                    make_range(0x1111, 0x2222), MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
-                    ConstString("[foo]"), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eYes, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eNo),
+                MemoryRegionInfo(make_range(0x1111, 0x2222),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, ConstString("[foo]"))
+                    .SetIsShadowStack(MemoryRegionInfo::eNo)
+                    .SetMemoryTagged(MemoryRegionInfo::eYes),
             },
             ""),
         // Whitespace ignored
@@ -221,13 +197,12 @@ INSTANTIATE_TEST_SUITE_P(
             "0-0 rw-p 00000000 00:00 0\n"
             "VmFlags:      mt      ",
             MemoryRegionInfos{
-                MemoryRegionInfo(
-                    make_range(0, 0), MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
-                    ConstString(nullptr), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eYes, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eNo),
+                MemoryRegionInfo(make_range(0, 0), MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 ConstString(nullptr))
+                    .SetIsShadowStack(MemoryRegionInfo::eNo)
+                    .SetMemoryTagged(MemoryRegionInfo::eYes),
             },
             ""),
         // VmFlags line means it has flag info, but nothing is set
@@ -235,13 +210,12 @@ INSTANTIATE_TEST_SUITE_P(
             "0-0 rw-p 00000000 00:00 0\n"
             "VmFlags:         ",
             MemoryRegionInfos{
-                MemoryRegionInfo(
-                    make_range(0, 0), MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
-                    ConstString(nullptr), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eNo),
+                MemoryRegionInfo(make_range(0, 0), MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 ConstString(nullptr))
+                    .SetIsShadowStack(MemoryRegionInfo::eNo)
+                    .SetMemoryTagged(MemoryRegionInfo::eNo),
             },
             ""),
         // Handle some pages not having a flags line
@@ -252,20 +226,16 @@ INSTANTIATE_TEST_SUITE_P(
             "3333-4444 r-xp 00000000 00:00 0    [bar]\n"
             "VmFlags: mt",
             MemoryRegionInfos{
-                MemoryRegionInfo(
-                    make_range(0x1111, 0x2222), MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
-                    ConstString("[foo]"), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eDontKnow),
-                MemoryRegionInfo(
-                    make_range(0x3333, 0x4444), MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
-                    ConstString("[bar]"), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eYes, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eNo),
+                MemoryRegionInfo(make_range(0x1111, 0x2222),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, ConstString("[foo]")),
+                MemoryRegionInfo(make_range(0x3333, 0x4444),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, ConstString("[bar]"))
+                    .SetIsShadowStack(MemoryRegionInfo::eNo)
+                    .SetMemoryTagged(MemoryRegionInfo::eYes),
             },
             ""),
         // Handle no pages having a flags line (older kernels)
@@ -277,20 +247,14 @@ INSTANTIATE_TEST_SUITE_P(
             "KernelPageSize:        4 kB\n"
             "MMUPageSize:           4 kB\n",
             MemoryRegionInfos{
-                MemoryRegionInfo(
-                    make_range(0x1111, 0x2222), MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
-                    ConstString(nullptr), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eDontKnow),
-                MemoryRegionInfo(
-                    make_range(0x3333, 0x4444), MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
-                    ConstString(nullptr), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eDontKnow),
+                MemoryRegionInfo(make_range(0x1111, 0x2222),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, ConstString(nullptr)),
+                MemoryRegionInfo(make_range(0x3333, 0x4444),
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eYes, ConstString(nullptr)),
             },
             ""),
         // We must look for exact flag strings, ignoring substrings of longer
@@ -299,13 +263,25 @@ INSTANTIATE_TEST_SUITE_P(
             "0-0 rw-p 00000000 00:00 0\n"
             "VmFlags: amt mtb amtb",
             MemoryRegionInfos{
-                MemoryRegionInfo(
-                    make_range(0, 0), MemoryRegionInfo::eYes,
-                    MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
-                    ConstString(nullptr), MemoryRegionInfo::eDontKnow, 0,
-                    MemoryRegionInfo::eNo, MemoryRegionInfo::eDontKnow,
-                    MemoryRegionInfo::eNo),
+                MemoryRegionInfo(make_range(0, 0), MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 ConstString(nullptr))
+                    .SetIsShadowStack(MemoryRegionInfo::eNo)
+                    .SetMemoryTagged(MemoryRegionInfo::eNo),
+            },
+            ""),
+        // "ss" means shadow stack.
+        std::make_tuple(
+            "0-0 rw-p 00000000 00:00 0\n"
+            "VmFlags: ss",
+            MemoryRegionInfos{
+                MemoryRegionInfo(make_range(0, 0), MemoryRegionInfo::eYes,
+                                 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                                 MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+                                 ConstString(nullptr))
+                    .SetIsShadowStack(MemoryRegionInfo::eYes)
+                    .SetMemoryTagged(MemoryRegionInfo::eNo),
             },
             "")));
 
diff --git a/lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp b/lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
index 30199bfe5c254..08a7a7331fa71 100644
--- a/lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
+++ b/lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
@@ -230,13 +230,11 @@ TEST(MemoryTagManagerAArch64MTETest, ExpandToGranule) {
 
 static MemoryRegionInfo MakeRegionInfo(lldb::addr_t base, lldb::addr_t size,
                                        bool tagged) {
-  return MemoryRegionInfo(
-      MemoryRegionInfo::RangeType(base, size), MemoryRegionInfo::eYes,
-      MemoryRegionInfo::eYes, MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
-      MemoryRegionInfo::eYes, ConstString(), MemoryRegionInfo::eNo, 0,
-      /*memory_tagged=*/
-      tagged ? MemoryRegionInfo::eYes : MemoryRegionInfo::eNo,
-      MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow);
+  return MemoryRegionInfo(MemoryRegionInfo::RangeType(base, size),
+                          MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+                          MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+                          MemoryRegionInfo::eYes, ConstString())
+      .SetMemoryTagged(tagged ? MemoryRegionInfo::eYes : MemoryRegionInfo::eNo);
 }
 
 TEST(MemoryTagManagerAArch64MTETest, MakeTaggedRange) {
diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
index 44f653c6fa135..085d617bc72b6 100644
--- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -384,20 +384,15 @@ TEST_F(MinidumpParserTest, GetMemoryRegionInfo) {
       parser->BuildMemoryRegions(),
       testing::Pair(testing::ElementsAre(
                         MemoryRegionInfo({0x0, 0x10000}, no, no, no, unknown,
-                                         no, ConstString(), unknown, 0, unknown,
-                                         unknown, unknown),
+                                         no, ConstString()),
                         MemoryRegionInfo({0x10000, 0x21000}, yes, yes, no,
-                                         unknown, yes, ConstString(), unknown,
-                                         0, unknown, unknown, unknown),
+                                         unknown, yes, ConstString()),
                         MemoryRegionInfo({0x40000, 0x1000}, yes, no, no,
-   ...
[truncated]

OptionalBool mapped, ConstString name, OptionalBool flash,
lldb::offset_t blocksize, OptionalBool memory_tagged,
OptionalBool stack_memory, OptionalBool shadow_stack)
OptionalBool mapped, ConstString name)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL OptionalBool. How's that different from LazyBool?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh coincidentally I had the need for such construct: #188722

LazyBool is technically the same thing, just thought the Lazy portion of it didn't describe the usage well in my case. But if we don't like it here, I'll not use OptionalBool in my PR either

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TriBool? :D

@JDevlieghere JDevlieghere Mar 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a more serious note, I think developers of LLDB should be sufficiently aware of LazyBool and IMHO the fact that it doesn't perfectly cover the "unknown" case is not sufficient motivation for adding more types. Maybe an alias eLazyBoolUnknown = eLazyBoolCalculate would be sufficient?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea don't have a strong opinion on it. Happy to just use the eLazyBoolCalculate as the third state

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be ok with picking just one. eDontKnow could be interpreted as "I did not check yet" or "I was not able to check". Which is what it means in the context of MemoryRegionInfo.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think I'm saying: Yes let's use LazyBool and add the alias Jonas suggested.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Michael137 In the interest of me not having to resolve a massive merge conflict with this PR, once this is done I can look at unifying the two types.

Use LazyBool in your code for the time being. Since OptionalBool is confined to MemoryRegionInfo.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DavidSpickett

Copy link
Copy Markdown
Contributor Author

Landed in: #189565, #189580, #189617, #189636

@DavidSpickett
DavidSpickett deleted the lldb-memory-region-info branch March 31, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants