llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Pranjal Kumar Bajaria (PranjalKBajaria) <details> <summary>Changes</summary> Add support for the `reportsOriginalInstructions` GDB remote protocol feature. This allows debuggers to query whether the remote stub/target preserves the original instruction bytes when memory is read around a breakpoint site. Currently, when working with external GDB servers like openOCD LLDB leaves the substitution upto the servers even if the server doesn't do the substitution and we can see breakpoint trap codes in disassembly (https://github.com/llvm/llvm-project/issues/197564). --- Full diff: https://github.com/llvm/llvm-project/pull/201176.diff 9 Files Affected: - (modified) lldb/include/lldb/Target/Process.h (+6) - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+9) - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (+4-2) - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp (+1) - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+4) - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (+2) - (modified) lldb/source/Target/Process.cpp (+3-1) - (modified) lldb/tools/debugserver/source/RNBRemote.cpp (+1-1) - (modified) lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp (+24) ``````````diff diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index f68ea3b639e93..df78a22f25b2b 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -2332,6 +2332,12 @@ class Process : public std::enable_shared_from_this<Process>, Status ClearBreakpointSiteByID(lldb::user_id_t break_id); + /// Check if this process reports the original opcodes when memory is read + /// around a breakpoint site. Some debuggers/stubs can provide the original + /// instruction bytes that were replaced by the breakpoint trap. If false, + /// LLDB will attempt to perform the opcode substitution itself. + virtual bool ReportsOriginalOpcodes() const { return false; } + lldb::break_id_t CreateBreakpointSite(const lldb::BreakpointLocationSP &owner, bool use_hardware); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 8df7936786b04..5bf49efe47d44 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -444,6 +444,8 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() { m_supports_multi_mem_read = eLazyBoolYes; else if (x == "jMultiBreakpoint+") m_supports_multi_breakpoint = eLazyBoolYes; + else if (x == "reportsOriginalInstructions+") + m_supports_reportingOriginalInsts = eLazyBoolYes; // Look for a list of compressions in the features list e.g. // qXfer:features:read+;PacketSize=20000;qEcho+;SupportedCompressions=zlib- // deflate,lzma @@ -687,6 +689,13 @@ bool GDBRemoteCommunicationClient::GetMemoryTaggingSupported() { return m_supports_memory_tagging == eLazyBoolYes; } +bool GDBRemoteCommunicationClient::GetReportsOriginalInstructions() const { + if (m_supports_reportingOriginalInsts == eLazyBoolCalculate) { + GetRemoteQSupported(); + } + return m_supports_reportingOriginalInsts == eLazyBoolYes; +} + DataBufferSP GDBRemoteCommunicationClient::ReadMemoryTags(lldb::addr_t addr, size_t len, int32_t type) { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h index 79ca0bcd3ed22..ba859da578dfd 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -449,6 +449,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase { bool GetMemoryTaggingSupported(); + bool GetReportsOriginalInstructions() const; + bool UsesNativeSignals(); lldb::DataBufferSP ReadMemoryTags(lldb::addr_t addr, size_t len, @@ -582,6 +584,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase { LazyBool m_supports_reverse_step = eLazyBoolCalculate; LazyBool m_supports_multi_mem_read = eLazyBoolCalculate; LazyBool m_supports_multi_breakpoint = eLazyBoolCalculate; + LazyBool m_supports_reportingOriginalInsts = eLazyBoolNo; bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1, m_supports_qUserName : 1, m_supports_qGroupName : 1, @@ -591,8 +594,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase { m_supports_qSymbol : 1, m_qSymbol_requests_done : 1, m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1, m_supports_jModulesInfo : 1, m_supports_vFileSize : 1, - m_supports_vFileMode : 1, m_supports_vFileExists : 1, - m_supports_vRun : 1; + m_supports_vFileMode : 1, m_supports_vFileExists : 1, m_supports_vRun : 1; /// Current gdb remote protocol process identifier for all other operations lldb::pid_t m_curr_pid = LLDB_INVALID_PROCESS_ID; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp index 16ded2c657d54..9d57b525e1848 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -1386,5 +1386,6 @@ std::vector<std::string> GDBRemoteCommunicationServerCommon::HandleFeatures( "QStartNoAckMode+", "qEcho+", "native-signals+", + "reportsOriginalInstructions+", }; } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index f6eaf5851338b..e7f3e52aa14f3 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2794,6 +2794,10 @@ void ProcessGDBRemote::WillPublicStop() { } } +bool ProcessGDBRemote::ReportsOriginalOpcodes() const { + return m_gdb_comm.GetReportsOriginalInstructions(); +} + // Process Memory size_t ProcessGDBRemote::DoReadMemory(addr_t addr, void *buf, size_t size, Status &error) { diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 0b71b304885c2..d945303d2b89d 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -484,6 +484,8 @@ class ProcessGDBRemote : public Process, void HandleStopReply() override; void HandleAsyncStructuredDataPacket(llvm::StringRef data) override; + bool ReportsOriginalOpcodes() const override; + lldb::ThreadSP HandleThreadAsyncInterrupt(uint8_t signo, const std::string &description) override; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index e77cb0b0835e1..c2c7b4c9e467d 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -1820,7 +1820,9 @@ size_t Process::RemoveBreakpointOpcodesFromBuffer(addr_t bp_addr, size_t size, bp_sites_in_range)) { bp_sites_in_range.ForEach([bp_addr, size, buf](BreakpointSite *bp_site) -> void { - if (bp_site->GetType() == BreakpointSite::eSoftware) { + if (bp_site->GetType() == BreakpointSite::eSoftware || + (bp_site->GetType() == BreakpointSite::eExternal && + !ReportsOriginalOpcodes())) { addr_t intersect_addr; size_t intersect_size; size_t opcode_offset; diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index 1c7c3eb8e41d7..9934258c052eb 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -3703,7 +3703,7 @@ rnb_err_t RNBRemote::HandlePacket_qSupported(const char *p) { std::stringstream reply; reply << "qXfer:features:read+;PacketSize=" << std::hex << max_packet_size << ";"; - reply << "qEcho+;native-signals+;"; + reply << "qEcho+;native-signals+;reportsOriginalInstructions+;"; bool enable_compression = false; (void)enable_compression; diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp index 3ec212b1c205d..6136be2d873af 100644 --- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp +++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp @@ -751,3 +751,27 @@ TEST_F(GDBRemoteCommunicationClientTest, MultiBreakpointdNotSupported) { ASSERT_FALSE(client.GetMultiBreakpointSupported()); async_result.wait(); } + +TEST_F(GDBRemoteCommunicationClientTest, ReportsOriginalInstructions) { + std::future<void> async_result = + std::async(std::launch::async, [&] { client.GetRemoteQSupported(); }); + + HandlePacket( + server, testing::StartsWith("qSupported:"), + "reportsOriginalInstructions+;qXfer:memory-map:read+;PacketSize=4000"); + + async_result.get(); + EXPECT_TRUE(client.GetReportsOriginalInstructions()); +} + +TEST_F(GDBRemoteCommunicationClientTest, + ReportsOriginalInstructionsUnsupported) { + std::future<void> async_result = + std::async(std::launch::async, [&] { client.GetRemoteQSupported(); }); + + HandlePacket(server, testing::StartsWith("qSupported:"), + "qXfer:memory-map:read+;PacketSize=4000"); + + async_result.get(); + EXPECT_FALSE(client.GetReportsOriginalInstructions()); +} \ No newline at end of file `````````` </details> https://github.com/llvm/llvm-project/pull/201176 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
