mgorny updated this revision to Diff 300532. mgorny added a comment. Rework error handling to use `llvm::Error` in the new methods.
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89335/new/ https://reviews.llvm.org/D89335 Files: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.cpp lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h lldb/source/Plugins/Process/Utility/CMakeLists.txt lldb/source/Plugins/Process/Utility/NativeRegisterContextCache.cpp lldb/source/Plugins/Process/Utility/NativeRegisterContextCache.h
Index: lldb/source/Plugins/Process/Utility/NativeRegisterContextCache.h =================================================================== --- /dev/null +++ lldb/source/Plugins/Process/Utility/NativeRegisterContextCache.h @@ -0,0 +1,53 @@ +//===-- NativeRegisterContextCache.h ----------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef lldb_NativeRegisterContextCache_h +#define lldb_NativeRegisterContextCache_h + +#include "Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h" + +#include "llvm/ADT/Optional.h" +#include "llvm/Support/Error.h" + +#include <cstdint> +#include <map> + +namespace lldb_private { + +enum class RegSetCacheStatus { + Empty, + Read, + Written, +}; + +class NativeRegisterContextCache + : public virtual NativeRegisterContextRegisterInfo { +public: + Status ReadRegister(const RegisterInfo *reg_info, RegisterValue ®_value) override; + Status WriteRegister(const RegisterInfo *reg_info, const RegisterValue ®_value) override; + llvm::Error FlushRegisterSets(); + +protected: + virtual llvm::Optional<uint32_t> GetRegisterSetForRegNum(uint32_t reg_index) const = 0; + + virtual llvm::Error ReadRegisterSet(uint32_t set) = 0; + virtual llvm::Error WriteRegisterSet(uint32_t set) = 0; + + virtual llvm::Error GetRegisterFromCache(const RegisterInfo *reg_info, RegisterValue ®_value, uint32_t set) = 0; + virtual llvm::Error SetRegisterInCache(const RegisterInfo *reg_info, const RegisterValue ®_value, uint32_t set) = 0; + + virtual llvm::Error FlushRegisterSet(uint32_t set); + +private: + std::map<int, RegSetCacheStatus> m_regset_cache_status; +}; + +} // namespace lldb_private + +#endif // #ifndef lldb_NativeRegisterContextCache_h + Index: lldb/source/Plugins/Process/Utility/NativeRegisterContextCache.cpp =================================================================== --- /dev/null +++ lldb/source/Plugins/Process/Utility/NativeRegisterContextCache.cpp @@ -0,0 +1,96 @@ +//===-- NativeRegisterContextCache.cpp ------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "NativeRegisterContextCache.h" + +using namespace lldb_private; + +Status +NativeRegisterContextCache::ReadRegister(const RegisterInfo *reg_info, + RegisterValue ®_value) { + Status error; + + if (!reg_info) { + error.SetErrorString("reg_info NULL"); + return error; + } + + uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB]; + if (reg == LLDB_INVALID_REGNUM) { + // This is likely an internal register for lldb use only and should not be + // directly queried. + error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb " + "register, cannot read directly", + reg_info->name); + return error; + } + + llvm::Optional<uint32_t> set = GetRegisterSetForRegNum(reg); + if (!set) { + error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set", + reg_info->name); + return error; + } + + llvm::Error error2 = ReadRegisterSet(set.getValue()); + if (!error2) + error2 = GetRegisterFromCache(reg_info, reg_value, set.getValue()); + return Status(std::move(error2)); +} + +Status NativeRegisterContextCache::WriteRegister( + const RegisterInfo *reg_info, const RegisterValue ®_value) { + + Status error; + + if (!reg_info) { + error.SetErrorString("reg_info NULL"); + return error; + } + + uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB]; + if (reg == LLDB_INVALID_REGNUM) { + // This is likely an internal register for lldb use only and should not be + // directly queried. + error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb " + "register, cannot read directly", + reg_info->name); + return error; + } + + llvm::Optional<uint32_t> set = GetRegisterSetForRegNum(reg); + if (!set) { + error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set", + reg_info->name); + return error; + } + + llvm::Error error2 = ReadRegisterSet(set.getValue()); + if (!error2) + error2 = SetRegisterInCache(reg_info, reg_value, set.getValue()); + if (!error2) + error2 = WriteRegisterSet(set.getValue()); + return Status(std::move(error2)); +} + +llvm::Error NativeRegisterContextCache::FlushRegisterSets() { + for (auto kv : m_regset_cache_status) { + if (kv.second == RegSetCacheStatus::Empty) + continue; + llvm::Error error = FlushRegisterSet(kv.first); + if (error) + return error; + } + + return llvm::Error::success(); +} + +llvm::Error NativeRegisterContextCache::FlushRegisterSet(uint32_t set) { + m_regset_cache_status[set] = RegSetCacheStatus::Empty; + return llvm::Error::success(); +} Index: lldb/source/Plugins/Process/Utility/CMakeLists.txt =================================================================== --- lldb/source/Plugins/Process/Utility/CMakeLists.txt +++ lldb/source/Plugins/Process/Utility/CMakeLists.txt @@ -9,6 +9,7 @@ LinuxProcMaps.cpp LinuxSignals.cpp MipsLinuxSignals.cpp + NativeRegisterContextCache.cpp NativeRegisterContextRegisterInfo.cpp NativeRegisterContextWatchpoint_x86.cpp NetBSDSignals.cpp Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h =================================================================== --- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h +++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h @@ -20,9 +20,12 @@ #include "Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h" #include "Plugins/Process/Utility/RegisterContext_x86.h" +#include "Plugins/Process/Utility/NativeRegisterContextCache.h" #include "Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h" #include "Plugins/Process/Utility/lldb-x86-register-enums.h" +#include "llvm/Support/Error.h" + namespace lldb_private { namespace process_netbsd { @@ -30,6 +33,7 @@ class NativeRegisterContextNetBSD_x86_64 : public NativeRegisterContextNetBSD, + public NativeRegisterContextCache, public NativeRegisterContextWatchpoint_x86 { public: NativeRegisterContextNetBSD_x86_64(const ArchSpec &target_arch, @@ -38,12 +42,6 @@ const RegisterSet *GetRegisterSet(uint32_t set_index) const override; - Status ReadRegister(const RegisterInfo *reg_info, - RegisterValue ®_value) override; - - Status WriteRegister(const RegisterInfo *reg_info, - const RegisterValue ®_value) override; - Status ReadAllRegisterValues(lldb::DataBufferSP &data_sp) override; Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override; @@ -51,6 +49,19 @@ Status CopyHardwareWatchpointsFrom(NativeRegisterContextNetBSD &source) override; +protected: + llvm::Optional<uint32_t> GetRegisterSetForRegNum(uint32_t reg_index) const override; + + llvm::Error ReadRegisterSet(uint32_t set) override; + llvm::Error WriteRegisterSet(uint32_t set) override; + + llvm::Error GetRegisterFromCache(const RegisterInfo *reg_info, + RegisterValue ®_value, + uint32_t set) override; + llvm::Error SetRegisterInCache(const RegisterInfo *reg_info, + const RegisterValue ®_value, + uint32_t set) override; + private: // Private member types. enum { GPRegSet, XStateRegSet, DBRegSet }; @@ -60,11 +71,7 @@ struct xstate m_xstate; struct dbreg m_dbr; - int GetSetForNativeRegNum(int reg_num) const; int GetDR(int num) const; - - Status ReadRegisterSet(uint32_t set); - Status WriteRegisterSet(uint32_t set); }; } // namespace process_netbsd Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp =================================================================== --- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp +++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp @@ -16,6 +16,8 @@ #include "lldb/Utility/RegisterValue.h" #include "lldb/Utility/Status.h" +#include "llvm/Support/FormatVariadic.h" + #include "Plugins/Process/Utility/RegisterContextNetBSD_i386.h" #include "Plugins/Process/Utility/RegisterContextNetBSD_x86_64.h" @@ -236,13 +238,7 @@ // CONSIDER after local and llgs debugging are merged, register set support can // be moved into a base x86-64 class with IsRegisterSetAvailable made virtual. uint32_t NativeRegisterContextNetBSD_x86_64::GetRegisterSetCount() const { - uint32_t sets = 0; - for (uint32_t set_index = 0; set_index < k_num_register_sets; ++set_index) { - if (GetSetForNativeRegNum(set_index) != -1) - ++sets; - } - - return sets; + return k_num_register_sets; } const RegisterSet * @@ -370,8 +366,8 @@ } } -int NativeRegisterContextNetBSD_x86_64::GetSetForNativeRegNum( - int reg_num) const { +llvm::Optional<uint32_t> NativeRegisterContextNetBSD_x86_64::GetRegisterSetForRegNum( + uint32_t reg_num) const { switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) { case llvm::Triple::x86: if (reg_num >= k_first_gpr_i386 && reg_num <= k_last_gpr_i386) @@ -381,9 +377,9 @@ if (reg_num >= k_first_avx_i386 && reg_num <= k_last_avx_i386) return XStateRegSet; // AVX if (reg_num >= k_first_mpxr_i386 && reg_num <= k_last_mpxr_i386) - return -1; // MPXR + return llvm::None; // MPXR if (reg_num >= k_first_mpxc_i386 && reg_num <= k_last_mpxc_i386) - return -1; // MPXC + return llvm::None; // MPXC if (reg_num >= k_first_dbr_i386 && reg_num <= k_last_dbr_i386) return DBRegSet; // DBR break; @@ -395,9 +391,9 @@ if (reg_num >= k_first_avx_x86_64 && reg_num <= k_last_avx_x86_64) return XStateRegSet; // AVX if (reg_num >= k_first_mpxr_x86_64 && reg_num <= k_last_mpxr_x86_64) - return -1; // MPXR + return llvm::None; // MPXR if (reg_num >= k_first_mpxc_x86_64 && reg_num <= k_last_mpxc_x86_64) - return -1; // MPXC + return llvm::None; // MPXC if (reg_num >= k_first_dbr_x86_64 && reg_num <= k_last_dbr_x86_64) return DBRegSet; // DBR break; @@ -408,7 +404,7 @@ llvm_unreachable("Register does not belong to any register set"); } -Status NativeRegisterContextNetBSD_x86_64::ReadRegisterSet(uint32_t set) { +llvm::Error NativeRegisterContextNetBSD_x86_64::ReadRegisterSet(uint32_t set) { switch (set) { case GPRegSet: return DoRegisterSet(PT_GETREGS, &m_gpr); @@ -422,7 +418,7 @@ llvm_unreachable("NativeRegisterContextNetBSD_x86_64::ReadRegisterSet"); } -Status NativeRegisterContextNetBSD_x86_64::WriteRegisterSet(uint32_t set) { +llvm::Error NativeRegisterContextNetBSD_x86_64::WriteRegisterSet(uint32_t set) { switch (set) { case GPRegSet: return DoRegisterSet(PT_SETREGS, &m_gpr); @@ -436,34 +432,12 @@ llvm_unreachable("NativeRegisterContextNetBSD_x86_64::WriteRegisterSet"); } -Status -NativeRegisterContextNetBSD_x86_64::ReadRegister(const RegisterInfo *reg_info, - RegisterValue ®_value) { - Status error; - - if (!reg_info) { - error.SetErrorString("reg_info NULL"); - return error; - } +llvm::Error +NativeRegisterContextNetBSD_x86_64::GetRegisterFromCache( + const RegisterInfo *reg_info, RegisterValue ®_value, uint32_t set) { + assert(reg_info); uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB]; - if (reg == LLDB_INVALID_REGNUM) { - // This is likely an internal register for lldb use only and should not be - // directly queried. - error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb " - "register, cannot read directly", - reg_info->name); - return error; - } - - int set = GetSetForNativeRegNum(reg); - if (set == -1) { - // This is likely an internal register for lldb use only and should not be - // directly queried. - error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set", - reg_info->name); - return error; - } switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) { case llvm::Triple::x86_64: @@ -475,10 +449,6 @@ llvm_unreachable("Unhandled target architecture."); } - error = ReadRegisterSet(set); - if (error.Fail()) - return error; - switch (reg) { #if defined(__x86_64__) case lldb_rax_x86_64: @@ -671,13 +641,10 @@ case lldb_xmm13_x86_64: case lldb_xmm14_x86_64: case lldb_xmm15_x86_64: - if (!(m_xstate.xs_rfbm & XCR0_SSE)) { - error.SetErrorStringWithFormat( - "register \"%s\" not supported by CPU/kernel", reg_info->name); - } else { - reg_value.SetBytes(&m_xstate.xs_fxsave.fx_xmm[reg - lldb_xmm0_x86_64], - reg_info->byte_size, endian::InlHostByteOrder()); - } + if (!(m_xstate.xs_rfbm & XCR0_SSE)) + return llvm::make_error<llvm::StringError>(llvm::inconvertibleErrorCode(), llvm::formatv("register '{0}' not supported by CPU/kernel", reg_info->name).str()); + reg_value.SetBytes(&m_xstate.xs_fxsave.fx_xmm[reg - lldb_xmm0_x86_64], + reg_info->byte_size, endian::InlHostByteOrder()); break; case lldb_ymm0_x86_64: case lldb_ymm1_x86_64: @@ -695,19 +662,19 @@ case lldb_ymm13_x86_64: case lldb_ymm14_x86_64: case lldb_ymm15_x86_64: + { if (!(m_xstate.xs_rfbm & XCR0_SSE) || - !(m_xstate.xs_rfbm & XCR0_YMM_Hi128)) { - error.SetErrorStringWithFormat( - "register \"%s\" not supported by CPU/kernel", reg_info->name); - } else { - uint32_t reg_index = reg - lldb_ymm0_x86_64; - YMMReg ymm = - XStateToYMM(m_xstate.xs_fxsave.fx_xmm[reg_index].xmm_bytes, - m_xstate.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes); - reg_value.SetBytes(ymm.bytes, reg_info->byte_size, - endian::InlHostByteOrder()); - } + !(m_xstate.xs_rfbm & XCR0_YMM_Hi128)) + return llvm::make_error<llvm::StringError>(llvm::inconvertibleErrorCode(), llvm::formatv("register '{0}' not supported by CPU/kernel", reg_info->name).str()); + + uint32_t reg_index = reg - lldb_ymm0_x86_64; + YMMReg ymm = + XStateToYMM(m_xstate.xs_fxsave.fx_xmm[reg_index].xmm_bytes, + m_xstate.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes); + reg_value.SetBytes(ymm.bytes, reg_info->byte_size, + endian::InlHostByteOrder()); break; + } case lldb_dr0_x86_64: case lldb_dr1_x86_64: case lldb_dr2_x86_64: @@ -722,37 +689,16 @@ llvm_unreachable("Reading unknown/unsupported register"); } - return error; + return llvm::Error::success(); } -Status NativeRegisterContextNetBSD_x86_64::WriteRegister( - const RegisterInfo *reg_info, const RegisterValue ®_value) { - - Status error; - - if (!reg_info) { - error.SetErrorString("reg_info NULL"); - return error; - } +llvm::Error NativeRegisterContextNetBSD_x86_64::SetRegisterInCache( + const RegisterInfo *reg_info, + const RegisterValue ®_value, + uint32_t set) { + assert(reg_info); uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB]; - if (reg == LLDB_INVALID_REGNUM) { - // This is likely an internal register for lldb use only and should not be - // directly queried. - error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb " - "register, cannot read directly", - reg_info->name); - return error; - } - - int set = GetSetForNativeRegNum(reg); - if (set == -1) { - // This is likely an internal register for lldb use only and should not be - // directly queried. - error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set", - reg_info->name); - return error; - } uint64_t new_xstate_bv = XCR0_X87; // the most common case @@ -766,10 +712,6 @@ llvm_unreachable("Unhandled target architecture."); } - error = ReadRegisterSet(set); - if (error.Fail()) - return error; - switch (reg) { #if defined(__x86_64__) case lldb_rax_x86_64: @@ -964,14 +906,11 @@ case lldb_xmm13_x86_64: case lldb_xmm14_x86_64: case lldb_xmm15_x86_64: - if (!(m_xstate.xs_rfbm & XCR0_SSE)) { - error.SetErrorStringWithFormat( - "register \"%s\" not supported by CPU/kernel", reg_info->name); - } else { - ::memcpy(&m_xstate.xs_fxsave.fx_xmm[reg - lldb_xmm0_x86_64], - reg_value.GetBytes(), reg_value.GetByteSize()); - new_xstate_bv = XCR0_SSE; - } + if (!(m_xstate.xs_rfbm & XCR0_SSE)) + return llvm::make_error<llvm::StringError>(llvm::inconvertibleErrorCode(), llvm::formatv("register '{0}' not supported by CPU/kernel", reg_info->name).str()); + ::memcpy(&m_xstate.xs_fxsave.fx_xmm[reg - lldb_xmm0_x86_64], + reg_value.GetBytes(), reg_value.GetByteSize()); + new_xstate_bv = XCR0_SSE; break; case lldb_ymm0_x86_64: case lldb_ymm1_x86_64: @@ -989,19 +928,19 @@ case lldb_ymm13_x86_64: case lldb_ymm14_x86_64: case lldb_ymm15_x86_64: + { if (!(m_xstate.xs_rfbm & XCR0_SSE) || - !(m_xstate.xs_rfbm & XCR0_YMM_Hi128)) { - error.SetErrorStringWithFormat( - "register \"%s\" not supported by CPU/kernel", reg_info->name); - } else { - uint32_t reg_index = reg - lldb_ymm0_x86_64; - YMMReg ymm; - ::memcpy(ymm.bytes, reg_value.GetBytes(), reg_value.GetByteSize()); - YMMToXState(ymm, m_xstate.xs_fxsave.fx_xmm[reg_index].xmm_bytes, - m_xstate.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes); - new_xstate_bv = XCR0_SSE | XCR0_YMM_Hi128; - } + !(m_xstate.xs_rfbm & XCR0_YMM_Hi128)) + return llvm::make_error<llvm::StringError>(llvm::inconvertibleErrorCode(), llvm::formatv("register '{0}' not supported by CPU/kernel", reg_info->name).str()); + + uint32_t reg_index = reg - lldb_ymm0_x86_64; + YMMReg ymm; + ::memcpy(ymm.bytes, reg_value.GetBytes(), reg_value.GetByteSize()); + YMMToXState(ymm, m_xstate.xs_fxsave.fx_xmm[reg_index].xmm_bytes, + m_xstate.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes); + new_xstate_bv = XCR0_SSE | XCR0_YMM_Hi128; break; + } case lldb_dr0_x86_64: case lldb_dr1_x86_64: case lldb_dr2_x86_64: @@ -1019,7 +958,7 @@ if (set == XStateRegSet) m_xstate.xs_xstate_bv |= new_xstate_bv; - return WriteRegisterSet(set); + return llvm::Error::success(); } Status NativeRegisterContextNetBSD_x86_64::ReadAllRegisterValues( @@ -1090,16 +1029,16 @@ Status NativeRegisterContextNetBSD_x86_64::CopyHardwareWatchpointsFrom( NativeRegisterContextNetBSD &source) { auto &r_source = static_cast<NativeRegisterContextNetBSD_x86_64&>(source); - Status res = r_source.ReadRegisterSet(DBRegSet); - if (!res.Fail()) { + llvm::Error error = r_source.ReadRegisterSet(DBRegSet); + if (!error) { // copy dbregs only if any watchpoints were set if ((r_source.m_dbr.dr[7] & 0xFF) == 0) - return res; + return Status(); m_dbr = r_source.m_dbr; - res = WriteRegisterSet(DBRegSet); + error = WriteRegisterSet(DBRegSet); } - return res; + return Status(std::move(error)); } #endif // defined(__x86_64__) Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h =================================================================== --- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h +++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h @@ -11,6 +11,8 @@ #include "lldb/Host/common/NativeThreadProtocol.h" +#include "llvm/Support/Error.h" + #include "Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h" namespace lldb_private { @@ -33,7 +35,7 @@ CopyHardwareWatchpointsFrom(NativeRegisterContextNetBSD &source) = 0; protected: - Status DoRegisterSet(int req, void *buf); + llvm::Error DoRegisterSet(int req, void *buf); virtual NativeProcessNetBSD &GetProcess(); virtual ::pid_t GetProcessPid(); }; Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.cpp =================================================================== --- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.cpp +++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.cpp @@ -20,9 +20,8 @@ #include <sys/ptrace.h> // clang-format on -Status NativeRegisterContextNetBSD::DoRegisterSet(int ptrace_req, void *buf) { - return NativeProcessNetBSD::PtraceWrapper(ptrace_req, GetProcessPid(), buf, - m_thread.GetID()); +llvm::Error NativeRegisterContextNetBSD::DoRegisterSet(int ptrace_req, void *buf) { + return NativeProcessNetBSD::PtraceWrapper(ptrace_req, GetProcessPid(), buf, m_thread.GetID()).ToError(); } NativeProcessNetBSD &NativeRegisterContextNetBSD::GetProcess() {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
