diff options
author | Adrian Prantl <aprantl@apple.com> | 2024-08-27 10:59:31 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-08-27 10:59:31 -0700 |
commit | 0642cd768b80665585c8500bed2933a3b99123dc (patch) | |
tree | a412a5eafff54ef9a7cb884e01907a4f521f5140 /lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | |
parent | acb33a0c9bc902dc1aef703c02b8fd3a1132cb14 (diff) | |
download | llvm-0642cd768b80665585c8500bed2933a3b99123dc.zip llvm-0642cd768b80665585c8500bed2933a3b99123dc.tar.gz llvm-0642cd768b80665585c8500bed2933a3b99123dc.tar.bz2 |
[lldb] Turn lldb_private::Status into a value type. (#106163)
This patch removes all of the Set.* methods from Status.
This cleanup is part of a series of patches that make it harder use the
anti-pattern of keeping a long-lives Status object around and updating
it while dropping any errors it contains on the floor.
This patch is largely NFC, the more interesting next steps this enables
is to:
1. remove Status.Clear()
2. assert that Status::operator=() never overwrites an error
3. remove Status::operator=()
Note that step (2) will bring 90% of the benefits for users, and step
(3) will dramatically clean up the error handling code in various
places. In the end my goal is to convert all APIs that are of the form
` ResultTy DoFoo(Status& error)
`
to
` llvm::Expected<ResultTy> DoFoo()
`
How to read this patch?
The interesting changes are in Status.h and Status.cpp, all other
changes are mostly
` perl -pi -e 's/\.SetErrorString/ = Status::FromErrorString/g' $(git
grep -l SetErrorString lldb/source)
`
plus the occasional manual cleanup.
Diffstat (limited to 'lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp')
-rw-r--r-- | lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | 149 |
1 files changed, 82 insertions, 67 deletions
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 0efe8fb..65b1b3a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -565,15 +565,15 @@ Status ProcessGDBRemote::DoConnectRemote(llvm::StringRef remote_url) { if (state != eStateInvalid) { SetPrivateState(state); } else - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "Process %" PRIu64 " was reported after connecting to " "'%s', but state was not stopped: %s", pid, remote_url.str().c_str(), StateAsCString(state)); } else - error.SetErrorStringWithFormat("Process %" PRIu64 - " was reported after connecting to '%s', " - "but no stop reply packet was received", - pid, remote_url.str().c_str()); + error = Status::FromErrorStringWithFormat( + "Process %" PRIu64 " was reported after connecting to '%s', " + "but no stop reply packet was received", + pid, remote_url.str().c_str()); } LLDB_LOGF(log, @@ -761,9 +761,9 @@ Status ProcessGDBRemote::DoLaunch(lldb_private::Module *exe_module, if (FileSpec exe_file = launch_info.GetExecutableFile()) args.ReplaceArgumentAtIndex(0, exe_file.GetPath(false)); if (llvm::Error err = m_gdb_comm.LaunchProcess(args)) { - error.SetErrorStringWithFormatv("Cannot launch '{0}': {1}", - args.GetArgumentAtIndex(0), - llvm::fmt_consume(std::move(err))); + error = Status::FromErrorStringWithFormatv( + "Cannot launch '{0}': {1}", args.GetArgumentAtIndex(0), + llvm::fmt_consume(std::move(err))); } else { SetID(m_gdb_comm.GetCurrentProcessID()); } @@ -834,7 +834,7 @@ Status ProcessGDBRemote::ConnectToDebugserver(llvm::StringRef connect_url) { if (!m_gdb_comm.IsConnected()) { if (error.Success()) - error.SetErrorString("not connected to remote gdb server"); + error = Status::FromErrorString("not connected to remote gdb server"); return error; } @@ -845,7 +845,7 @@ Status ProcessGDBRemote::ConnectToDebugserver(llvm::StringRef connect_url) { if (!m_gdb_comm.HandshakeWithServer(&error)) { m_gdb_comm.Disconnect(); if (error.Success()) - error.SetErrorString("not connected to remote gdb server"); + error = Status::FromErrorString("not connected to remote gdb server"); return error; } @@ -1364,11 +1364,13 @@ Status ProcessGDBRemote::DoResume() { } if (continue_packet_error) { - error.SetErrorString("can't make continue packet for this resume"); + error = + Status::FromErrorString("can't make continue packet for this resume"); } else { EventSP event_sp; if (!m_async_thread.IsJoinable()) { - error.SetErrorString("Trying to resume but the async thread is dead."); + error = Status::FromErrorString( + "Trying to resume but the async thread is dead."); LLDB_LOGF(log, "ProcessGDBRemote::DoResume: Trying to resume but the " "async thread is dead."); return error; @@ -1379,11 +1381,12 @@ Status ProcessGDBRemote::DoResume() { m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue, data_sp); if (!listener_sp->GetEvent(event_sp, std::chrono::seconds(5))) { - error.SetErrorString("Resume timed out."); + error = Status::FromErrorString("Resume timed out."); LLDB_LOGF(log, "ProcessGDBRemote::DoResume: Resume timed out."); } else if (event_sp->BroadcasterIs(&m_async_broadcaster)) { - error.SetErrorString("Broadcast continue, but the async thread was " - "killed before we got an ack back."); + error = Status::FromErrorString( + "Broadcast continue, but the async thread was " + "killed before we got an ack back."); LLDB_LOGF(log, "ProcessGDBRemote::DoResume: Broadcast continue, but the " "async thread was killed before we got an ack back."); @@ -2648,16 +2651,18 @@ size_t ProcessGDBRemote::DoReadMemory(addr_t addr, void *buf, size_t size, llvm::MutableArrayRef<uint8_t>((uint8_t *)buf, size), '\xdd'); } } else if (response.IsErrorResponse()) - error.SetErrorStringWithFormat("memory read failed for 0x%" PRIx64, addr); + error = Status::FromErrorStringWithFormat( + "memory read failed for 0x%" PRIx64, addr); else if (response.IsUnsupportedResponse()) - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "GDB server does not support reading memory"); else - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "unexpected response to GDB server memory read packet '%s': '%s'", packet, response.GetStringRef().data()); } else { - error.SetErrorStringWithFormat("failed to send packet: '%s'", packet); + error = Status::FromErrorStringWithFormat("failed to send packet: '%s'", + packet); } return 0; } @@ -2738,13 +2743,15 @@ Status ProcessGDBRemote::FlashErase(lldb::addr_t addr, size_t size) { // about only one region's block size. DoMemoryWrite is this function's // primary user, and it can easily keep writes within a single memory region if (addr + size > region.GetRange().GetRangeEnd()) { - status.SetErrorString("Unable to erase flash in multiple regions"); + status = + Status::FromErrorString("Unable to erase flash in multiple regions"); return status; } uint64_t blocksize = region.GetBlocksize(); if (blocksize == 0) { - status.SetErrorString("Unable to erase flash because blocksize is 0"); + status = + Status::FromErrorString("Unable to erase flash because blocksize is 0"); return status; } @@ -2789,18 +2796,19 @@ Status ProcessGDBRemote::FlashErase(lldb::addr_t addr, size_t size) { m_erased_flash_ranges.Insert(range, true); } else { if (response.IsErrorResponse()) - status.SetErrorStringWithFormat("flash erase failed for 0x%" PRIx64, - addr); + status = Status::FromErrorStringWithFormat( + "flash erase failed for 0x%" PRIx64, addr); else if (response.IsUnsupportedResponse()) - status.SetErrorStringWithFormat("GDB server does not support flashing"); + status = Status::FromErrorStringWithFormat( + "GDB server does not support flashing"); else - status.SetErrorStringWithFormat( + status = Status::FromErrorStringWithFormat( "unexpected response to GDB server flash erase packet '%s': '%s'", packet.GetData(), response.GetStringRef().data()); } } else { - status.SetErrorStringWithFormat("failed to send packet: '%s'", - packet.GetData()); + status = Status::FromErrorStringWithFormat("failed to send packet: '%s'", + packet.GetData()); } return status; } @@ -2819,16 +2827,18 @@ Status ProcessGDBRemote::FlashDone() { m_erased_flash_ranges.Clear(); } else { if (response.IsErrorResponse()) - status.SetErrorStringWithFormat("flash done failed"); + status = Status::FromErrorStringWithFormat("flash done failed"); else if (response.IsUnsupportedResponse()) - status.SetErrorStringWithFormat("GDB server does not support flashing"); + status = Status::FromErrorStringWithFormat( + "GDB server does not support flashing"); else - status.SetErrorStringWithFormat( + status = Status::FromErrorStringWithFormat( "unexpected response to GDB server flash done packet: '%s'", response.GetStringRef().data()); } } else { - status.SetErrorStringWithFormat("failed to send flash done packet"); + status = + Status::FromErrorStringWithFormat("failed to send flash done packet"); } return status; } @@ -2855,7 +2865,7 @@ size_t ProcessGDBRemote::DoWriteMemory(addr_t addr, const void *buf, if (is_flash) { if (!m_allow_flash_writes) { - error.SetErrorString("Writing to flash memory is not allowed"); + error = Status::FromErrorString("Writing to flash memory is not allowed"); return 0; } // Keep the write within a flash memory region @@ -2880,18 +2890,18 @@ size_t ProcessGDBRemote::DoWriteMemory(addr_t addr, const void *buf, error.Clear(); return size; } else if (response.IsErrorResponse()) - error.SetErrorStringWithFormat("memory write failed for 0x%" PRIx64, - addr); + error = Status::FromErrorStringWithFormat( + "memory write failed for 0x%" PRIx64, addr); else if (response.IsUnsupportedResponse()) - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "GDB server does not support writing memory"); else - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "unexpected response to GDB server memory write packet '%s': '%s'", packet.GetData(), response.GetStringRef().data()); } else { - error.SetErrorStringWithFormat("failed to send packet: '%s'", - packet.GetData()); + error = Status::FromErrorStringWithFormat("failed to send packet: '%s'", + packet.GetData()); } return 0; } @@ -2933,7 +2943,7 @@ lldb::addr_t ProcessGDBRemote::DoAllocateMemory(size_t size, } if (allocated_addr == LLDB_INVALID_ADDRESS) - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "unable to allocate %" PRIu64 " bytes of memory with permissions %s", (uint64_t)size, GetPermissionsAsCString(permissions)); else @@ -2964,13 +2974,13 @@ Status ProcessGDBRemote::DoDeallocateMemory(lldb::addr_t addr) { case eLazyBoolCalculate: // We should never be deallocating memory without allocating memory first // so we should never get eLazyBoolCalculate - error.SetErrorString( + error = Status::FromErrorString( "tried to deallocate memory without ever allocating memory"); break; case eLazyBoolYes: if (!m_gdb_comm.DeallocateMemory(addr)) - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "unable to deallocate memory at 0x%" PRIx64, addr); break; @@ -2982,7 +2992,7 @@ Status ProcessGDBRemote::DoDeallocateMemory(lldb::addr_t addr) { InferiorCallMunmap(this, addr, pos->second)) m_addr_to_mmap_size.erase(pos); else - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "unable to deallocate memory at 0x%" PRIx64, addr); } break; @@ -3063,10 +3073,10 @@ Status ProcessGDBRemote::EnableBreakpointSite(BreakpointSite *bp_site) { // fall through and try another form of breakpoint. if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) { if (error_no != UINT8_MAX) - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "error: %d sending the breakpoint request", error_no); else - error.SetErrorString("error sending the breakpoint request"); + error = Status::FromErrorString("error sending the breakpoint request"); return error; } @@ -3098,14 +3108,15 @@ Status ProcessGDBRemote::EnableBreakpointSite(BreakpointSite *bp_site) { if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) { // Unable to set this hardware breakpoint if (error_no != UINT8_MAX) - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "error: %d sending the hardware breakpoint request " "(hardware breakpoint resources might be exhausted or unavailable)", error_no); else - error.SetErrorString("error sending the hardware breakpoint request " - "(hardware breakpoint resources " - "might be exhausted or unavailable)"); + error = Status::FromErrorString( + "error sending the hardware breakpoint request " + "(hardware breakpoint resources " + "might be exhausted or unavailable)"); return error; } @@ -3118,7 +3129,7 @@ Status ProcessGDBRemote::EnableBreakpointSite(BreakpointSite *bp_site) { // Don't fall through when hardware breakpoints were specifically requested if (bp_site->HardwareRequired()) { - error.SetErrorString("hardware breakpoints are not supported"); + error = Status::FromErrorString("hardware breakpoints are not supported"); return error; } @@ -3151,14 +3162,14 @@ Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) { if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, false, addr, bp_op_size, GetInterruptTimeout())) - error.SetErrorToGenericError(); + error = Status::FromErrorString("unknown error"); break; case BreakpointSite::eExternal: { if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, false, addr, bp_op_size, GetInterruptTimeout())) - error.SetErrorToGenericError(); + error = Status::FromErrorString("unknown error"); } break; } if (error.Success()) @@ -3172,7 +3183,7 @@ Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) { } if (error.Success()) - error.SetErrorToGenericError(); + error = Status::FromErrorString("unknown error"); return error; } @@ -3196,7 +3207,7 @@ GetGDBStoppointType(const WatchpointResourceSP &wp_res_sp) { Status ProcessGDBRemote::EnableWatchpoint(WatchpointSP wp_sp, bool notify) { Status error; if (!wp_sp) { - error.SetErrorString("No watchpoint specified"); + error = Status::FromErrorString("No watchpoint specified"); return error; } user_id_t watchID = wp_sp->GetID(); @@ -3282,7 +3293,8 @@ Status ProcessGDBRemote::EnableWatchpoint(WatchpointSP wp_sp, bool notify) { m_gdb_comm.SendGDBStoppointTypePacket(type, false, addr, size, GetInterruptTimeout()); } - error.SetErrorString("Setting one of the watchpoint resources failed"); + error = Status::FromErrorString( + "Setting one of the watchpoint resources failed"); } return error; } @@ -3290,7 +3302,7 @@ Status ProcessGDBRemote::EnableWatchpoint(WatchpointSP wp_sp, bool notify) { Status ProcessGDBRemote::DisableWatchpoint(WatchpointSP wp_sp, bool notify) { Status error; if (!wp_sp) { - error.SetErrorString("Watchpoint argument was NULL."); + error = Status::FromErrorString("Watchpoint argument was NULL."); return error; } @@ -3341,7 +3353,8 @@ Status ProcessGDBRemote::DisableWatchpoint(WatchpointSP wp_sp, bool notify) { wp_sp->SetEnabled(false, notify); if (!disabled_all) - error.SetErrorString("Failure disabling one of the watchpoint locations"); + error = Status::FromErrorString( + "Failure disabling one of the watchpoint locations"); } return error; } @@ -3357,7 +3370,8 @@ Status ProcessGDBRemote::DoSignal(int signo) { LLDB_LOGF(log, "ProcessGDBRemote::DoSignal (signal = %d)", signo); if (!m_gdb_comm.SendAsyncSignal(signo, GetInterruptTimeout())) - error.SetErrorStringWithFormat("failed to send signal %i", signo); + error = + Status::FromErrorStringWithFormat("failed to send signal %i", signo); return error; } @@ -3369,7 +3383,7 @@ ProcessGDBRemote::EstablishConnectionIfNeeded(const ProcessInfo &process_info) { PlatformSP platform_sp(GetTarget().GetPlatform()); if (platform_sp && !platform_sp->IsHost()) - return Status("Lost debug server connection"); + return Status::FromErrorString("Lost debug server connection"); auto error = LaunchAndConnectToDebugserver(process_info); if (error.Fail()) { @@ -3439,7 +3453,7 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver( // reasons. int sockets[2]; /* the pair of socket descriptors */ if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) == -1) { - error.SetErrorToErrno(); + error = Status::FromErrno(); return error; } @@ -3486,7 +3500,7 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver( // connecting (send NULL URL) error = ConnectToDebugserver(""); } else { - error.SetErrorString("connection failed"); + error = Status::FromErrorString("connection failed"); } } return error; @@ -3895,10 +3909,11 @@ Status ProcessGDBRemote::SendEventData(const char *data) { return_value = m_gdb_comm.SendLaunchEventDataPacket(data, &was_supported); if (return_value != 0) { if (!was_supported) - error.SetErrorString("Sending events is not supported for this process."); + error = Status::FromErrorString( + "Sending events is not supported for this process."); else - error.SetErrorStringWithFormat("Error sending event data: %d.", - return_value); + error = Status::FromErrorStringWithFormat("Error sending event data: %d.", + return_value); } return error; } @@ -5195,7 +5210,7 @@ Status ProcessGDBRemote::GetFileLoadAddress(const FileSpec &file, std::string file_path = file.GetPath(false); if (file_path.empty()) - return Status("Empty file name specified"); + return Status::FromErrorString("Empty file name specified"); StreamString packet; packet.PutCString("qFileLoadAddress:"); @@ -5204,7 +5219,7 @@ Status ProcessGDBRemote::GetFileLoadAddress(const FileSpec &file, StringExtractorGDBRemote response; if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response) != GDBRemoteCommunication::PacketResult::Success) - return Status("Sending qFileLoadAddress packet failed"); + return Status::FromErrorString("Sending qFileLoadAddress packet failed"); if (response.IsErrorResponse()) { if (response.GetError() == 1) { @@ -5214,7 +5229,7 @@ Status ProcessGDBRemote::GetFileLoadAddress(const FileSpec &file, return Status(); } - return Status( + return Status::FromErrorString( "Fetching file load address from remote server returned an error"); } @@ -5224,7 +5239,7 @@ Status ProcessGDBRemote::GetFileLoadAddress(const FileSpec &file, return Status(); } - return Status( + return Status::FromErrorString( "Unknown error happened during sending the load address packet"); } |