[Lldb-commits] [lldb] [lldb] Add LLDB_ASSERT_OR_RETURN macro and make use of it (PR #71175)

David Spickett via lldb-commits lldb-commits at lists.llvm.org
Mon Nov 6 03:36:51 PST 2023


https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/71175

>From e14805de814d60f15a2671dd739caf10d19f2ea6 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Fri, 3 Nov 2023 10:58:15 +0000
Subject: [PATCH] [lldb] Replace some register handling asserts with lldbassert

These asserts are important if they fail, but using assert meant
there was no checking in a release build.

lldbassert does the check and reports any failure but doesn't
crash the debugger.

I know we're not supposed to be adding new lldbasserts, but it's
exactly what's needed here.
---
 .../Linux/NativeRegisterContextLinux_arm.cpp  | 12 +++--
 .../NativeRegisterContextLinux_arm64.cpp      | 52 ++++++++++++-------
 ...NativeRegisterContextLinux_loongarch64.cpp |  9 ++--
 .../NativeRegisterContextLinux_riscv64.cpp    | 10 ++--
 .../RegisterContextPOSIXCore_arm64.cpp        | 16 +++---
 5 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
index 5ad2f7a8e9455b1..47c91a2cc8fb5c6 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);
+  lldbassert(fpr_offset < sizeof m_fpr &&
+             "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);
+    lldbassert(fpr_offset < sizeof m_fpr &&
+               "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,8 @@ 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");
+  lldbassert(offset % 4 == 0 &&
+             "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 +816,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");
+  lldbassert(offset % 4 == 0 &&
+             "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..9e08f31034fbde2 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -242,7 +242,7 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info,
       return error;
 
     offset = reg_info->byte_offset;
-    assert(offset < GetGPRSize());
+    lldbassert(offset < GetGPRSize() && "Invalid GPR register read offset");
     src = (uint8_t *)GetGPRBuffer() + offset;
 
   } else if (IsFPR(reg)) {
@@ -253,7 +253,7 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info,
         return error;
 
       offset = CalculateFprOffset(reg_info);
-      assert(offset < GetFPRSize());
+      lldbassert(offset < GetFPRSize() && "Invalid FPR register read offset");
       src = (uint8_t *)GetFPRBuffer() + offset;
     } else {
       // SVE or SSVE enabled, we will read and cache SVE ptrace data.
@@ -288,7 +288,8 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info,
         offset = CalculateSVEOffset(GetRegisterInfoAtIndex(sve_reg_num));
       }
 
-      assert(offset < GetSVEBufferSize());
+      lldbassert(offset < GetSVEBufferSize() &&
+                 "Invalid SVE FPR register read offset");
       src = (uint8_t *)GetSVEBuffer() + offset;
     }
   } else if (IsTLS(reg)) {
@@ -297,7 +298,8 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info,
       return error;
 
     offset = reg_info->byte_offset - GetRegisterInfo().GetTLSOffset();
-    assert(offset < GetTLSBufferSize());
+    lldbassert(offset < GetTLSBufferSize() &&
+               "Invalid TLS register read offset");
     src = (uint8_t *)GetTLSBuffer() + offset;
   } else if (IsSVE(reg)) {
     if (m_sve_state == SVEState::Disabled || m_sve_state == SVEState::Unknown)
@@ -321,13 +323,15 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info,
 
         if (GetRegisterInfo().IsSVEZReg(reg)) {
           offset = CalculateSVEOffset(reg_info);
-          assert(offset < GetSVEBufferSize());
+          lldbassert(offset < GetSVEBufferSize() &&
+                     "Invalid SVE register read offset");
           ::memcpy(sve_reg_non_live.data(), (uint8_t *)GetSVEBuffer() + offset,
                    16);
         }
       } else {
         offset = CalculateSVEOffset(reg_info);
-        assert(offset < GetSVEBufferSize());
+        lldbassert(offset < GetSVEBufferSize() &&
+                   "Invalid SVE register read offset");
         src = (uint8_t *)GetSVEBuffer() + offset;
       }
     }
