wallace updated this revision to Diff 302439.
wallace added a comment.
After a sync up with Greg, I've followed his recommedations and did the
following changes:
- Kept the packet as a simple packet returning one single trace type. However,
now the response is a json object {"pluginName": <name>, "description":
<text>}. This makes it very explicit that the name returned should match a
Trace plug-in name, making future implementations simpler.
We analyzed the existing jTrace packets, which were done several years ago by
Intel and we realized that they are not generic enough for the way we are
implementing Trace plugin. Those packets are too intel-pt specific and we'll be
patching them to make them more generic. For example, the "metabuffersize" is
something specific to intel pt.
We also don't like the fact that it's using a public enum TraceType. We are now
trying to use instead a string name, which will be more user-friendly and will
help connect different gdb-servers and Trace plug-ins.
- Removed the changes to the TraceType enum and the old intel pt plugin.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90490/new/
https://reviews.llvm.org/D90490
Files:
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Host/common/NativeProcessProtocol.h
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
lldb/source/Plugins/Process/Linux/ProcessorTrace.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Target/Trace.cpp
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===================================================================
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -362,6 +362,45 @@
EXPECT_FALSE(result.get().Success());
}
+TEST_F(GDBRemoteCommunicationClientTest, SendTraceSupportedTypePacket) {
+ // Success response
+ {
+ std::future<llvm::Expected<TraceTypeInfo>> result = std::async(
+ std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+ HandlePacket(
+ server, "jTraceSupportedType",
+ R"({"pluginName":"intel-pt","description":"Intel Processor Trace"}])");
+
+ llvm::Expected<TraceTypeInfo> trace_type_or_err = result.get();
+ EXPECT_THAT_EXPECTED(trace_type_or_err, llvm::Succeeded());
+ ASSERT_STREQ(trace_type_or_err->plugin_name.c_str(), "intel-pt");
+ ASSERT_STREQ(trace_type_or_err->description.c_str(),
+ "Intel Processor Trace");
+ }
+
+ // Error response - wrong json
+ {
+ std::future<llvm::Expected<TraceTypeInfo>> result = std::async(
+ std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+ HandlePacket(server, "jTraceSupportedType", R"({"type":"intel-pt"}])");
+
+ llvm::Expected<TraceTypeInfo> trace_type_or_err = result.get();
+ EXPECT_THAT_EXPECTED(trace_type_or_err, llvm::Failed());
+ }
+
+ // Error response
+ {
+ std::future<llvm::Expected<TraceTypeInfo>> result = std::async(
+ std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+ HandlePacket(server, "jTraceSupportedType", "E23");
+ llvm::Expected<TraceTypeInfo> trace_type_or_err = result.get();
+ ASSERT_THAT_EXPECTED(trace_type_or_err, llvm::Failed());
+ }
+}
+
TEST_F(GDBRemoteCommunicationClientTest, SendStartTracePacket) {
TraceOptions options;
Status error;
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===================================================================
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -310,6 +310,8 @@
return eServerPacketType_jTraceStart;
if (PACKET_STARTS_WITH("jTraceStop:"))
return eServerPacketType_jTraceStop;
+ if (PACKET_MATCHES("jTraceSupportedType"))
+ return eServerPacketType_jTraceSupportedType;
break;
case 'v':
Index: lldb/source/Target/Trace.cpp
===================================================================
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -266,3 +266,18 @@
return index < end_position;
});
}
+
+namespace llvm {
+namespace json {
+
+bool fromJSON(const Value &value, lldb_private::TraceTypeInfo &info,
+ Path path) {
+ ObjectMapper o(value, path);
+ if (!o)
+ return false;
+ o.map("description", info.description);
+ return o.map("pluginName", info.plugin_name);
+}
+
+} // namespace json
+} // namespace llvm
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -175,6 +175,8 @@
llvm::MutableArrayRef<uint8_t> &buffer,
size_t offset = 0) override;
+ llvm::Expected<TraceTypeInfo> GetSupportedTraceType() override;
+
Status GetTraceConfig(lldb::user_id_t uid, TraceOptions &options) override;
Status GetWatchpointSupportInfo(uint32_t &num, bool &after) override;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1224,6 +1224,10 @@
return m_gdb_comm.SendGetTraceConfigPacket(uid, options);
}
+llvm::Expected<TraceTypeInfo> ProcessGDBRemote::GetSupportedTraceType() {
+ return m_gdb_comm.SendGetSupportedTraceType();
+}
+
void ProcessGDBRemote::DidExit() {
// When we exit, disconnect from the GDB server communications
m_gdb_comm.Disconnect();
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -164,6 +164,8 @@
PacketResult Handle_jTraceConfigRead(StringExtractorGDBRemote &packet);
+ PacketResult Handle_jTraceSupportedType(StringExtractorGDBRemote &packet);
+
PacketResult Handle_QRestoreRegisterState(StringExtractorGDBRemote &packet);
PacketResult Handle_vAttach(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -191,6 +191,9 @@
RegisterMemberFunctionHandler(
StringExtractorGDBRemote::eServerPacketType_jTraceConfigRead,
&GDBRemoteCommunicationServerLLGS::Handle_jTraceConfigRead);
+ RegisterMemberFunctionHandler(
+ StringExtractorGDBRemote::eServerPacketType_jTraceSupportedType,
+ &GDBRemoteCommunicationServerLLGS::Handle_jTraceSupportedType);
RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_g,
&GDBRemoteCommunicationServerLLGS::Handle_g);
@@ -1226,6 +1229,33 @@
return SendOKResponse();
}
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_jTraceSupportedType(
+ StringExtractorGDBRemote &packet) {
+
+ // Fail if we don't have a current process.
+ if (!m_debugged_process_up ||
+ (m_debugged_process_up->GetID() == LLDB_INVALID_PROCESS_ID))
+ return SendErrorResponse(68);
+
+ llvm::Expected<TraceTypeInfo> supported_trace_type =
+ m_debugged_process_up->GetSupportedTraceType();
+ if (!supported_trace_type)
+ return SendErrorResponse(Status(supported_trace_type.takeError()));
+
+ StreamGDBRemote escaped_response;
+ StructuredData::Dictionary json_packet;
+
+ json_packet.AddStringItem("pluginName", supported_trace_type->plugin_name);
+ json_packet.AddStringItem("description", supported_trace_type->description);
+
+ StreamString json_string;
+ json_packet.Dump(json_string, false);
+ escaped_response.PutEscapedBytes(json_string.GetData(),
+ json_string.GetSize());
+ return SendPacketNoLock(escaped_response.GetString());
+}
+
GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerLLGS::Handle_jTraceConfigRead(
StringExtractorGDBRemote &packet) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -18,6 +18,7 @@
#include <vector>
#include "lldb/Host/File.h"
+#include "lldb/Target/Trace.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/GDBRemote.h"
#include "lldb/Utility/ProcessInfo.h"
@@ -519,6 +520,8 @@
Status SendGetTraceConfigPacket(lldb::user_id_t uid, TraceOptions &options);
+ llvm::Expected<TraceTypeInfo> SendGetSupportedTraceType();
+
protected:
LazyBool m_supports_not_sending_acks;
LazyBool m_supports_thread_suffix;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -3454,6 +3454,31 @@
return SendGetTraceDataPacket(escaped_packet, uid, thread_id, buffer, offset);
}
+llvm::Expected<TraceTypeInfo>
+GDBRemoteCommunicationClient::SendGetSupportedTraceType() {
+ Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+
+ StreamGDBRemote escaped_packet;
+ escaped_packet.PutCString("jTraceSupportedType");
+
+ StringExtractorGDBRemote response;
+ if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response,
+ true) ==
+ GDBRemoteCommunication::PacketResult::Success) {
+ if (response.IsNormalResponse()) {
+ if (Expected<TraceTypeInfo> type =
+ llvm::json::parse<TraceTypeInfo>(response.Peek()))
+ return *type;
+ else
+ return type.takeError();
+ } else
+ return response.GetStatus().ToError();
+ }
+ LLDB_LOG(log, "failed to send packet: jTraceSupportedType");
+ return createStringError(inconvertibleErrorCode(),
+ "failed to send packet: jTraceSupportedType");
+}
+
Status
GDBRemoteCommunicationClient::SendGetTraceConfigPacket(lldb::user_id_t uid,
TraceOptions &options) {
Index: lldb/source/Plugins/Process/Linux/ProcessorTrace.h
===================================================================
--- lldb/source/Plugins/Process/Linux/ProcessorTrace.h
+++ lldb/source/Plugins/Process/Linux/ProcessorTrace.h
@@ -93,6 +93,10 @@
void SetThreadID(lldb::tid_t tid) { m_thread_id = tid; }
public:
+ static llvm::Expected<uint32_t> GetOSEventType();
+
+ static bool IsSupported();
+
static Status GetCPUType(TraceOptions &config);
static llvm::Expected<ProcessorTraceMonitorUP>
Index: lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
+++ lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
@@ -26,6 +26,8 @@
using namespace llvm;
lldb::user_id_t ProcessorTraceMonitor::m_trace_num = 1;
+const char *kOSEventIntelPTTypeFile =
+ "/sys/bus/event_source/devices/intel_pt/type";
Status ProcessorTraceMonitor::GetTraceConfig(TraceOptions &config) const {
#ifndef PERF_ATTR_SIZE_VER5
@@ -44,6 +46,27 @@
#endif
}
+Expected<uint32_t> ProcessorTraceMonitor::GetOSEventType() {
+ auto intel_pt_type_text =
+ llvm::MemoryBuffer::getFileAsStream(kOSEventIntelPTTypeFile);
+
+ if (!intel_pt_type_text)
+ return createStringError(inconvertibleErrorCode(),
+ "Can't open the file '%s'",
+ kOSEventIntelPTTypeFile);
+
+ uint32_t intel_pt_type = 0;
+ StringRef buffer = intel_pt_type_text.get()->getBuffer();
+ if (buffer.empty() || buffer.trim().getAsInteger(10, intel_pt_type))
+ return createStringError(
+ inconvertibleErrorCode(),
+ "The file '%s' has a invalid value. It should be an unsigned int.",
+ kOSEventIntelPTTypeFile);
+ return intel_pt_type;
+}
+
+bool ProcessorTraceMonitor::IsSupported() { return (bool)GetOSEventType(); }
+
Status ProcessorTraceMonitor::StartTrace(lldb::pid_t pid, lldb::tid_t tid,
const TraceOptions &config) {
#ifndef PERF_ATTR_SIZE_VER5
@@ -76,25 +99,15 @@
attr.exclude_idle = 1;
attr.mmap = 1;
- int intel_pt_type = 0;
-
- auto ret = llvm::MemoryBuffer::getFileAsStream(
- "/sys/bus/event_source/devices/intel_pt/type");
- if (!ret) {
- LLDB_LOG(log, "failed to open Config file");
- return ret.getError();
- }
+ Expected<uint32_t> intel_pt_type = GetOSEventType();
- StringRef rest = ret.get()->getBuffer();
- if (rest.empty() || rest.trim().getAsInteger(10, intel_pt_type)) {
- LLDB_LOG(log, "failed to read Config file");
- error.SetErrorString("invalid file");
+ if (!intel_pt_type) {
+ error = intel_pt_type.takeError();
return error;
}
- rest.trim().getAsInteger(10, intel_pt_type);
- LLDB_LOG(log, "intel pt type {0}", intel_pt_type);
- attr.type = intel_pt_type;
+ LLDB_LOG(log, "intel pt type {0}", *intel_pt_type);
+ attr.type = *intel_pt_type;
LLDB_LOG(log, "meta buffer size {0}", metabufsize);
LLDB_LOG(log, "buffer size {0} ", bufsize);
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -117,6 +117,8 @@
Status GetTraceConfig(lldb::user_id_t traceid, TraceOptions &config) override;
+ virtual llvm::Expected<TraceTypeInfo> GetSupportedTraceType() override;
+
// Interface used by NativeRegisterContext-derived classes.
static Status PtraceWrapper(int req, lldb::pid_t pid, void *addr = nullptr,
void *data = nullptr, size_t data_size = 0,
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1995,6 +1995,12 @@
return error;
}
+llvm::Expected<TraceTypeInfo> NativeProcessLinux::GetSupportedTraceType() {
+ if (ProcessorTraceMonitor::IsSupported())
+ return TraceTypeInfo{"intel-pt", "Intel Processor Trace"};
+ return NativeProcessProtocol::GetSupportedTraceType();
+}
+
lldb::user_id_t
NativeProcessLinux::StartTraceGroup(const TraceOptions &config,
Status &error) {
Index: lldb/include/lldb/lldb-enumerations.h
===================================================================
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -772,7 +772,7 @@
enum TraceType {
eTraceTypeNone = 0,
- // Hardware Trace generated by the processor.
+ /// Intel Processor Trace
eTraceTypeProcessorTrace
};
Index: lldb/include/lldb/Utility/StringExtractorGDBRemote.h
===================================================================
--- lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -167,6 +167,7 @@
eServerPacketType_jTraceMetaRead,
eServerPacketType_jTraceStop,
eServerPacketType_jTraceConfigRead,
+ eServerPacketType_jTraceSupportedType,
};
ServerPacketType GetServerPacketType() const;
Index: lldb/include/lldb/Target/Trace.h
===================================================================
--- lldb/include/lldb/Target/Trace.h
+++ lldb/include/lldb/Target/Trace.h
@@ -181,6 +181,21 @@
virtual size_t GetInstructionCount(const Thread &thread) = 0;
};
+/// This struct represents a return value from the jTraceSupportedType packet in
+/// lldb-gdb-remote.txt
+struct TraceTypeInfo {
+ std::string plugin_name;
+ std::string description;
+};
+
} // namespace lldb_private
+namespace llvm {
+namespace json {
+
+bool fromJSON(const Value &value, lldb_private::TraceTypeInfo &info, Path path);
+
+} // namespace json
+} // namespace llvm
+
#endif // LLDB_TARGET_TRACE_H
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -38,6 +38,7 @@
#include "lldb/Target/QueueList.h"
#include "lldb/Target/ThreadList.h"
#include "lldb/Target/ThreadPlanStack.h"
+#include "lldb/Target/Trace.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Broadcaster.h"
#include "lldb/Utility/Event.h"
@@ -2542,6 +2543,17 @@
return Status("Not implemented");
}
+ /// See the information on the jTraceSupportedType packet in
+ /// lldb-gdb-remote.txt.
+ ///
+ /// \return
+ /// The supported trace type or an \a llvm::Error if tracing is
+ /// not supported for the inferior.
+ virtual llvm::Expected<TraceTypeInfo> GetSupportedTraceType() {
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Tracing is not supported");
+ }
+
// This calls a function of the form "void * (*)(void)".
bool CallVoidArgVoidPtrReturn(const Address *address,
lldb::addr_t &returned_func,
Index: lldb/include/lldb/Host/common/NativeProcessProtocol.h
===================================================================
--- lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -14,6 +14,7 @@
#include "NativeWatchpointList.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/MainLoop.h"
+#include "lldb/Target/Trace.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/TraceOptions.h"
@@ -392,6 +393,12 @@
return Status("Not implemented");
}
+ /// \copydoc Process::GetSupportedTraceType()
+ virtual llvm::Expected<TraceTypeInfo> GetSupportedTraceType() {
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Tracing is not supported");
+ }
+
protected:
struct SoftwareBreakpoint {
uint32_t ref_count;
Index: lldb/docs/lldb-gdb-remote.txt
===================================================================
--- lldb/docs/lldb-gdb-remote.txt
+++ lldb/docs/lldb-gdb-remote.txt
@@ -234,6 +234,28 @@
send packet: QListThreadsInStopReply
read packet: OK
+//----------------------------------------------------------------------
+// jTraceSupportedType
+//
+// BRIEF
+// Get the preferred processor tracing type supported by the gdb-server for
+// the current inferior. Responses might be different depending on the
+// architecture and capabilities of the underlying OS.
+//
+// The return packet is a JSON object with the following schema
+//
+// {
+// "pluginName": <lldb trace plug-in name, e.g. intel-pt>
+// "description": <optional description string for this technology>
+// }
+//
+// If no tracing technology is supported for the inferior, or no process is
+// running, then an error should be returned.
+//----------------------------------------------------------------------
+
+send packet: jTraceSupportedType
+read packet: {"pluginName": <name>, "description", <description>}/E<error code>
+
//----------------------------------------------------------------------
// jTraceStart:
//
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
