llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) <details> <summary>Changes</summary> This macro expands to: ``` assert(expr); if (!expr) return retval; ``` This is to be used in places where during development you want a hard error so it's less confusing to debug, but in release builds you'd want to return early so you don't cause damage. I've made use of this in the Arm/AArch64 native and corefile classes, plus some in LoongArch and riscv64 that were easy to find. In some places I've not given each one a custom error string because what you'd probably do is try an asserts build and get exact location information from that, which serves the same purpose. --- Full diff: https://github.com/llvm/llvm-project/pull/71175.diff 6 Files Affected: - (modified) lldb/include/lldb/lldb-defines.h (+9) - (modified) lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp (+9-4) - (modified) lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp (+24-18) - (modified) lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp (+6-4) - (modified) lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp (+6-4) - (modified) lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp (+8-8) ``````````diff diff --git a/lldb/include/lldb/lldb-defines.h b/lldb/include/lldb/lldb-defines.h index 6950a4f3a496acf..600240d903d302d 100644 --- a/lldb/include/lldb/lldb-defines.h +++ b/lldb/include/lldb/lldb-defines.h @@ -139,4 +139,13 @@ #define LLDB_DEPRECATED_FIXME(MSG, FIX) LLDB_DEPRECATED(MSG) #endif +// When asserts are enabled, use an assert to check expr. If they are not +// enabled return the value retval if expr is not true. Used when a hard failure +// is useful during development, but you want to return early to prevent +// corrupted state in release builds. +#define LLDB_ASSERT_OR_RETURN(expr, retval) \ + assert(expr); \ + if (!(expr)) \ + return (retval); + #endif // LLDB_LLDB_DEFINES_H diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp index 5ad2f7a8e9455b1..88f87fc6d22b82a 100644 --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp @@ -143,7 +143,8 @@ NativeRegisterContextLinux_arm::ReadRegister(const RegisterInfo *reg_info, // Get pointer to m_fpr variable and set the data from it. uint32_t fpr_offset = CalculateFprOffset(reg_info); - assert(fpr_offset < sizeof m_fpr); + LLDB_ASSERT_OR_RETURN(fpr_offset < sizeof m_fpr, + Status("Invalid fpr offset for register read")); uint8_t *src = (uint8_t *)&m_fpr + fpr_offset; switch (reg_info->byte_size) { case 2: @@ -186,7 +187,8 @@ NativeRegisterContextLinux_arm::WriteRegister(const RegisterInfo *reg_info, if (IsFPR(reg_index)) { // Get pointer to m_fpr variable and set the data to it. uint32_t fpr_offset = CalculateFprOffset(reg_info); - assert(fpr_offset < sizeof m_fpr); + LLDB_ASSERT_OR_RETURN(fpr_offset < sizeof m_fpr, + Status("Invalid fpr offset for register read")); uint8_t *dst = (uint8_t *)&m_fpr + fpr_offset; ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size); @@ -794,7 +796,9 @@ Status NativeRegisterContextLinux_arm::DoReadRegisterValue( // read out the full GPR register set instead. This approach is about 4 times // slower but the performance overhead is negligible in comparison to // processing time in lldb-server. - assert(offset % 4 == 0 && "Try to write a register with unaligned offset"); + LLDB_ASSERT_OR_RETURN( + offset % 4 == 0, + Status("Trying to read a register with unaligned offset")); if (offset + sizeof(uint32_t) > sizeof(m_gpr_arm)) return Status("Register isn't fit into the size of the GPR area"); @@ -813,7 +817,8 @@ Status NativeRegisterContextLinux_arm::DoWriteRegisterValue( // read out the full GPR register set, modify the requested register and // write it back. This approach is about 4 times slower but the performance // overhead is negligible in comparison to processing time in lldb-server. - assert(offset % 4 == 0 && "Try to write a register with unaligned offset"); + LLDB_ASSERT_OR_RETURN( + offset % 4 == 0, Status("Try to write a register with unaligned offset")); if (offset + sizeof(uint32_t) > sizeof(m_gpr_arm)) return Status("Register isn't fit into the size of the GPR area"); diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp index e23165933c221cf..edfa79e87d714e8 100644 --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp @@ -235,6 +235,7 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info, uint32_t offset = LLDB_INVALID_INDEX32; uint64_t sve_vg; std::vector<uint8_t> sve_reg_non_live; + Status offset_error("Invalid register read offset"); if (IsGPR(reg)) { error = ReadGPR(); @@ -242,7 +243,7 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info, return error; offset = reg_info->byte_offset; - assert(offset < GetGPRSize()); + LLDB_ASSERT_OR_RETURN(offset < GetGPRSize(), offset_error); src = (uint8_t *)GetGPRBuffer() + offset; } else if (IsFPR(reg)) { @@ -253,7 +254,7 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info, return error; offset = CalculateFprOffset(reg_info); - assert(offset < GetFPRSize()); + LLDB_ASSERT_OR_RETURN(offset < GetFPRSize(), offset_error); src = (uint8_t *)GetFPRBuffer() + offset; } else { // SVE or SSVE enabled, we will read and cache SVE ptrace data. @@ -288,7 +289,7 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info, offset = CalculateSVEOffset(GetRegisterInfoAtIndex(sve_reg_num)); } - assert(offset < GetSVEBufferSize()); + LLDB_ASSERT_OR_RETURN(offset < GetSVEBufferSize(), offset_error); src = (uint8_t *)GetSVEBuffer() + offset; } } else if (IsTLS(reg)) { @@ -297,7 +298,7 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info, return error; offset = reg_info->byte_offset - GetRegisterInfo().GetTLSOffset(); - assert(offset < GetTLSBufferSize()); + LLDB_ASSERT_OR_RETURN(offset < GetTLSBufferSize(), offset_error); src = (uint8_t *)GetTLSBuffer() + offset; } else if (IsSVE(reg)) { if (m_sve_state == SVEState::Disabled || m_sve_state == SVEState::Unknown) @@ -321,13 +322,13 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info, if (GetRegisterInfo().IsSVEZReg(reg)) { offset = CalculateSVEOffset(reg_info); - assert(offset < GetSVEBufferSize()); + LLDB_ASSERT_OR_RETURN(offset < GetSVEBufferSize(), offset_error); ::memcpy(sve_reg_non_live.data(), (uint8_t *)GetSVEBuffer() + offset, 16); } } else { offset = CalculateSVEOffset(reg_info); - assert(offset < GetSVEBufferSize()); + LLDB_ASSERT_OR_RETURN(offset < GetSVEBufferSize(), offset_error); src = (uint8_t *)GetSVEBuffer() + offset; } } @@ -337,7 +338,7 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info, return error; offset = reg_info->byte_offset - GetRegisterInfo().GetPAuthOffset(); - assert(offset < GetPACMaskSize()); + LLDB_ASSERT_OR_RETURN(offset < GetPACMaskSize(), offset_error); src = (uint8_t *)GetPACMask() + offset; } else if (IsMTE(reg)) { error = ReadMTEControl(); @@ -345,7 +346,7 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info, return error; offset = reg_info->byte_offset - GetRegisterInfo().GetMTEOffset(); - assert(offset < GetMTEControlSize()); + LLDB_ASSERT_OR_RETURN(offset < GetMTEControlSize(), offset_error); src = (uint8_t *)GetMTEControl() + offset; } else if (IsSME(reg)) { if (GetRegisterInfo().IsSMERegZA(reg)) { @@ -391,7 +392,7 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info, ReadSMEControl(); offset = reg_info->byte_offset - GetRegisterInfo().GetSMEOffset(); - assert(offset < GetSMEPseudoBufferSize()); + LLDB_ASSERT_OR_RETURN(offset < GetSMEPseudoBufferSize(), offset_error); src = (uint8_t *)GetSMEPseudoBuffer() + offset; } } else @@ -421,13 +422,14 @@ Status NativeRegisterContextLinux_arm64::WriteRegister( uint8_t *dst; uint32_t offset = LLDB_INVALID_INDEX32; std::vector<uint8_t> sve_reg_non_live; + Status offset_error("Invalid register write offset"); if (IsGPR(reg)) { error = ReadGPR(); if (error.Fail()) return error; - assert(reg_info->byte_offset < GetGPRSize()); + LLDB_ASSERT_OR_RETURN(reg_info->byte_offset < GetGPRSize(), offset_error); dst = (uint8_t *)GetGPRBuffer() + reg_info->byte_offset; ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size); @@ -440,7 +442,7 @@ Status NativeRegisterContextLinux_arm64::WriteRegister( return error; offset = CalculateFprOffset(reg_info); - assert(offset < GetFPRSize()); + LLDB_ASSERT_OR_RETURN(offset < GetFPRSize(), offset_error); dst = (uint8_t *)GetFPRBuffer() + offset; ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size); @@ -476,7 +478,7 @@ Status NativeRegisterContextLinux_arm64::WriteRegister( offset = CalculateSVEOffset(GetRegisterInfoAtIndex(sve_reg_num)); } - assert(offset < GetSVEBufferSize()); + LLDB_ASSERT_OR_RETURN(offset < GetSVEBufferSize(), offset_error); dst = (uint8_t *)GetSVEBuffer() + offset; ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size); return WriteAllSVE(); @@ -541,7 +543,7 @@ Status NativeRegisterContextLinux_arm64::WriteRegister( // We are writing a Z register which is zero beyond 16 bytes so copy // first 16 bytes only as SVE payload mirrors legacy fpsimd structure offset = CalculateSVEOffset(reg_info); - assert(offset < GetSVEBufferSize()); + LLDB_ASSERT_OR_RETURN(offset < GetSVEBufferSize(), offset_error); dst = (uint8_t *)GetSVEBuffer() + offset; ::memcpy(dst, reg_value.GetBytes(), 16); @@ -550,7 +552,7 @@ Status NativeRegisterContextLinux_arm64::WriteRegister( return Status("SVE state change operation not supported"); } else { offset = CalculateSVEOffset(reg_info); - assert(offset < GetSVEBufferSize()); + LLDB_ASSERT_OR_RETURN(offset < GetSVEBufferSize(), offset_error); dst = (uint8_t *)GetSVEBuffer() + offset; ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size); return WriteAllSVE(); @@ -562,7 +564,7 @@ Status NativeRegisterContextLinux_arm64::WriteRegister( return error; offset = reg_info->byte_offset - GetRegisterInfo().GetMTEOffset(); - assert(offset < GetMTEControlSize()); + LLDB_ASSERT_OR_RETURN(offset < GetMTEControlSize(), offset_error); dst = (uint8_t *)GetMTEControl() + offset; ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size); @@ -573,7 +575,7 @@ Status NativeRegisterContextLinux_arm64::WriteRegister( return error; offset = reg_info->byte_offset - GetRegisterInfo().GetTLSOffset(); - assert(offset < GetTLSBufferSize()); + LLDB_ASSERT_OR_RETURN(offset < GetTLSBufferSize(), offset_error); dst = (uint8_t *)GetTLSBuffer() + offset; ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size); @@ -759,10 +761,13 @@ Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues( // constants and the functions vec_set_vector_length, sve_set_common and // za_set in the Linux Kernel. + Status za_header_error("Unexpected ZA header size"); + if ((m_sve_state != SVEState::Streaming) && GetRegisterInfo().IsZAPresent()) { // Use the header size not the buffer size, as we may be using the buffer // for fake data, which we do not want to write out. - assert(m_za_header.size <= GetZABufferSize()); + LLDB_ASSERT_OR_RETURN(m_za_header.size <= GetZABufferSize(), + za_header_error); dst = AddSavedRegisters(dst, RegisterSetType::SME, GetZABuffer(), m_za_header.size); } @@ -778,7 +783,8 @@ Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues( } if ((m_sve_state == SVEState::Streaming) && GetRegisterInfo().IsZAPresent()) { - assert(m_za_header.size <= GetZABufferSize()); + LLDB_ASSERT_OR_RETURN(m_za_header.size <= GetZABufferSize(), + za_header_error); dst = AddSavedRegisters(dst, RegisterSetType::SME, GetZABuffer(), m_za_header.size); } diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp index 8d7bd35b3bdbf45..1b86e88fc6ba735 100644 --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp @@ -106,6 +106,7 @@ Status NativeRegisterContextLinux_loongarch64::ReadRegister( uint8_t *src = nullptr; uint32_t offset = LLDB_INVALID_INDEX32; + Status offset_error("Invalid register read offset"); if (IsGPR(reg)) { error = ReadGPR(); @@ -113,7 +114,7 @@ Status NativeRegisterContextLinux_loongarch64::ReadRegister( return error; offset = reg_info->byte_offset; - assert(offset < GetGPRSize()); + LLDB_ASSERT_OR_RETURN(offset < GetGPRSize(), offset_error); src = (uint8_t *)GetGPRBuffer() + offset; } else if (IsFPR(reg)) { @@ -122,7 +123,7 @@ Status NativeRegisterContextLinux_loongarch64::ReadRegister( return error; offset = CalculateFprOffset(reg_info); - assert(offset < GetFPRSize()); + LLDB_ASSERT_OR_RETURN(offset < GetFPRSize(), offset_error); src = (uint8_t *)GetFPRBuffer() + offset; } else return Status("failed - register wasn't recognized to be a GPR or an FPR, " @@ -150,13 +151,14 @@ Status NativeRegisterContextLinux_loongarch64::WriteRegister( uint8_t *dst = nullptr; uint32_t offset = LLDB_INVALID_INDEX32; + Status offset_error("Invalid register write offset"); if (IsGPR(reg)) { error = ReadGPR(); if (error.Fail()) return error; - assert(reg_info->byte_offset < GetGPRSize()); + LLDB_ASSERT_OR_RETURN(reg_info->byte_offset < GetGPRSize(), offset_error); dst = (uint8_t *)GetGPRBuffer() + reg_info->byte_offset; ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size); @@ -167,7 +169,7 @@ Status NativeRegisterContextLinux_loongarch64::WriteRegister( return error; offset = CalculateFprOffset(reg_info); - assert(offset < GetFPRSize()); + LLDB_ASSERT_OR_RETURN(offset < GetFPRSize(), offset_error); dst = (uint8_t *)GetFPRBuffer() + offset; ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size); diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp index 1d51726a86df166..402d34ca303199d 100644 --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp @@ -113,6 +113,7 @@ NativeRegisterContextLinux_riscv64::ReadRegister(const RegisterInfo *reg_info, uint8_t *src = nullptr; uint32_t offset = LLDB_INVALID_INDEX32; + Status offset_error("Invalid register read offset"); if (IsGPR(reg)) { error = ReadGPR(); @@ -120,7 +121,7 @@ NativeRegisterContextLinux_riscv64::ReadRegister(const RegisterInfo *reg_info, return error; offset = reg_info->byte_offset; - assert(offset < GetGPRSize()); + LLDB_ASSERT_OR_RETURN(offset < GetGPRSize(), offset_error); src = (uint8_t *)GetGPRBuffer() + offset; } else if (IsFPR(reg)) { @@ -129,7 +130,7 @@ NativeRegisterContextLinux_riscv64::ReadRegister(const RegisterInfo *reg_info, return error; offset = CalculateFprOffset(reg_info); - assert(offset < GetFPRSize()); + LLDB_ASSERT_OR_RETURN(offset < GetFPRSize(), offset_error); src = (uint8_t *)GetFPRBuffer() + offset; } else return Status("failed - register wasn't recognized to be a GPR or an FPR, " @@ -162,13 +163,14 @@ Status NativeRegisterContextLinux_riscv64::WriteRegister( uint8_t *dst = nullptr; uint32_t offset = LLDB_INVALID_INDEX32; + Status offset_error("Invalid register write offset"); if (IsGPR(reg)) { error = ReadGPR(); if (error.Fail()) return error; - assert(reg_info->byte_offset < GetGPRSize()); + LLDB_ASSERT_OR_RETURN(reg_info->byte_offset < GetGPRSize(), offset_error); dst = (uint8_t *)GetGPRBuffer() + reg_info->byte_offset; ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size); @@ -179,7 +181,7 @@ Status NativeRegisterContextLinux_riscv64::WriteRegister( return error; offset = CalculateFprOffset(reg_info); - assert(offset < GetFPRSize()); + LLDB_ASSERT_OR_RETURN(offset < GetFPRSize(), offset_error); dst = (uint8_t *)GetFPRBuffer() + offset; ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size); diff --git a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp index 85073b56f64bf79..8129667079dc158 100644 --- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp +++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp @@ -246,8 +246,8 @@ bool RegisterContextCorePOSIX_arm64::ReadRegister(const RegisterInfo *reg_info, offset = CalculateSVEOffset(GetRegisterInfoAtIndex(sve_reg_num)); } - assert(sve_reg_num != LLDB_INVALID_REGNUM); - assert(offset < m_sve_data.GetByteSize()); + LLDB_ASSERT_OR_RETURN(sve_reg_num != LLDB_INVALID_REGNUM, false); + LLDB_ASSERT_OR_RETURN(offset < m_sve_data.GetByteSize(), false); value.SetFromMemoryData(*reg_info, GetSVEBuffer(offset), reg_info->byte_size, lldb::eByteOrderLittle, error); @@ -269,7 +269,7 @@ bool RegisterContextCorePOSIX_arm64::ReadRegister(const RegisterInfo *reg_info, if (IsSVEZ(reg)) { byte_size = 16; offset = CalculateSVEOffset(reg_info); - assert(offset < m_sve_data.GetByteSize()); + LLDB_ASSERT_OR_RETURN(offset < m_sve_data.GetByteSize(), false); src = GetSVEBuffer(offset); } value.SetFromMemoryData(*reg_info, src, byte_size, lldb::eByteOrderLittle, @@ -278,7 +278,7 @@ bool RegisterContextCorePOSIX_arm64::ReadRegister(const RegisterInfo *reg_info, case SVEState::Full: case SVEState::Streaming: offset = CalculateSVEOffset(reg_info); - assert(offset < m_sve_data.GetByteSize()); + LLDB_ASSERT_OR_RETURN(offset < m_sve_data.GetByteSize(), false); value.SetFromMemoryData(*reg_info, GetSVEBuffer(offset), reg_info->byte_size, lldb::eByteOrderLittle, error); @@ -289,17 +289,17 @@ bool RegisterContextCorePOSIX_arm64::ReadRegister(const RegisterInfo *reg_info, } } else if (IsPAuth(reg)) { offset = reg_info->byte_offset - m_register_info_up->GetPAuthOffset(); - assert(offset < m_pac_data.GetByteSize()); + LLDB_ASSERT_OR_RETURN(offset < m_pac_data.GetByteSize(), false); value.SetFromMemoryData(*reg_info, m_pac_data.GetDataStart() + offset, reg_info->byte_size, lldb::eByteOrderLittle, error); } else if (IsTLS(reg)) { offset = reg_info->byte_offset - m_register_info_up->GetTLSOffset(); - assert(offset < m_tls_data.GetByteSize()); + LLDB_ASSERT_OR_RETURN(offset < m_tls_data.GetByteSize(), false); value.SetFromMemoryData(*reg_info, m_tls_data.GetDataStart() + offset, reg_info->byte_size, lldb::eByteOrderLittle, error); } else if (IsMTE(reg)) { offset = reg_info->byte_offset - m_register_info_up->GetMTEOffset(); - assert(offset < m_mte_data.GetByteSize()); + LLDB_ASSERT_OR_RETURN(offset < m_mte_data.GetByteSize(), false); value.SetFromMemoryData(*reg_info, m_mte_data.GetDataStart() + offset, reg_info->byte_size, lldb::eByteOrderLittle, error); } else if (IsSME(reg)) { @@ -343,7 +343,7 @@ bool RegisterContextCorePOSIX_arm64::ReadRegister(const RegisterInfo *reg_info, error); } else { offset = reg_info->byte_offset - m_register_info_up->GetSMEOffset(); - assert(offset < sizeof(m_sme_pseudo_regs)); + LLDB_ASSERT_OR_RETURN(offset < sizeof(m_sme_pseudo_regs), false); // Host endian since these values are derived instead of being read from a // core file note. value.SetFromMemoryData( `````````` </details> https://github.com/llvm/llvm-project/pull/71175 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
