wallace created this revision.
wallace added reviewers: persona0220, jj10306.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Thanks to rnofe...@fb.com for coming up with these changes.
This diff adds support for passing units in the trace size inputs. For example,
it's now possible to specify 64KB as the trace size, instead of the
problematic 65536. This makes the user experience a bit friendlier.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D129613
Files:
lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
lldb/test/API/commands/trace/TestTraceStartStop.py
Index: lldb/test/API/commands/trace/TestTraceStartStop.py
===================================================================
--- lldb/test/API/commands/trace/TestTraceStartStop.py
+++ lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -46,6 +46,62 @@
self.traceStartThread(iptTraceSize=1048576)
+ @testSBAPIAndCommands
+ def testStartSessionWithSizeDeclarationInUnits(self):
+ self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+ self.expect("b main")
+ self.expect("r")
+
+ self.traceStartThread(
+ error=True, iptTraceSize="abc",
+ substrs=["invalid bytes expression for 'abc'"])
+
+ self.traceStartThread(
+ error=True, iptTraceSize="123.12",
+ substrs=["invalid bytes expression for '123.12'"])
+
+ self.traceStartThread(
+ error=True, iptTraceSize="\"\"",
+ substrs=["invalid bytes expression for ''"])
+
+ self.traceStartThread(
+ error=True, iptTraceSize="2000B",
+ substrs=["The intel pt trace size must be a power of 2 greater than or equal to 4096 (2^12) bytes. It was 2000"])
+
+ self.traceStartThread(
+ error=True, iptTraceSize="3MB",
+ substrs=["The intel pt trace size must be a power of 2 greater than or equal to 4096 (2^12) bytes. It was 3145728"])
+
+ self.traceStartThread(
+ error=True, iptTraceSize="3MiB",
+ substrs=["The intel pt trace size must be a power of 2 greater than or equal to 4096 (2^12) bytes. It was 3145728"])
+
+ self.traceStartThread(
+ error=True, iptTraceSize="3mib",
+ substrs=["The intel pt trace size must be a power of 2 greater than or equal to 4096 (2^12) bytes. It was 3145728"])
+
+ self.traceStartThread(
+ error=True, iptTraceSize="3M",
+ substrs=["The intel pt trace size must be a power of 2 greater than or equal to 4096 (2^12) bytes. It was 3145728"])
+
+ self.traceStartThread(
+ error=True, iptTraceSize="3KB",
+ substrs=["The intel pt trace size must be a power of 2 greater than or equal to 4096 (2^12) bytes. It was 3072"])
+
+ self.traceStartThread(
+ error=True, iptTraceSize="3KiB",
+ substrs=["The intel pt trace size must be a power of 2 greater than or equal to 4096 (2^12) bytes. It was 3072"])
+
+ self.traceStartThread(
+ error=True, iptTraceSize="3K",
+ substrs=["The intel pt trace size must be a power of 2 greater than or equal to 4096 (2^12) bytes. It was 3072"])
+
+ self.traceStartThread(
+ error=True, iptTraceSize="3MS",
+ substrs=["invalid bytes expression for '3MS'"])
+
+ self.traceStartThread(iptTraceSize="1048576")
+
@skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
def testSBAPIHelp(self):
self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
@@ -11,7 +11,9 @@
Arg<"Value">,
Desc<"Trace size in bytes per thread. It must be a power of 2 greater "
"than or equal to 4096 (2^12). The trace is circular keeping "
- "the most recent data. Defaults to 4096 bytes.">;
+ "the most recent data. Defaults to 4096 bytes. It's possible to "
+ "specify size using multiples of unit bytes, e.g., 4KB, 1MB, 1MiB, "
+ "where 1K is 1024 bytes and 1M is 1048576 bytes.">;
def thread_trace_start_intel_pt_tsc: Option<"tsc", "t">,
Group<1>,
Desc<"Enable the use of TSC timestamps. This is supported on all devices "
@@ -40,7 +42,9 @@
Arg<"Value">,
Desc<"Size in bytes used by each individual per-thread or per-cpu trace "
"buffer. It must be a power of 2 greater than or equal to 4096 (2^12) "
- "bytes.">;
+ "bytes. It's possible to "
+ "specify size using multiples of unit bytes, e.g., 4KB, 1MB, 1MiB, "
+ "where 1K is 1024 bytes and 1M is 1048576 bytes.">;
def process_trace_start_intel_pt_per_cpu_tracing:
Option<"per-cpu-tracing", "c">,
Group<1>,
@@ -62,7 +66,9 @@
"Whenever a thread is attempted to be traced due to this command and "
"the limit would be reached, the process is stopped with a "
"\"processor trace\" reason, so that the user can retrace the process "
- "if needed. Defaults to 500MB.">;
+ "if needed. Defaults to 500MB. It's possible to "
+ "specify size using multiples of unit bytes, e.g., 4KB, 500MB, 500MiB, "
+ "where 1K is 1024 bytes and 1M is 1048576 bytes.">;
def process_trace_start_intel_pt_tsc: Option<"tsc", "t">,
Group<1>,
Desc<"Enable the use of TSC timestamps. This is supported on all devices "
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -476,7 +476,16 @@
if (configuration) {
if (StructuredData::Dictionary *dict = configuration->GetAsDictionary()) {
- dict->GetValueForKeyAsInteger("iptTraceSize", ipt_trace_size);
+ llvm::StringRef ipt_trace_size_not_parsed;
+ if (dict->GetValueForKeyAsString("iptTraceSize", ipt_trace_size_not_parsed)) {
+ if (Optional<uint64_t> bytes = ParsingUtils::ParseUserFriendlySizeExpression(ipt_trace_size_not_parsed))
+ ipt_trace_size = *bytes;
+ else
+ return createStringError(inconvertibleErrorCode(), "iptTraceSize is wrong bytes expression");
+ } else {
+ dict->GetValueForKeyAsInteger("iptTraceSize", ipt_trace_size);
+ }
+
dict->GetValueForKeyAsBoolean("enableTsc", enable_tsc);
dict->GetValueForKeyAsInteger("psbPeriod", psb_period);
} else {
Index: lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
@@ -109,6 +109,17 @@
CommandOptions m_options;
};
+namespace ParsingUtils {
+ /// Convert an integral size expression like 12KiB or 4MB into bytes. The units are
+ /// taken loosely to help users input sizes into LLDB, e.g. KiB and KB are considered the same (2^20 bytes) for simplicity.
+ ///
+ /// \param[in] size_expression
+ /// String expression which is an integral number plus a unit that can be lower or upper case. Supported units: K, KB and KiB for 2^10 bytes; M, MB and MiB for 2^20 bytes; and B for bytes. A single integral number is considered bytes.
+ /// \return
+ /// The converted number of bytes or \a llvm::None if the expression is invalid.
+ llvm::Optional<uint64_t> ParseUserFriendlySizeExpression(llvm::StringRef size_expression);
+} // namespace ParsingUtils
+
} // namespace trace_intel_pt
} // namespace lldb_private
Index: lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
@@ -32,13 +32,10 @@
switch (short_option) {
case 's': {
- int64_t ipt_trace_size;
- if (option_arg.empty() || option_arg.getAsInteger(0, ipt_trace_size) ||
- ipt_trace_size < 0)
- error.SetErrorStringWithFormat("invalid integer value for option '%s'",
- option_arg.str().c_str());
+ if (Optional<uint64_t> bytes = ParsingUtils::ParseUserFriendlySizeExpression(option_arg))
+ m_ipt_trace_size = *bytes;
else
- m_ipt_trace_size = ipt_trace_size;
+ error.SetErrorStringWithFormat("invalid bytes expression for '%s'", option_arg.str().c_str());
break;
}
case 't': {
@@ -98,24 +95,17 @@
switch (short_option) {
case 's': {
- int64_t ipt_trace_size;
- if (option_arg.empty() || option_arg.getAsInteger(0, ipt_trace_size) ||
- ipt_trace_size < 0)
- error.SetErrorStringWithFormat("invalid integer value for option '%s'",
- option_arg.str().c_str());
+ if (Optional<uint64_t> bytes = ParsingUtils::ParseUserFriendlySizeExpression(option_arg))
+ m_ipt_trace_size = *bytes;
else
- m_ipt_trace_size = ipt_trace_size;
+ error.SetErrorStringWithFormat("invalid bytes expression for '%s'", option_arg.str().c_str());
break;
}
case 'l': {
- int64_t process_buffer_size_limit;
- if (option_arg.empty() ||
- option_arg.getAsInteger(0, process_buffer_size_limit) ||
- process_buffer_size_limit < 0)
- error.SetErrorStringWithFormat("invalid integer value for option '%s'",
- option_arg.str().c_str());
+ if (Optional<uint64_t> bytes = ParsingUtils::ParseUserFriendlySizeExpression(option_arg))
+ m_process_buffer_size_limit = *bytes;
else
- m_process_buffer_size_limit = process_buffer_size_limit;
+ error.SetErrorStringWithFormat("invalid bytes expression for '%s'", option_arg.str().c_str());
break;
}
case 't': {
@@ -168,3 +158,43 @@
return result.Succeeded();
}
+
+Optional<uint64_t> ParsingUtils::ParseUserFriendlySizeExpression(llvm::StringRef size_expression) {
+ if (size_expression.empty()) {
+ return llvm::None;
+ }
+
+ std::map<std::string, uint64_t> multipliers = {
+ {"mib", 1024 * 1024},
+ {"mb", 1024 * 1024},
+ {"m", 1024 * 1024},
+ {"kib", 1024},
+ {"kb", 1024},
+ {"k", 1024},
+ {"b", 1},
+ {"", 1}
+ };
+
+ const auto non_digit_index = size_expression.find_first_not_of("0123456789");
+ if (non_digit_index == 0) { // expression starts from from non-digit char.
+ return llvm::None;
+ }
+
+ const llvm::StringRef number_part = non_digit_index == llvm::StringRef::npos ? size_expression : size_expression.substr(0, non_digit_index);
+ uint64_t parsed_number;
+ if (number_part.getAsInteger(10, parsed_number)) {
+ return llvm::None;
+ }
+
+ if (non_digit_index != llvm::StringRef::npos) { // if expression has units.
+ const auto multiplier = size_expression.substr(non_digit_index).lower();
+
+ if (!multipliers.count(multiplier)) {
+ return llvm::None;
+ }
+
+ return parsed_number * multipliers.at(multiplier);
+ } else {
+ return parsed_number;
+ }
+}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
