Skip to content

Commit

Permalink
[lldb] Fix term-width setting (llvm#82736)
Browse files Browse the repository at this point in the history
I noticed that the term-width setting would always report its default
value (80) despite the driver correctly setting the value with
SBDebugger::SetTerminalWidth.

```
(lldb) settings show term-width
term-width (int) = 80
```

The issue is that the setting was defined as a SInt64 instead of a
UInt64 while the getter returned an unsigned value. There's no reason
the terminal width should be a signed value. My best guess it that it
was using SInt64 because UInt64 didn't support min and max values. I
fixed that and correct the type and now lldb reports the correct
terminal width:

```
(lldb) settings show term-width
term-width (unsigned) = 189
```

rdar://123488999
  • Loading branch information
JDevlieghere authored Feb 23, 2024
1 parent 0d72fe9 commit afd4690
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 15 deletions.
4 changes: 2 additions & 2 deletions lldb/include/lldb/Interpreter/OptionValueSInt64.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ class OptionValueSInt64 : public Cloneable<OptionValueSInt64, OptionValue> {
protected:
int64_t m_current_value = 0;
int64_t m_default_value = 0;
int64_t m_min_value = INT64_MIN;
int64_t m_max_value = INT64_MAX;
int64_t m_min_value = std::numeric_limits<int64_t>::min();
int64_t m_max_value = std::numeric_limits<int64_t>::max();
};

} // namespace lldb_private
Expand Down
26 changes: 24 additions & 2 deletions lldb/include/lldb/Interpreter/OptionValueUInt64.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,35 @@ class OptionValueUInt64 : public Cloneable<OptionValueUInt64, OptionValue> {

uint64_t GetDefaultValue() const { return m_default_value; }

void SetCurrentValue(uint64_t value) { m_current_value = value; }
bool SetCurrentValue(uint64_t value) {
if (value >= m_min_value && value <= m_max_value) {
m_current_value = value;
return true;
}
return false;
}

bool SetDefaultValue(uint64_t value) {
if (value >= m_min_value && value <= m_max_value) {
m_default_value = value;
return true;
}
return false;
}

void SetMinimumValue(int64_t v) { m_min_value = v; }

uint64_t GetMinimumValue() const { return m_min_value; }

void SetMaximumValue(int64_t v) { m_max_value = v; }

void SetDefaultValue(uint64_t value) { m_default_value = value; }
uint64_t GetMaximumValue() const { return m_max_value; }

protected:
uint64_t m_current_value = 0;
uint64_t m_default_value = 0;
uint64_t m_min_value = std::numeric_limits<uint64_t>::min();
uint64_t m_max_value = std::numeric_limits<uint64_t>::max();
};

} // namespace lldb_private
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Core/CoreProperties.td
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ let Definition = "debugger" in {
Global,
DefaultStringValue<"${ansi.normal}">,
Desc<"When displaying the line marker in a color-enabled terminal, use the ANSI terminal code specified in this format immediately after the line to be marked.">;
def TerminalWidth: Property<"term-width", "SInt64">,
def TerminalWidth: Property<"term-width", "UInt64">,
Global,
DefaultUnsignedValue<80>,
Desc<"The maximum number of columns to use for displaying text.">;
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Core/Debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -886,8 +886,8 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, void *baton)
}
assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");

OptionValueSInt64 *term_width =
m_collection_sp->GetPropertyAtIndexAsOptionValueSInt64(
OptionValueUInt64 *term_width =
m_collection_sp->GetPropertyAtIndexAsOptionValueUInt64(
ePropertyTerminalWidth);
term_width->SetMinimumValue(10);
term_width->SetMaximumValue(1024);
Expand Down
13 changes: 10 additions & 3 deletions lldb/source/Interpreter/OptionValueUInt64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,16 @@ Status OptionValueUInt64::SetValueFromString(llvm::StringRef value_ref,
llvm::StringRef value_trimmed = value_ref.trim();
uint64_t value;
if (llvm::to_integer(value_trimmed, value)) {
m_value_was_set = true;
m_current_value = value;
NotifyValueChanged();
if (value >= m_min_value && value <= m_max_value) {
m_value_was_set = true;
m_current_value = value;
NotifyValueChanged();
} else {
error.SetErrorStringWithFormat(
"%" PRIu64 " is out of range, valid values must be between %" PRIu64
" and %" PRIu64 ".",
value, m_min_value, m_max_value);
}
} else {
error.SetErrorStringWithFormat("invalid uint64_t string value: '%s'",
value_ref.str().c_str());
Expand Down
15 changes: 11 additions & 4 deletions lldb/test/API/commands/settings/TestSettings.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
Test lldb settings command.
"""


import json
import os
import re
Expand Down Expand Up @@ -151,14 +150,22 @@ def test_set_term_width(self):
self.expect(
"settings show term-width",
SETTING_MSG("term-width"),
startstr="term-width (int) = 70",
startstr="term-width (unsigned) = 70",
)

# The overall display should also reflect the new setting.
self.expect(
"settings show",
SETTING_MSG("term-width"),
substrs=["term-width (int) = 70"],
substrs=["term-width (unsigned) = 70"],
)

self.dbg.SetTerminalWidth(60)

self.expect(
"settings show",
SETTING_MSG("term-width"),
substrs=["term-width (unsigned) = 60"],
)

# rdar://problem/10712130
Expand Down Expand Up @@ -593,7 +600,7 @@ def test_settings_with_trailing_whitespace(self):
self.expect(
"settings show term-width",
SETTING_MSG("term-width"),
startstr="term-width (int) = 60",
startstr="term-width (unsigned) = 60",
)
self.runCmd("settings clear term-width", check=False)
# string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ def do_test(self, term_width, pattern_list):
)
self.expect("set set term-width " + str(term_width))
self.expect(
"set show term-width", substrs=["term-width (int) = " + str(term_width)]
"set show term-width",
substrs=["term-width (unsigned) = " + str(term_width)],
)

self.child.send("file " + self.getBuildArtifact("a.out") + "\n")
Expand Down

0 comments on commit afd4690

Please sign in to comment.