llvm / llvm/llvm-project

Change to default value of `target.max-children-depth` setting broke repeating `memory read` commands

Open
#192,057 1 comment 1 reaction 0 assignees View on GitHub
lldb
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

Currently the default is:
```
(lldb) settings show target.max-children-depth
target.max-children-depth (unsigned) = 5
```
This was set by https://github.com/llvm/llvm-project/pull/149282, which changed it from `0xFFFFFFFF` aka `UINT32_MAX`.

The result of this is that `OptionGroupValueObjectDisplay` now thinks that `AnyOptionWasSet` is always true in the case where there is a target to get the setting from.

The result of this is that `memory read` cannot be repeated as expected (other commands too I expect but they are less likely to have a repeating mode). Whenever any option was set, the set of options from the previous use is discarded.

Current behaviour:
```
(lldb) memory read -f "uint8_t" main
0xaaaaaaaa0714: {0xff 0x43 0x00 ...
0xaaaaaaaa0794: {0x6c 0xfe 0xff ...
0xaaaaaaaa0814: {0x42 0x0e 0x00 ...
0xaaaaaaaa0894: {0x00 0x00 0x00 ...
(lldb)
0xaaaaaaaa0914: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
0xaaaaaaaa0924: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
```
The second command should keep the format from the first. Which is what happens if I restore the previous default:
```
(lldb) memory read -f "uint8_t" main
0xaaaaaaaa0714: {0xff 0x43 ...
0xaaaaaaaa0794: {0x6c 0xfe ...
0xaaaaaaaa0814: {0x42 0x0e ...
0xaaaaaaaa0894: {0x00 0x00 ...
(lldb)
0xaaaaaaaa0914: {0x00 0x00 ...
0xaaaaaaaa0994: {0x00 0x00 ...
0xaaaaaaaa0a14: {0x00 0x00 ...
0xaaaaaaaa0a94: {0x00 0x00 ...
```
I only realised this when running memory tagging tests, specifically `test_mte_memory_read_tag_display_repeated`. So that's on me for not doing that for some time, but we can test this general feature on any hardware.

I suspect we test the auto increment of the address but not carrying over command options.

One wonders if memory read should even have these options, and apparently the answer is yes, because you can give it a type to show the memory as:
```
(lldb) memory read -t "Foo" main
(Foo) 0xaaaaaaaa0714 = {
a = -788511745
a_bar = {
b = 1384120320
a_baz = (c = -1862253569)
}
}
(lldb) settings set target.max-children-depth 1
(lldb) memory read -t "Foo" main
(Foo) 0xaaaaaaaa0714 = {
a = -788511745
a_bar = {...}
}
```

I think the fix is something like instead of checking for `max_depth != UINT32_MAX`, check for `!max_depth_is_default`. Though this will always be false if we get the depth from the setting and the setting is not the default value of 5.

So really the question is does the new depth match the old depth, regardless of where it came from.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.