@@ -337,7 +341,8 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info,
       return error;
 
     offset = reg_info->byte_offset - GetRegisterInfo().GetPAuthOffset();
-    assert(offset < GetPACMaskSize());
+    lldbassert(offset < GetPACMaskSize() &&
+               "Invalid PAuth register read offset");
     src = (uint8_t *)GetPACMask() + offset;
   } else if (IsMTE(reg)) {
     error = ReadMTEControl();
@@ -345,7 +350,8 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info,
       return error;
 
     offset = reg_info->byte_offset - GetRegisterInfo().GetMTEOffset();
-    assert(offset < GetMTEControlSize());
+    lldbassert(offset < GetMTEControlSize() &&
+               "Invalid MTE register read offset");
     src = (uint8_t *)GetMTEControl() + offset;
   } else if (IsSME(reg)) {
     if (GetRegisterInfo().IsSMERegZA(reg)) {
@@ -391,7 +397,8 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info,
       ReadSMEControl();
 
       offset = reg_info->byte_offset - GetRegisterInfo().GetSMEOffset();
-      assert(offset < GetSMEPseudoBufferSize());
+      lldbassert(offset < GetSMEPseudoBufferSize() &&
+                 "Invalid SME register read offset");
       src = (uint8_t *)GetSMEPseudoBuffer() + offset;
     }
   } else
@@ -427,7 +434,8 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
     if (error.Fail())
       return error;
 
-    assert(reg_info->byte_offset < GetGPRSize());
+    lldbassert(reg_info->byte_offset < GetGPRSize() &&
+               "Invalid GPR register write offset");
     dst = (uint8_t *)GetGPRBuffer() + reg_info->byte_offset;
     ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
 
@@ -440,7 +448,7 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
         return error;
 
       offset = CalculateFprOffset(reg_info);
-      assert(offset < GetFPRSize());
+      lldbassert(offset < GetFPRSize() && "Invalid FPR register write offset");
       dst = (uint8_t *)GetFPRBuffer() + offset;
       ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
 
@@ -476,7 +484,8 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
         offset = CalculateSVEOffset(GetRegisterInfoAtIndex(sve_reg_num));
       }
 
-      assert(offset < GetSVEBufferSize());
+      lldbassert(offset < GetSVEBufferSize() &&
+                 "Invalid SVE register write offset");
       dst = (uint8_t *)GetSVEBuffer() + offset;
       ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
       return WriteAllSVE();
@@ -541,7 +550,8 @@ 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());
+          lldbassert(offset < GetSVEBufferSize() &&
+                     "Invalid SVE register write offset");
           dst = (uint8_t *)GetSVEBuffer() + offset;
           ::memcpy(dst, reg_value.GetBytes(), 16);
 
@@ -550,7 +560,8 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
           return Status("SVE state change operation not supported");
       } else {
         offset = CalculateSVEOffset(reg_info);
-        assert(offset < GetSVEBufferSize());
+        lldbassert(offset < GetSVEBufferSize() &&
+                   "Invalid SVE register write offset");
         dst = (uint8_t *)GetSVEBuffer() + offset;
         ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
         return WriteAllSVE();
@@ -562,7 +573,8 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
       return error;
 
     offset = reg_info->byte_offset - GetRegisterInfo().GetMTEOffset();
-    assert(offset < GetMTEControlSize());
+    lldbassert(offset < GetMTEControlSize() &&
+               "Invalid MTE register write offset");
     dst = (uint8_t *)GetMTEControl() + offset;
     ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
 
@@ -573,7 +585,8 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
       return error;
 
     offset = reg_info->byte_offset - GetRegisterInfo().GetTLSOffset();
-    assert(offset < GetTLSBufferSize());
+    lldbassert(offset < GetTLSBufferSize() &&
+               "Invalid MTE register write offset");
     dst = (uint8_t *)GetTLSBuffer() + offset;
     ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
 
@@ -758,11 +771,11 @@ Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
   // For more information on this, look up the uses of the relevant NT_ARM_
   // constants and the functions vec_set_vector_length, sve_set_common and
   // za_set in the Linux Kernel.
-
   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());
