Skip to content

Commit cb917dd

Browse files
committed
[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. (cherry picked from commit c1e401f)
1 parent a1bcbf7 commit cb917dd

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
@@ -5283,7 +5283,7 @@ uint32_t TargetProperties::GetMaxZeroPaddingInFloatFormat() const {
52835283

52845284
uint32_t TargetProperties::GetMaximumNumberOfChildrenToDisplay() const {
52855285
const uint32_t idx = ePropertyMaxChildrenCount;
5286-
return GetPropertyAtIndexAs<int64_t>(
5286+
return GetPropertyAtIndexAs<uint64_t>(
52875287
idx, g_target_properties[idx].default_uint_value);
52885288
}
52895289

lldb/source/Target/TargetProperties.td

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ let Definition = "target" in {
116116
def MaxZeroPaddingInFloatFormat: Property<"max-zero-padding-in-float-format", "UInt64">,
117117
DefaultUnsignedValue<6>,
118118
Desc<"The maximum number of zeroes to insert when displaying a very small float before falling back to scientific notation.">;
119-
def MaxChildrenCount: Property<"max-children-count", "SInt64">,
119+
def MaxChildrenCount: Property<"max-children-count", "UInt64">,
120120
DefaultUnsignedValue<256>,
121121
Desc<"Maximum number of children to expand in any level of depth.">;
122122
def MaxChildrenDepth: Property<"max-children-depth", "UInt64">,
@@ -125,7 +125,7 @@ let Definition = "target" in {
125125
def MaxSummaryLength: Property<"max-string-summary-length", "UInt64">,
126126
DefaultUnsignedValue<1024>,
127127
Desc<"Maximum number of characters to show when using %s in summary strings.">;
128-
def MaxMemReadSize: Property<"max-memory-read-size", "SInt64">,
128+
def MaxMemReadSize: Property<"max-memory-read-size", "UInt64">,
129129
DefaultUnsignedValue<1024>,
130130
Desc<"Maximum number of bytes that 'memory read' will fetch before --force must be specified.">;
131131
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)