[Lldb-commits] [lldb] 9f45f23 - [trace][intelpt] Support system-wide tracing [21] - Support long numbers in JSON
Walter Erquinigo via lldb-commits
lldb-commits at lists.llvm.org
Thu Jun 16 11:42:35 PDT 2022
Author: Walter Erquinigo
Date: 2022-06-16T11:42:22-07:00
New Revision: 9f45f23d860251edceed6128110855c51af4f39f
URL: https://github.com/llvm/llvm-project/commit/9f45f23d860251edceed6128110855c51af4f39f
DIFF: https://github.com/llvm/llvm-project/commit/9f45f23d860251edceed6128110855c51af4f39f.diff
LOG: [trace][intelpt] Support system-wide tracing [21] - Support long numbers in JSON
llvm's JSON parser supports 64 bit integers, but other tools like the
ones written in JS don't support numbers that big, so we need to
represent these possibly big numbers as a string. This diff uses that to
represent addresses and tsc zero. The former is printed in hex for and
the latter in decimal string form. The schema was updated mentioning
that.
Besides that, I fixed some remaining issues and now all test pass. Before I wasn't running all tests because for some reason my computer reverted perf_paranoid to 1.
Differential Revision: https://reviews.llvm.org/D127819
Added:
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_with_string_numbers.json
Modified:
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
lldb/source/Plugins/Process/Linux/Perf.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
lldb/test/API/commands/trace/TestTraceLoad.py
Removed:
################################################################################
diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 4f0cfa4585b0..d0f069a5c7a9 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -299,14 +299,14 @@ read packet: {"name":<name>, "description":<description>}/E<error code>;AAAAAAAA
// intel-pt supports both "thread tracing" and "process tracing".
//
// "Process tracing" is implemented in two
diff erent ways. If the
-// "perCoreTracing" option is false, then each thread is traced individually
+// "perCpuTracing" option is false, then each thread is traced individually
// but managed by the same "process trace" instance. This means that the
// amount of trace buffers used is proportional to the number of running
// threads. This is the recommended option unless the number of threads is
-// huge. If "perCoreTracing" is true, then each cpu core is traced invidually
+// huge. If "perCpuTracing" is true, then each cpu core is traced invidually
// instead of each thread, which uses a fixed number of trace buffers, but
// might result in less data available for less frequent threads. See
-// "perCoreTracing" below for more information.
+// "perCpuTracing" below for more information.
//
// Each actual intel pt trace buffer, either from "process tracing" or "thread
// tracing", is stored in an in-memory circular buffer, which keeps the most
@@ -352,7 +352,7 @@ read packet: {"name":<name>, "description":<description>}/E<error code>;AAAAAAAA
// 0 if supported.
//
// /* process tracing only */
-// "perCoreTracing": <boolean>
+// "perCpuTracing": <boolean>
// Instead of having an individual trace buffer per thread, this option
// triggers the collection on a per cpu core basis. This effectively
// traces the entire activity on all cores. At decoding time, in order
@@ -374,13 +374,13 @@ read packet: {"name":<name>, "description":<description>}/E<error code>;AAAAAAAA
// buffers for the current process, excluding the ones started with
// "thread tracing".
//
-// If "perCoreTracing" is false, whenever a thread is attempted to be
+// If "perCpuTracing" is false, whenever a thread is attempted to be
// traced due to "process tracing" and the limit would be reached, the
// process is stopped with a "tracing" reason along with a meaningful
// description, so that the user can retrace the process if needed.
//
-// If "perCoreTracing" is true, then starting the system-wide trace
-// session fails if all the individual per-core trace buffers require
+// If "perCpuTracing" is true, then starting the system-wide trace
+// session fails if all the individual per-cpu trace buffers require
// in total more memory that the limit impossed by this parameter.
// }
//
diff --git a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
index 6a9fde7050ee..36b594613a91 100644
--- a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
@@ -59,6 +59,21 @@ bool fromJSON(const llvm::json::Value &value, TraceIntelPTStartRequest &packet,
llvm::json::Value toJSON(const TraceIntelPTStartRequest &packet);
/// \}
+/// Helper structure to help parse long numbers that can't
+/// be easily represented by a JSON number that is compatible with
+/// Javascript (52 bits) or that can also be represented as hex.
+///
+/// \{
+struct JSONUINT64 {
+ uint64_t value;
+};
+
+llvm::json::Value toJSON(const JSONUINT64 &uint64, bool hex);
+
+bool fromJSON(const llvm::json::Value &value, JSONUINT64 &uint64,
+ llvm::json::Path path);
+/// \}
+
/// jLLDBTraceGetState gdb-remote packet
/// \{
@@ -86,7 +101,7 @@ struct LinuxPerfZeroTscConversion {
uint32_t time_mult;
uint16_t time_shift;
- uint64_t time_zero;
+ JSONUINT64 time_zero;
};
struct TraceIntelPTGetStateResponse : TraceGetStateResponse {
diff --git a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
index 1a30d5c71a0b..8d37495fd6fb 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
@@ -142,7 +142,7 @@ llvm::Error IntelPTMultiCoreTrace::TraceStart(lldb::tid_t tid) {
Error IntelPTMultiCoreTrace::TraceStop(lldb::tid_t tid) {
return createStringError(inconvertibleErrorCode(),
"Can't stop tracing an individual thread when "
- "per-core process tracing is enabled.");
+ "per-cpu process tracing is enabled.");
}
Expected<Optional<std::vector<uint8_t>>>
diff --git a/lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h b/lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
index 773151b9a870..c26945af6132 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
+++ b/lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
@@ -16,7 +16,7 @@
namespace lldb_private {
namespace process_linux {
-/// Interface to be implemented by each 'process trace' strategy (per core, per
+/// Interface to be implemented by each 'process trace' strategy (per cpu, per
/// thread, etc).
class IntelPTProcessTrace {
public:
diff --git a/lldb/source/Plugins/Process/Linux/Perf.cpp b/lldb/source/Plugins/Process/Linux/Perf.cpp
index b6e24f921f31..39ea92f49785 100644
--- a/lldb/source/Plugins/Process/Linux/Perf.cpp
+++ b/lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -45,7 +45,7 @@ lldb_private::process_linux::LoadPerfTscConversionParameters() {
perf_event_mmap_page &mmap_metada = perf_event->GetMetadataPage();
if (mmap_metada.cap_user_time && mmap_metada.cap_user_time_zero) {
return LinuxPerfZeroTscConversion{
- mmap_metada.time_mult, mmap_metada.time_shift, mmap_metada.time_zero};
+ mmap_metada.time_mult, mmap_metada.time_shift, {mmap_metada.time_zero}};
} else {
auto err_cap =
!mmap_metada.cap_user_time ? "cap_user_time" : "cap_user_time_zero";
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
index 85f589e7f40d..85b1bfb7e6a0 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
@@ -34,7 +34,7 @@ json::Value toJSON(const JSONModule &module) {
json_module["systemPath"] = module.system_path;
if (module.file)
json_module["file"] = *module.file;
- json_module["loadAddress"] = module.load_address;
+ json_module["loadAddress"] = toJSON(module.load_address, true);
if (module.uuid)
json_module["uuid"] = *module.uuid;
return std::move(json_module);
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
index 277f0c0f636f..f140ef01ccdb 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
@@ -25,7 +25,7 @@ namespace trace_intel_pt {
struct JSONModule {
std::string system_path;
llvm::Optional<std::string> file;
- uint64_t load_address;
+ JSONUINT64 load_address;
llvm::Optional<std::string> uuid;
};
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
index 590f38de18cc..b841f2569a32 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
@@ -52,7 +52,7 @@ Error TraceIntelPTSessionFileParser::ParseModule(Target &target,
return error.ToError();
bool load_addr_changed = false;
- module_sp->SetLoadAddress(target, module.load_address, false,
+ module_sp->SetLoadAddress(target, module.load_address.value, false,
load_addr_changed);
return Error::success();
};
@@ -185,7 +185,7 @@ StringRef TraceIntelPTSessionFileParser::GetSchema() {
// Original path of the module at runtime.
"file"?: string,
// Path to a copy of the file if not available at "systemPath".
- "loadAddress": integer,
+ "loadAddress": integer | string decimal | hex string,
// Lowest address of the sections of the module loaded on memory.
"uuid"?: string,
// Build UUID for the file for sanity checks.
@@ -212,7 +212,7 @@ StringRef TraceIntelPTSessionFileParser::GetSchema() {
"timeMult": integer,
"timeShift": integer,
- "timeZero": integer,
+ "timeZero": integer | string decimal | hex string,
}
}
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
index c282da392656..e7e35bb0229c 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
@@ -228,9 +228,9 @@ BuildModulesSection(Process &process, FileSpec directory) {
inconvertibleErrorCode(),
formatv("couldn't write to the file. {0}", ec.message()));
- json_modules.push_back(JSONModule{system_path,
- path_to_copy_module.GetPath(), load_addr,
- module_sp->GetUUID().GetAsString()});
+ json_modules.push_back(
+ JSONModule{system_path, path_to_copy_module.GetPath(),
+ JSONUINT64{load_addr}, module_sp->GetUUID().GetAsString()});
}
return json_modules;
}
diff --git a/lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp b/lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
index a168bf030c6b..913825cc4dca 100644
--- a/lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ b/lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -22,13 +22,33 @@ bool TraceIntelPTStartRequest::IsPerCpuTracing() const {
return per_cpu_tracing.getValueOr(false);
}
+json::Value toJSON(const JSONUINT64 &uint64, bool hex) {
+ if (hex)
+ return json::Value(formatv("{0:x+}", uint64.value));
+ else
+ return json::Value(formatv("{0}", uint64.value));
+}
+
+bool fromJSON(const json::Value &value, JSONUINT64 &uint64, Path path) {
+ if (Optional<uint64_t> val = value.getAsUINT64()) {
+ uint64.value = *val;
+ return true;
+ } else if (Optional<StringRef> val = value.getAsString()) {
+ if (!val->getAsInteger(/*radix=*/0, uint64.value))
+ return true;
+ path.report("invalid string number");
+ }
+ path.report("invalid number or string number");
+ return false;
+}
+
bool fromJSON(const json::Value &value, TraceIntelPTStartRequest &packet,
Path path) {
ObjectMapper o(value, path);
- if (!o || !fromJSON(value, (TraceStartRequest &)packet, path) ||
- !o.map("enableTsc", packet.enable_tsc) ||
- !o.map("psbPeriod", packet.psb_period) ||
- !o.map("iptTraceSize", packet.ipt_trace_size))
+ if (!(o && fromJSON(value, (TraceStartRequest &)packet, path) &&
+ o.map("enableTsc", packet.enable_tsc) &&
+ o.map("psbPeriod", packet.psb_period) &&
+ o.map("iptTraceSize", packet.ipt_trace_size)))
return false;
if (packet.IsProcessTracing()) {
@@ -54,11 +74,11 @@ uint64_t LinuxPerfZeroTscConversion::ToNanos(uint64_t tsc) const {
uint64_t quot = tsc >> time_shift;
uint64_t rem_flag = (((uint64_t)1 << time_shift) - 1);
uint64_t rem = tsc & rem_flag;
- return time_zero + quot * time_mult + ((rem * time_mult) >> time_shift);
+ return time_zero.value + quot * time_mult + ((rem * time_mult) >> time_shift);
}
uint64_t LinuxPerfZeroTscConversion::ToTSC(uint64_t nanos) const {
- uint64_t time = nanos - time_zero;
+ uint64_t time = nanos - time_zero.value;
uint64_t quot = time / time_mult;
uint64_t rem = time % time_mult;
return (quot << time_shift) + (rem << time_shift) / time_mult;
@@ -68,19 +88,18 @@ json::Value toJSON(const LinuxPerfZeroTscConversion &packet) {
return json::Value(json::Object{
{"timeMult", packet.time_mult},
{"timeShift", packet.time_shift},
- {"timeZero", packet.time_zero},
+ {"timeZero", toJSON(packet.time_zero, /*hex=*/false)},
});
}
bool fromJSON(const json::Value &value, LinuxPerfZeroTscConversion &packet,
json::Path path) {
ObjectMapper o(value, path);
- uint64_t time_mult, time_shift, time_zero;
- if (!o || !o.map("timeMult", time_mult) || !o.map("timeShift", time_shift) ||
- !o.map("timeZero", time_zero))
+ uint64_t time_mult, time_shift;
+ if (!(o && o.map("timeMult", time_mult) && o.map("timeShift", time_shift) &&
+ o.map("timeZero", packet.time_zero)))
return false;
packet.time_mult = time_mult;
- packet.time_zero = time_zero;
packet.time_shift = time_shift;
return true;
}
diff --git a/lldb/test/API/commands/trace/TestTraceLoad.py b/lldb/test/API/commands/trace/TestTraceLoad.py
index 352ccf9e201f..3dfcc88fa777 100644
--- a/lldb/test/API/commands/trace/TestTraceLoad.py
+++ b/lldb/test/API/commands/trace/TestTraceLoad.py
@@ -21,6 +21,18 @@ def testLoadMultiCoreTrace(self):
substrs=["67910: [tsc=0x008fb5211bfdf270] 0x0000000000400bd7 addl $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
+ def testLoadMultiCoreTraceWithStringNumbers(self):
+ src_dir = self.getSourceDir()
+ trace_definition_file = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_with_string_numbers.json")
+ self.expect("trace load -v " + trace_definition_file, substrs=["intel-pt"])
+ self.expect("thread trace dump instructions 2 -t",
+ substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+ "m.out`foo() + 65 at multi_thread.cpp:12:21",
+ "19520: [tsc=0x008fb5211bfbc69e] 0x0000000000400ba7 jg 0x400bb3"])
+ self.expect("thread trace dump instructions 3 -t",
+ substrs=["67910: [tsc=0x008fb5211bfdf270] 0x0000000000400bd7 addl $0x1, -0x4(%rbp)",
+ "m.out`bar() + 26 at multi_thread.cpp:20:6"])
+
def testLoadMultiCoreTraceWithMissingThreads(self):
src_dir = self.getSourceDir()
trace_definition_file = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_missing_threads.json")
diff --git a/lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_with_string_numbers.json b/lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_with_string_numbers.json
new file mode 100644
index 000000000000..d7c13074aba7
--- /dev/null
+++ b/lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_with_string_numbers.json
@@ -0,0 +1,50 @@
+{
+ "cpus": [
+ {
+ "contextSwitchTrace": "cores/45.perf_context_switch_trace",
+ "id": 45,
+ "iptTrace": "cores/45.intelpt_trace"
+ },
+ {
+ "contextSwitchTrace": "cores/51.perf_context_switch_trace",
+ "id": 51,
+ "iptTrace": "cores/51.intelpt_trace"
+ }
+ ],
+ "cpuInfo": {
+ "family": 6,
+ "model": 85,
+ "stepping": 4,
+ "vendor": "GenuineIntel"
+ },
+ "processes": [
+ {
+ "modules": [
+ {
+ "file": "modules/m.out",
+ "systemPath": "/tmp/m.out",
+ "loadAddress": "4194304",
+ "uuid": "AEFB0D59-233F-80FF-6D3C-4DED498534CF-11017B3B"
+ }
+ ],
+ "pid": 3497234,
+ "threads": [
+ {
+ "tid": 3497234
+ },
+ {
+ "tid": 3497496
+ },
+ {
+ "tid": 3497497
+ }
+ ]
+ }
+ ],
+ "tscPerfZeroConversion": {
+ "timeMult": 1076264588,
+ "timeShift": 31,
+ "timeZero": "18433473881008870804"
+ },
+ "type": "intel-pt"
+}
More information about the lldb-commits
mailing list