[Lldb-commits] [lldb] [lldb] Add missing operations to GetOpcodeDataSize (PR #120163)

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Thu Jan 23 09:37:12 PST 2025


https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/120163

>From 21c9bbb7edab88caeb1b121dfe9090a6a3ef4404 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Wed, 22 Jan 2025 17:36:24 -0800
Subject: [PATCH 1/2] [lldb] Add missing operations to GetOpcodeDataSize

The improved error reporting in #120162 revealed that we were missing
opcodes in GetOpcodeDataSize. I changed the function to remove the
default case and switch over the enum type which will cause the compiler
to emit a warning if there are unhandled operations in the future.

rdar://139705570
---
 lldb/source/Expression/DWARFExpression.cpp | 75 ++++++++++++++++++----
 1 file changed, 62 insertions(+), 13 deletions(-)

diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 1d826e341e2c44..072cc74bbaafea 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -132,10 +132,35 @@ static llvm::Error ReadRegisterValueAsScalar(RegisterContext *reg_ctx,
 /// are made on the state of \p data after this call.
 static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
                                         const lldb::offset_t data_offset,
-                                        const uint8_t op,
+                                        const LocationAtom op,
                                         const DWARFUnit *dwarf_cu) {
   lldb::offset_t offset = data_offset;
   switch (op) {
+  // Only used in LLVM metadata.
+  case DW_OP_LLVM_fragment:
+  case DW_OP_LLVM_convert:
+  case DW_OP_LLVM_tag_offset:
+  case DW_OP_LLVM_entry_value:
+  case DW_OP_LLVM_implicit_pointer:
+  case DW_OP_LLVM_arg:
+  case DW_OP_LLVM_extract_bits_sext:
+  case DW_OP_LLVM_extract_bits_zext:
+    break;
+  // Vendor extensions:
+  case DW_OP_HP_is_value:
+  case DW_OP_HP_fltconst4:
+  case DW_OP_HP_fltconst8:
+  case DW_OP_HP_mod_range:
+  case DW_OP_HP_unmod_range:
+  case DW_OP_HP_tls:
+  case DW_OP_INTEL_bit_piece:
+  case DW_OP_WASM_location:
+  case DW_OP_WASM_location_int:
+  case DW_OP_APPLE_uninit:
+  case DW_OP_PGI_omp_thread_num:
+  case DW_OP_hi_user:
+    break;
+
   case DW_OP_addr:
   case DW_OP_call_ref: // 0x9a 1 address sized offset of DIE (DWARF3)
     return data.GetAddressByteSize();
@@ -246,6 +271,7 @@ static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
   case DW_OP_pick:        // 0x15 1 1-byte stack index
   case DW_OP_deref_size:  // 0x94 1 1-byte size of data retrieved
   case DW_OP_xderef_size: // 0x95 1 1-byte size of data retrieved
+  case DW_OP_deref_type:  // 0xa6 1 1-byte constant
     return 1;
 
   // Opcodes with a single 2 byte arguments
@@ -268,7 +294,6 @@ static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
     return 8;
 
   // All opcodes that have a single ULEB (signed or unsigned) argument
-  case DW_OP_addrx:           // 0xa1 1 ULEB128 index
   case DW_OP_constu:          // 0x10 1 ULEB128 constant
   case DW_OP_consts:          // 0x11 1 SLEB128 constant
   case DW_OP_plus_uconst:     // 0x23 1 ULEB128 addend
@@ -307,14 +332,20 @@ static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
   case DW_OP_regx:            // 0x90 1 ULEB128 register
   case DW_OP_fbreg:           // 0x91 1 SLEB128 offset
   case DW_OP_piece:           // 0x93 1 ULEB128 size of piece addressed
+  case DW_OP_convert:         // 0xa8 1 ULEB128 offset
+  case DW_OP_reinterpret:     // 0xa9 1 ULEB128 offset
+  case DW_OP_addrx:           // 0xa1 1 ULEB128 index
+  case DW_OP_constx:          // 0xa2 1 ULEB128 index
+  case DW_OP_xderef_type:     // 0xa7 1 ULEB128 index
   case DW_OP_GNU_addr_index:  // 0xfb 1 ULEB128 index
   case DW_OP_GNU_const_index: // 0xfc 1 ULEB128 index
     data.Skip_LEB128(&offset);
     return offset - data_offset;
 
   // All opcodes that have a 2 ULEB (signed or unsigned) arguments
-  case DW_OP_bregx:     // 0x92 2 ULEB128 register followed by SLEB128 offset
-  case DW_OP_bit_piece: // 0x9d ULEB128 bit size, ULEB128 bit offset (DWARF3);
+  case DW_OP_bregx:       // 0x92 2 ULEB128 register followed by SLEB128 offset
+  case DW_OP_bit_piece:   // 0x9d ULEB128 bit size, ULEB128 bit offset (DWARF3);
+  case DW_OP_regval_type: // 0xa5 ULEB128 + ULEB128
     data.Skip_LEB128(&offset);
     data.Skip_LEB128(&offset);
     return offset - data_offset;
@@ -327,6 +358,12 @@ static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
     return offset - data_offset;
   }
 
+  case DW_OP_implicit_pointer: // 0xa0 4 + LEB128
+  {
+    data.Skip_LEB128(&offset);
+    return 4 + offset - data_offset;
+  }
+
   case DW_OP_GNU_entry_value:
   case DW_OP_entry_value: // 0xa3 ULEB128 size + variable-length block
   {
@@ -334,20 +371,32 @@ static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
     return (offset - data_offset) + subexpr_len;
   }
 
-  default:
-    if (!dwarf_cu) {
-      return LLDB_INVALID_OFFSET;
-    }
+  case DW_OP_const_type: // 0xa4 ULEB128 + size + variable-length block
+  {
+    data.Skip_LEB128(&offset);
+    uint8_t length = data.GetU8(&offset);
+    return (offset - data_offset) + length;
+  }
+
+  case DW_OP_LLVM_user: // 0xe9: ULEB128 + variable length constant
+  {
+    uint64_t constants = data.GetULEB128(&offset);
+    return (offset - data_offset) + constants;
+  }
+  }
+
+  if (dwarf_cu)
     return dwarf_cu->GetSymbolFileDWARF().GetVendorDWARFOpcodeSize(
         data, data_offset, op);
-  }
+
+  return LLDB_INVALID_OFFSET;
 }
 
 llvm::Expected<lldb::addr_t>
 DWARFExpression::GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu) const {
   lldb::offset_t offset = 0;
   while (m_data.ValidOffset(offset)) {
-    const uint8_t op = m_data.GetU8(&offset);
+    const LocationAtom op = static_cast<LocationAtom>(m_data.GetU8(&offset));
 
     if (op == DW_OP_addr)
       return m_data.GetAddress(&offset);
@@ -376,7 +425,7 @@ bool DWARFExpression::Update_DW_OP_addr(const DWARFUnit *dwarf_cu,
                                         lldb::addr_t file_addr) {
   lldb::offset_t offset = 0;
   while (m_data.ValidOffset(offset)) {
-    const uint8_t op = m_data.GetU8(&offset);
+    const LocationAtom op = static_cast<LocationAtom>(m_data.GetU8(&offset));
 
     if (op == DW_OP_addr) {
       const uint32_t addr_byte_size = m_data.GetAddressByteSize();
@@ -434,7 +483,7 @@ bool DWARFExpression::ContainsThreadLocalStorage(
     const DWARFUnit *dwarf_cu) const {
   lldb::offset_t offset = 0;
   while (m_data.ValidOffset(offset)) {
-    const uint8_t op = m_data.GetU8(&offset);
+    const LocationAtom op = static_cast<LocationAtom>(m_data.GetU8(&offset));
 
     if (op == DW_OP_form_tls_address || op == DW_OP_GNU_push_tls_address)
       return true;
@@ -465,7 +514,7 @@ bool DWARFExpression::LinkThreadLocalStorage(
   lldb::addr_t const_value = 0;
   size_t const_byte_size = 0;
   while (m_data.ValidOffset(offset)) {
-    const uint8_t op = m_data.GetU8(&offset);
+    const LocationAtom op = static_cast<LocationAtom>(m_data.GetU8(&offset));
 
     bool decoded_data = false;
     switch (op) {

>From 19aeb5e27b9b849fd2d722c4cdd9e8c1b1cbffd9 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Thu, 23 Jan 2025 09:36:40 -0800
Subject: [PATCH 2/2] Fix DW_OP_implicit_pointer

---
 lldb/source/Expression/DWARFExpression.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 072cc74bbaafea..f48f3ab9307dd1 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -358,10 +358,11 @@ static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
     return offset - data_offset;
   }
 
-  case DW_OP_implicit_pointer: // 0xa0 4 + LEB128
+  case DW_OP_implicit_pointer: // 0xa0 4-byte (or 8-byte for DWARF 64) constant
+                               // + LEB128
   {
     data.Skip_LEB128(&offset);
-    return 4 + offset - data_offset;
+    return DWARFUnit::GetAddressByteSize(dwarf_cu) + offset - data_offset;
   }
 
   case DW_OP_GNU_entry_value:



More information about the lldb-commits mailing list