[Lldb-commits] [lldb] 9d2dd6d - [NFC][lldb][trace] Use uint64_t when decoding and enconding json

Walter Erquinigo via lldb-commits lldb-commits at lists.llvm.org
Mon May 9 21:55:50 PDT 2022


Author: Walter Erquinigo
Date: 2022-05-09T21:55:43-07:00
New Revision: 9d2dd6d7622335ba9c19b55ac7d463cf662cab0d

URL: https://github.com/llvm/llvm-project/commit/9d2dd6d7622335ba9c19b55ac7d463cf662cab0d
DIFF: https://github.com/llvm/llvm-project/commit/9d2dd6d7622335ba9c19b55ac7d463cf662cab0d.diff

LOG: [NFC][lldb][trace] Use uint64_t when decoding and enconding json

llvm's json parser supports uint64_t, so let's better use it for the
packets being sent between lldb and lldb-server instead of using int64_t
as an intermediate type, which might be error-prone.

Added: 
    

Modified: 
    lldb/include/lldb/Utility/TraceGDBRemotePackets.h
    lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
    lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
    lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
    lldb/source/Target/Trace.cpp
    lldb/source/Utility/TraceGDBRemotePackets.cpp
    lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
index b2669ee3d813d..8e8919c4d8d87 100644
--- a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
@@ -45,7 +45,7 @@ struct TraceStartRequest {
 
   /// If \a llvm::None, then this starts tracing the whole process. Otherwise,
   /// only tracing for the specified threads is enabled.
-  llvm::Optional<std::vector<int64_t>> tids;
+  llvm::Optional<std::vector<lldb::tid_t>> tids;
 
   /// \return
   ///     \b true if \a tids is \a None, i.e. whole process tracing.
@@ -73,7 +73,7 @@ struct TraceStopRequest {
   std::string type;
   /// If \a llvm::None, then this stops tracing the whole process. Otherwise,
   /// only tracing for the specified threads is stopped.
-  llvm::Optional<std::vector<int64_t>> tids;
+  llvm::Optional<std::vector<lldb::tid_t>> tids;
 };
 
 bool fromJSON(const llvm::json::Value &value, TraceStopRequest &packet,
@@ -98,7 +98,7 @@ struct TraceBinaryData {
   /// Identifier of data to fetch with jLLDBTraceGetBinaryData.
   std::string kind;
   /// Size in bytes for this data.
-  int64_t size;
+  uint64_t size;
 };
 
 bool fromJSON(const llvm::json::Value &value, TraceBinaryData &packet,
@@ -107,7 +107,7 @@ bool fromJSON(const llvm::json::Value &value, TraceBinaryData &packet,
 llvm::json::Value toJSON(const TraceBinaryData &packet);
 
 struct TraceThreadState {
-  int64_t tid;
+  lldb::tid_t tid;
   /// List of binary data objects for this thread.
   std::vector<TraceBinaryData> binaryData;
 };
@@ -161,11 +161,11 @@ struct TraceGetBinaryDataRequest {
   /// Identifier for the data.
   std::string kind;
   /// Optional tid if the data is related to a thread.
-  llvm::Optional<int64_t> tid;
+  llvm::Optional<lldb::tid_t> tid;
   /// Offset in bytes from where to start reading the data.
-  int64_t offset;
+  uint64_t offset;
   /// Number of bytes to read.
-  int64_t size;
+  uint64_t size;
 };
 
 bool fromJSON(const llvm::json::Value &value,

diff  --git a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
index ddb758fe95e98..cb79fb351a438 100644
--- a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
@@ -16,6 +16,9 @@
 #include <chrono>
 
 /// See docs/lldb-gdb-remote.txt for more information.
+///
+/// Do not use system-dependent types, like size_t, because they might cause
+/// issues when compiling on arm.
 namespace lldb_private {
 
 // List of data kinds used by jLLDBGetState and jLLDBGetBinaryData.
@@ -28,20 +31,20 @@ struct IntelPTDataKinds {
 /// \{
 struct TraceIntelPTStartRequest : TraceStartRequest {
   /// Size in bytes to use for each thread's trace buffer.
-  int64_t trace_buffer_size;
+  uint64_t trace_buffer_size;
 
   /// Whether to enable TSC
   bool enable_tsc;
 
   /// PSB packet period
-  llvm::Optional<int64_t> psb_period;
+  llvm::Optional<uint64_t> psb_period;
 
   /// Required when doing "process tracing".
   ///
   /// Limit in bytes on all the thread traces started by this "process trace"
   /// instance. When a thread is about to be traced and the limit would be hit,
   /// then a "tracing" stop event is triggered.
-  llvm::Optional<int64_t> process_buffer_size_limit;
+  llvm::Optional<uint64_t> process_buffer_size_limit;
 
   /// Whether to have a trace buffer per thread or per cpu core.
   llvm::Optional<bool> per_core_tracing;

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
index bf0b77b39f9b8..a1544680043f5 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -72,10 +72,9 @@ std::vector<TraceThreadState>
 IntelPTThreadTraceCollection::GetThreadStates() const {
   std::vector<TraceThreadState> states;
   for (const auto &it : m_thread_traces)
-    states.push_back({static_cast<int64_t>(it.first),
+    states.push_back({it.first,
                       {TraceBinaryData{IntelPTDataKinds::kTraceBuffer,
-                                       static_cast<int64_t>(
-                                           it.second->GetTraceBufferSize())}}});
+                                       it.second->GetTraceBufferSize()}}});
   return states;
 }
 
@@ -201,8 +200,8 @@ Expected<json::Value> IntelPTCollector::GetState() const {
     return cpu_info.takeError();
 
   TraceGetStateResponse state;
-  state.processBinaryData.push_back({IntelPTDataKinds::kProcFsCpuInfo,
-                                     static_cast<int64_t>(cpu_info->size())});
+  state.processBinaryData.push_back(
+      {IntelPTDataKinds::kProcFsCpuInfo, cpu_info->size()});
 
   std::vector<TraceThreadState> thread_states =
       m_thread_traces.GetThreadStates();

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
index c9e77dde2282d..9c1412c16acd9 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -305,7 +305,8 @@ Error TraceIntelPT::Start(size_t trace_buffer_size,
   request.trace_buffer_size = trace_buffer_size;
   request.process_buffer_size_limit = total_buffer_size_limit;
   request.enable_tsc = enable_tsc;
-  request.psb_period = psb_period.map([](size_t val) { return (int64_t)val; });
+  request.psb_period =
+      psb_period.map([](size_t val) { return static_cast<uint64_t>(val); });
   request.type = GetPluginName().str();
   request.per_core_tracing = per_core_tracing;
   return Trace::Start(toJSON(request));
@@ -342,7 +343,8 @@ llvm::Error TraceIntelPT::Start(llvm::ArrayRef<lldb::tid_t> tids,
   TraceIntelPTStartRequest request;
   request.trace_buffer_size = trace_buffer_size;
   request.enable_tsc = enable_tsc;
-  request.psb_period = psb_period.map([](size_t val) { return (int64_t)val; });
+  request.psb_period =
+      psb_period.map([](size_t val) { return static_cast<uint64_t>(val); });
   request.type = GetPluginName().str();
   request.tids.emplace();
   for (lldb::tid_t tid : tids)

diff  --git a/lldb/source/Target/Trace.cpp b/lldb/source/Target/Trace.cpp
index b33d5afb3b34c..fba1b94a03014 100644
--- a/lldb/source/Target/Trace.cpp
+++ b/lldb/source/Target/Trace.cpp
@@ -154,9 +154,8 @@ Trace::GetLiveThreadBinaryData(lldb::tid_t tid, llvm::StringRef kind) {
         "Tracing data \"%s\" is not available for thread %" PRIu64 ".",
         kind.data(), tid);
 
-  TraceGetBinaryDataRequest request{GetPluginName().str(), kind.str(),
-                                    static_cast<int64_t>(tid), 0,
-                                    static_cast<int64_t>(*size)};
+  TraceGetBinaryDataRequest request{GetPluginName().str(), kind.str(), tid, 0,
+                                    *size};
   return m_live_process->TraceGetBinaryData(request);
 }
 
@@ -172,7 +171,7 @@ Trace::GetLiveProcessBinaryData(llvm::StringRef kind) {
         "Tracing data \"%s\" is not available for the process.", kind.data());
 
   TraceGetBinaryDataRequest request{GetPluginName().str(), kind.str(), None, 0,
-                                    static_cast<int64_t>(*size)};
+                                    *size};
   return m_live_process->TraceGetBinaryData(request);
 }
 

diff  --git a/lldb/source/Utility/TraceGDBRemotePackets.cpp b/lldb/source/Utility/TraceGDBRemotePackets.cpp
index 5c1326a5f353a..b8ac71be51357 100644
--- a/lldb/source/Utility/TraceGDBRemotePackets.cpp
+++ b/lldb/source/Utility/TraceGDBRemotePackets.cpp
@@ -48,7 +48,7 @@ TraceStopRequest::TraceStopRequest(llvm::StringRef type,
     : type(type) {
   tids.emplace();
   for (lldb::tid_t tid : tids_)
-    tids->push_back(static_cast<int64_t>(tid));
+    tids->push_back(tid);
 }
 
 bool TraceStopRequest::IsProcessTracing() const { return !(bool)tids; }

diff  --git a/lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp b/lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
index afc7cdf85509c..27f1d4c6229b7 100644
--- a/lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ b/lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -56,9 +56,9 @@ LinuxPerfZeroTscConversion::Convert(uint64_t raw_counter_value) {
 json::Value LinuxPerfZeroTscConversion::toJSON() {
   return json::Value(json::Object{
       {"kind", "tsc-perf-zero-conversion"},
-      {"time_mult", static_cast<int64_t>(m_time_mult)},
-      {"time_shift", static_cast<int64_t>(m_time_shift)},
-      {"time_zero", static_cast<int64_t>(m_time_zero)},
+      {"time_mult", m_time_mult},
+      {"time_shift", m_time_shift},
+      {"time_zero", m_time_zero},
   });
 }
 
@@ -66,14 +66,14 @@ bool fromJSON(const json::Value &value, TraceTscConversionUP &tsc_conversion,
               json::Path path) {
   ObjectMapper o(value, path);
 
-  int64_t time_mult, time_shift, time_zero;
+  uint64_t time_mult, time_shift, time_zero;
   if (!o || !o.map("time_mult", time_mult) ||
       !o.map("time_shift", time_shift) || !o.map("time_zero", time_zero))
     return false;
 
   tsc_conversion = std::make_unique<LinuxPerfZeroTscConversion>(
       static_cast<uint32_t>(time_mult), static_cast<uint16_t>(time_shift),
-      static_cast<uint64_t>(time_zero));
+      time_zero);
 
   return true;
 }


        


More information about the lldb-commits mailing list