Skip to content

Commit c1e401f

Browse files
authored
[lldb] Change the two remaining SInt64 settings in Target to uint (llvm#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 llvm#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.
1 parent 4a2a1b5 commit c1e401f

File tree

6 files changed

+48
-6
lines changed

6 files changed

+48
-6
lines changed

lldb/source/Target/Target.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4609,7 +4609,7 @@ uint32_t TargetProperties::GetMaxZeroPaddingInFloatFormat() const {
46094609

46104610
uint32_t TargetProperties::GetMaximumNumberOfChildrenToDisplay() const {
46114611
const uint32_t idx = ePropertyMaxChildrenCount;
4612-
return GetPropertyAtIndexAs<int64_t>(
4612+
return GetPropertyAtIndexAs<uint64_t>(
46134613
idx, g_target_properties[idx].default_uint_value);
46144614
}
46154615

lldb/source/Target/TargetProperties.td

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ let Definition = "target" in {
9292
def MaxZeroPaddingInFloatFormat: Property<"max-zero-padding-in-float-format", "UInt64">,
9393
DefaultUnsignedValue<6>,
9494
Desc<"The maximum number of zeroes to insert when displaying a very small float before falling back to scientific notation.">;
95-
def MaxChildrenCount: Property<"max-children-count", "SInt64">,
95+
def MaxChildrenCount: Property<"max-children-count", "UInt64">,
9696
DefaultUnsignedValue<256>,
9797
Desc<"Maximum number of children to expand in any level of depth.">;
9898
def MaxChildrenDepth: Property<"max-children-depth", "UInt64">,
@@ -101,7 +101,7 @@ let Definition = "target" in {
101101
def MaxSummaryLength: Property<"max-string-summary-length", "UInt64">,
102102
DefaultUnsignedValue<1024>,
103103
Desc<"Maximum number of characters to show when using %s in summary strings.">;
104-
def MaxMemReadSize: Property<"max-memory-read-size", "SInt64">,
104+
def MaxMemReadSize: Property<"max-memory-read-size", "UInt64">,
105105
DefaultUnsignedValue<1024>,
106106
Desc<"Maximum number of bytes that 'memory read' will fetch before --force must be specified.">;
107107
def BreakpointUseAvoidList: Property<"breakpoints-use-platform-avoid-list", "Boolean">,

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
Test lldb data formatter subsystem.
33
"""
44

5-
65
import lldb
76
from lldbsuite.test.decorators import *
87
from lldbsuite.test.lldbtest import *
@@ -51,7 +50,7 @@ def do_test(self, stdlib_type):
5150
self.expect(
5251
"settings show target.max-children-count",
5352
matching=True,
54-
substrs=["target.max-children-count (int) = 256"],
53+
substrs=["target.max-children-count (unsigned) = 256"],
5554
)
5655

5756
self.expect(
@@ -132,7 +131,7 @@ def do_test_ptr_and_ref(self, stdlib_type):
132131
self.expect(
133132
"settings show target.max-children-count",
134133
matching=True,
135-
substrs=["target.max-children-count (int) = 256"],
134+
substrs=["target.max-children-count (unsigned) = 256"],
136135
)
137136

138137
self.expect(
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
C_SOURCES := main.c
2+
3+
include Makefile.rules
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
"""
2+
Test the maximum memory read setting.
3+
"""
4+
5+
import lldb
6+
from lldbsuite.test.decorators import *
7+
from lldbsuite.test.lldbtest import *
8+
import lldbsuite.test.lldbutil as lldbutil
9+
10+
11+
class TestMemoryReadMaximumSize(TestBase):
12+
def test_memory_read_max_setting(self):
13+
"""Test the target.max-memory-read-size setting."""
14+
self.build()
15+
(
16+
self.target,
17+
self.process,
18+
self.thread,
19+
self.bp,
20+
) = lldbutil.run_to_source_breakpoint(
21+
self, "breakpoint here", lldb.SBFileSpec("main.c")
22+
)
23+
self.assertTrue(self.bp.IsValid())
24+
25+
self.expect(
26+
"mem rea -f x -s 4 -c 2048 `&c`",
27+
error=True,
28+
substrs=["Normally, 'memory read' will not read over 1024 bytes of data"],
29+
)
30+
self.runCmd("settings set target.max-memory-read-size `2048 * sizeof(int)`")
31+
self.expect("mem rea -f x -s 4 -c 2048 `&c`", substrs=["feed"])
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#include <string.h>
2+
int main() {
3+
int c[2048];
4+
memset(c, 0, 2048 * sizeof(int));
5+
6+
c[2047] = 0xfeed;
7+
8+
return c[2047]; // breakpoint here
9+
}

0 commit comments

Comments
 (0)