From 9f45f23d860251edceed6128110855c51af4f39f Mon Sep 17 00:00:00 2001 From: Walter Erquinigo Date: Tue, 14 Jun 2022 19:05:30 -0700 Subject: [PATCH] [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 --- lldb/docs/lldb-gdb-remote.txt | 14 +++--- .../lldb/Utility/TraceIntelPTGDBRemotePackets.h | 17 +++++++- .../Process/Linux/IntelPTMultiCoreTrace.cpp | 2 +- .../Plugins/Process/Linux/IntelPTProcessTrace.h | 2 +- lldb/source/Plugins/Process/Linux/Perf.cpp | 2 +- .../Trace/intel-pt/TraceIntelPTJSONStructs.cpp | 2 +- .../Trace/intel-pt/TraceIntelPTJSONStructs.h | 2 +- .../intel-pt/TraceIntelPTSessionFileParser.cpp | 6 +-- .../Trace/intel-pt/TraceIntelPTSessionSaver.cpp | 6 +-- .../Utility/TraceIntelPTGDBRemotePackets.cpp | 41 +++++++++++++----- lldb/test/API/commands/trace/TestTraceLoad.py | 12 ++++++ .../trace_with_string_numbers.json | 50 ++++++++++++++++++++++ 12 files changed, 126 insertions(+), 30 deletions(-) create mode 100644 lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_with_string_numbers.json diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt index 4f0cfa4..d0f069a 100644 --- a/lldb/docs/lldb-gdb-remote.txt +++ b/lldb/docs/lldb-gdb-remote.txt @@ -299,14 +299,14 @@ read packet: {"name":, "description":}/E;AAAAAAAA // 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 @@ read packet: {"name":, "description":}/E;AAAAAAAA // 0 if supported. // // /* process tracing only */ -// "perCoreTracing": +// "perCpuTracing": // 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":, "description":}/E;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 6a9fde7..36b5946 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 1a30d5c..8d37495 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>> diff --git a/lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h b/lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h index 773151b..c26945a 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 b6e24f9..39ea92f 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 85f589e..85b1bfb 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 277f0c0..f140ef0 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 file; - uint64_t load_address; + JSONUINT64 load_address; llvm::Optional uuid; }; diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp index 590f38d..b841f25 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 c282da3..e7e35bb 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 a168bf0..913825c 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 val = value.getAsUINT64()) { + uint64.value = *val; + return true; + } else if (Optional 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 352ccf9..3dfcc88 100644 --- a/lldb/test/API/commands/trace/TestTraceLoad.py +++ b/lldb/test/API/commands/trace/TestTraceLoad.py @@ -21,6 +21,18 @@ class TestTraceLoad(TraceIntelPTTestCaseBase): 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 0000000..d7c1307 --- /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" +} -- 2.7.4