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 <jo...@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 <jo...@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: _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits