wallace created this revision.
wallace added a reviewer: jj10306.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D127819
Files:
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
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_with_string_numbers.json
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_with_string_numbers.json
===================================================================
--- /dev/null
+++ 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"
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===================================================================
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -21,6 +21,18 @@
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")
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===================================================================
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -22,13 +22,33 @@
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 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 @@
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;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
@@ -228,9 +228,9 @@
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;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
@@ -52,7 +52,7 @@
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 @@
// 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 @@
"timeMult": integer,
"timeShift": integer,
- "timeZero": integer,
+ "timeZero": integer | string decimal | hex string,
}
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
@@ -25,7 +25,7 @@
struct JSONModule {
std::string system_path;
llvm::Optional<std::string> file;
- uint64_t load_address;
+ JSONUINT64 load_address;
llvm::Optional<std::string> uuid;
};
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
@@ -34,7 +34,7 @@
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);
Index: lldb/source/Plugins/Process/Linux/Perf.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/Perf.cpp
+++ lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -45,7 +45,7 @@
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";
Index: lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
===================================================================
--- lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
+++ 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:
Index: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
@@ -142,7 +142,7 @@
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>>>
Index: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
===================================================================
--- lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
+++ lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
@@ -59,6 +59,21 @@
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 @@
uint32_t time_mult;
uint16_t time_shift;
- uint64_t time_zero;
+ JSONUINT64 time_zero;
};
struct TraceIntelPTGetStateResponse : TraceGetStateResponse {
Index: lldb/docs/lldb-gdb-remote.txt
===================================================================
--- lldb/docs/lldb-gdb-remote.txt
+++ lldb/docs/lldb-gdb-remote.txt
@@ -299,14 +299,14 @@
// intel-pt supports both "thread tracing" and "process tracing".
//
// "Process tracing" is implemented in two different 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 @@
// 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 @@
// 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.
// }
//
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
