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 +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
