From 9d2dd6d7622335ba9c19b55ac7d463cf662cab0d Mon Sep 17 00:00:00 2001 From: Walter Erquinigo Date: Mon, 9 May 2022 21:44:09 -0700 Subject: [PATCH] [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. --- lldb/include/lldb/Utility/TraceGDBRemotePackets.h | 14 +++++++------- .../lldb/Utility/TraceIntelPTGDBRemotePackets.h | 9 ++++++--- .../Plugins/Process/Linux/IntelPTCollector.cpp | 9 ++++----- .../source/Plugins/Trace/intel-pt/TraceIntelPT.cpp | 6 ++++-- lldb/source/Target/Trace.cpp | 7 +++---- lldb/source/Utility/TraceGDBRemotePackets.cpp | 2 +- .../Utility/TraceIntelPTGDBRemotePackets.cpp | 10 +++++----- 7 files changed, 30 insertions(+), 27 deletions(-) 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> tids; + llvm::Optional> 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> tids; + llvm::Optional> 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 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 tid; + llvm::Optional 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 /// 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 psb_period; + llvm::Optional 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 process_buffer_size_limit; + llvm::Optional process_buffer_size_limit; /// Whether to have a trace buffer per thread or per cpu core. llvm::Optional 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 IntelPTThreadTraceCollection::GetThreadStates() const { std::vector states; for (const auto &it : m_thread_traces) - states.push_back({static_cast(it.first), + states.push_back({it.first, {TraceBinaryData{IntelPTDataKinds::kTraceBuffer, - static_cast( - it.second->GetTraceBufferSize())}}}); + it.second->GetTraceBufferSize()}}}); return states; } @@ -201,8 +200,8 @@ Expected IntelPTCollector::GetState() const { return cpu_info.takeError(); TraceGetStateResponse state; - state.processBinaryData.push_back({IntelPTDataKinds::kProcFsCpuInfo, - static_cast(cpu_info->size())}); + state.processBinaryData.push_back( + {IntelPTDataKinds::kProcFsCpuInfo, cpu_info->size()}); std::vector 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(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 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(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(tid), 0, - static_cast(*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(*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(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(m_time_mult)}, - {"time_shift", static_cast(m_time_shift)}, - {"time_zero", static_cast(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( static_cast(time_mult), static_cast(time_shift), - static_cast(time_zero)); + time_zero); return true; }