[Lldb-commits] [lldb] c1e401f - [lldb] Change the two remaining SInt64 settings in Target to uint (#105460)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Aug 22 10:10:18 PDT 2024
Author: Jason Molenda
Date: 2024-08-22T10:10:15-07:00
New Revision: c1e401f3624780f85f4c9a26960752ee3f37fafb
URL: https://github.com/llvm/llvm-project/commit/c1e401f3624780f85f4c9a26960752ee3f37fafb
DIFF: https://github.com/llvm/llvm-project/commit/c1e401f3624780f85f4c9a26960752ee3f37fafb.diff
LOG: [lldb] Change the two remaining SInt64 settings in Target to uint (#105460)
TargetProperties.td had a few settings listed as signed integral values,
but the Target.cpp methods reading those values were reading them as
unsigned. e.g. target.max-memory-read-size, some accesses of
target.max-children-count, still today, previously
target.max-string-summary-length.
After Jonas' change to use templates to read these values in
https://reviews.llvm.org/D149774, when the code tried to fetch these
values, we'd eventually end up calling OptionValue::GetAsUInt64 which
checks that the value is actually a UInt64 before returning it; finding
that it was an SInt64, it would drop the user setting and return the
default value. This manifested as a bug that target.max-memory-read-size
is never used for memory read.
target.max-children-count is less straightforward, where one read of
that setting was fetching it as an int64_t, the other as a uint64_t.
I suspect all of these settings were originally marked as SInt64 so a
user could do -1 for "infinite", getting it static_cast to a UINT64_MAX
value along the way. I can't find any documentation for this behavior,
but it seems like something Greg would have done. We've partially lost
that behavior already via
https://github.com/llvm/llvm-project/pull/72233 for
target.max-string-summary-length, and this further removes it.
We're still fetching UInt64's and returning them as uint32_t's but I'm
not overly pressed about someone setting a count/size limit over 4GB.
I added a simple API test for the memory read setting limit.
Added:
lldb/test/API/functionalities/memory/big-read/Makefile
lldb/test/API/functionalities/memory/big-read/TestMemoryReadMaximumSize.py
lldb/test/API/functionalities/memory/big-read/main.c
Modified:
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
Removed:
################################################################################
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 5a5d689e03fbc0..260974bddedf3a 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -4609,7 +4609,7 @@ uint32_t TargetProperties::GetMaxZeroPaddingInFloatFormat() const {
uint32_t TargetProperties::GetMaximumNumberOfChildrenToDisplay() const {
const uint32_t idx = ePropertyMaxChildrenCount;
- return GetPropertyAtIndexAs<int64_t>(
+ return GetPropertyAtIndexAs<uint64_t>(
idx, g_target_properties[idx].default_uint_value);
}
diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td
index 421252aa4aea26..7bb5bd53688b14 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -92,7 +92,7 @@ let Definition = "target" in {
def MaxZeroPaddingInFloatFormat: Property<"max-zero-padding-in-float-format", "UInt64">,
DefaultUnsignedValue<6>,
Desc<"The maximum number of zeroes to insert when displaying a very small float before falling back to scientific notation.">;
- def MaxChildrenCount: Property<"max-children-count", "SInt64">,
+ def MaxChildrenCount: Property<"max-children-count", "UInt64">,
DefaultUnsignedValue<256>,
Desc<"Maximum number of children to expand in any level of depth.">;
def MaxChildrenDepth: Property<"max-children-depth", "UInt64">,
@@ -101,7 +101,7 @@ let Definition = "target" in {
def MaxSummaryLength: Property<"max-string-summary-length", "UInt64">,
DefaultUnsignedValue<1024>,
Desc<"Maximum number of characters to show when using %s in summary strings.">;
- def MaxMemReadSize: Property<"max-memory-read-size", "SInt64">,
+ def MaxMemReadSize: Property<"max-memory-read-size", "UInt64">,
DefaultUnsignedValue<1024>,
Desc<"Maximum number of bytes that 'memory read' will fetch before --force must be specified.">;
def BreakpointUseAvoidList: Property<"breakpoints-use-platform-avoid-list", "Boolean">,
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
index 072a580afe24e4..185a24cf6dce30 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
@@ -2,7 +2,6 @@
Test lldb data formatter subsystem.
"""
-
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
@@ -51,7 +50,7 @@ def do_test(self, stdlib_type):
self.expect(
"settings show target.max-children-count",
matching=True,
- substrs=["target.max-children-count (int) = 256"],
+ substrs=["target.max-children-count (unsigned) = 256"],
)
self.expect(
@@ -132,7 +131,7 @@ def do_test_ptr_and_ref(self, stdlib_type):
self.expect(
"settings show target.max-children-count",
matching=True,
- substrs=["target.max-children-count (int) = 256"],
+ substrs=["target.max-children-count (unsigned) = 256"],
)
self.expect(
diff --git a/lldb/test/API/functionalities/memory/big-read/Makefile b/lldb/test/API/functionalities/memory/big-read/Makefile
new file mode 100644
index 00000000000000..10495940055b63
--- /dev/null
+++ b/lldb/test/API/functionalities/memory/big-read/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/memory/big-read/TestMemoryReadMaximumSize.py b/lldb/test/API/functionalities/memory/big-read/TestMemoryReadMaximumSize.py
new file mode 100644
index 00000000000000..259fde71a63626
--- /dev/null
+++ b/lldb/test/API/functionalities/memory/big-read/TestMemoryReadMaximumSize.py
@@ -0,0 +1,31 @@
+"""
+Test the maximum memory read setting.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class TestMemoryReadMaximumSize(TestBase):
+ def test_memory_read_max_setting(self):
+ """Test the target.max-memory-read-size setting."""
+ self.build()
+ (
+ self.target,
+ self.process,
+ self.thread,
+ self.bp,
+ ) = lldbutil.run_to_source_breakpoint(
+ self, "breakpoint here", lldb.SBFileSpec("main.c")
+ )
+ self.assertTrue(self.bp.IsValid())
+
+ self.expect(
+ "mem rea -f x -s 4 -c 2048 `&c`",
+ error=True,
+ substrs=["Normally, 'memory read' will not read over 1024 bytes of data"],
+ )
+ self.runCmd("settings set target.max-memory-read-size `2048 * sizeof(int)`")
+ self.expect("mem rea -f x -s 4 -c 2048 `&c`", substrs=["feed"])
diff --git a/lldb/test/API/functionalities/memory/big-read/main.c b/lldb/test/API/functionalities/memory/big-read/main.c
new file mode 100644
index 00000000000000..a9143a50d093b8
--- /dev/null
+++ b/lldb/test/API/functionalities/memory/big-read/main.c
@@ -0,0 +1,9 @@
+#include <string.h>
+int main() {
+ int c[2048];
+ memset(c, 0, 2048 * sizeof(int));
+
+ c[2047] = 0xfeed;
+
+ return c[2047]; // breakpoint here
+}
More information about the lldb-commits
mailing list