+    lldbassert(m_za_header.size <= GetZABufferSize() &&
+               "Unexpected ZA header size");
     dst = AddSavedRegisters(dst, RegisterSetType::SME, GetZABuffer(),
                             m_za_header.size);
   }
@@ -778,7 +791,8 @@ Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
   }
 
   if ((m_sve_state == SVEState::Streaming) && GetRegisterInfo().IsZAPresent()) {
-    assert(m_za_header.size <= GetZABufferSize());
+    lldbassert(m_za_header.size <= GetZABufferSize() &&
+               "Unexpected ZA Header Size");
     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..3f9d120cf2ba13e 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp
@@ -113,7 +113,7 @@ Status NativeRegisterContextLinux_loongarch64::ReadRegister(
       return error;
 
     offset = reg_info->byte_offset;
-    assert(offset < GetGPRSize());
+    lldbassert(offset < GetGPRSize() && "Invalid GPR register read offset");
     src = (uint8_t *)GetGPRBuffer() + offset;
 
   } else if (IsFPR(reg)) {
@@ -122,7 +122,7 @@ Status NativeRegisterContextLinux_loongarch64::ReadRegister(
       return error;
 
     offset = CalculateFprOffset(reg_info);
-    assert(offset < GetFPRSize());
+    lldbassert(offset < GetFPRSize() && "Invalid FPR register read offset");
     src = (uint8_t *)GetFPRBuffer() + offset;
   } else
     return Status("failed - register wasn't recognized to be a GPR or an FPR, "
@@ -156,7 +156,8 @@ Status NativeRegisterContextLinux_loongarch64::WriteRegister(
     if (error.Fail())
       return error;
 
-    assert(reg_info->byte_offset < GetGPRSize());
+    lldbassert(reg_info->byte_offset < GetGPRSize() &&
+               "Invalid GPR register write offset");
     dst = (uint8_t *)GetGPRBuffer() + reg_info->byte_offset;
     ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
 
@@ -167,7 +168,7 @@ Status NativeRegisterContextLinux_loongarch64::WriteRegister(
       return error;
 
     offset = CalculateFprOffset(reg_info);
-    assert(offset < GetFPRSize());
+    lldbassert(offset < GetFPRSize() && "Invalid FPR register write offset");
     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..9222cb4b0658ab1 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());
+    lldbassert(offset < GetGPRSize() && "Invalid GPR register read offset");
     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());
+    lldbassert(offset < GetFPRSize() && "Invalid FPR register read offset");
     src = (uint8_t *)GetFPRBuffer() + offset;
   } else
     return Status("failed - register wasn't recognized to be a GPR or an FPR, "
@@ -168,7 +169,8 @@ Status NativeRegisterContextLinux_riscv64::WriteRegister(
     if (error.Fail())
       return error;
 
-    assert(reg_info->byte_offset < GetGPRSize());
+    lldbassert(reg_info->byte_offset < GetGPRSize() &&
+               "Invalid GPR register write offset");
     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());
+    lldbassert(offset < GetFPRSize() && "Invalid FPR register write offset");
     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..9596cb6dff9b751 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());
+      lldbassert(sve_reg_num != LLDB_INVALID_REGNUM);
+      lldbassert(offset < m_sve_data.GetByteSize());
       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());
+        lldbassert(offset < m_sve_data.GetByteSize());
         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());
+      lldbassert(offset < m_sve_data.GetByteSize());
       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());
+    lldbassert(offset < m_pac_data.GetByteSize());
     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());
+    lldbassert(offset < m_tls_data.GetByteSize());
     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());
+    lldbassert(offset < m_mte_data.GetByteSize());
     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));
+      lldbassert(offset < sizeof(m_sme_pseudo_regs));
       // Host endian since these values are derived instead of being read from a
       // core file note.
       value.SetFromMemoryData(



More information about the lldb-commits mailing list