This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9f45f23d8602: [trace][intelpt] Support system-wide tracing 
[21] - Support long numbers in JSON (authored by Walter Erquinigo 
<wall...@fb.com>).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127819/new/

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

Reply via email to