[LLDB] Finish implementing support for DW_FORM_data16

[walter-erquinigo]

This FORM already has support within LLDB to be parsed as a 16-byte
BLOCK, and all that is left to properly support it in the DWARFParser is
to add it to some enums.

With this, I can debug programs that use libstdc++.so.6.0.33 built with
GCC.

Sadly, I haven't figured out a good way to test this.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Walter Erquinigo (walter-erquinigo)

Changes

This FORM already has support within LLDB to be parsed as a 16-byte
BLOCK, and all that is left to properly support it in the DWARFParser is
to add it to some enums.

With this, I can debug programs that use libstdc++.so.6.0.33 built with
GCC.

Sadly, I haven't figured out a good way to test this.

---

Full diff: https://github.com/llvm/llvm-project/pull/106799.diff

1 Files Affected:

• (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp (+7)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
index e1f73f1997e369..e1fcbda469f16d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -306,6 +306,11 @@ bool DWARFFormValue::SkipValue(dw_form_t form,
       *offset_ptr += 8;
       return true;
 
+    // 16 byte values
+    case DW_FORM_data16:
+      *offset_ptr += 16;
+      return true;
+
     // signed or unsigned LEB 128 values
     case DW_FORM_addrx:
     case DW_FORM_loclistx:
@@ -578,6 +583,7 @@ bool DWARFFormValue::IsBlockForm(const dw_form_t form) {
   case DW_FORM_block1:
   case DW_FORM_block2:
   case DW_FORM_block4:
+  case DW_FORM_data16:
     return true;
   default:
     return false;
@@ -611,6 +617,7 @@ bool DWARFFormValue::FormIsSupported(dw_form_t form) {
     case DW_FORM_data2:
     case DW_FORM_data4:
     case DW_FORM_data8:
+    case DW_FORM_data16:
     case DW_FORM_string:
     case DW_FORM_block:
     case DW_FORM_block1:

[labath]

Sadly, I haven't figured out a good way to test this.

test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s tests various encodings of DW_AT_const_value.

It doesn't do DW_FORM_data16, most likely because it was not working at the time. You can add another case to the file to exercise this code path.

[clayborg]

We need the DWARFFormValue::ExtractValue to work. We spoke about pointing the DWARFFormValue::ValueType::data at the 16 bytes of data and have the DWARFFormValue::ValueType::value.uval contain 16 as the length of the data in DWARFFormValue::ValueType::data. That is the only thing missing.

[aainscow]

Vote. This is affecting the Ceph project.

[JDevlieghere]

LGTM with the other folks' concerns addressed.

[jasonmolenda]

Do we want IsBlockForm to return true for a DW_FORM_data16? I only ask because we don't do that for any of the other DW_FORM_data's.

[clayborg]

We might simply because this is represented the same way. Many systems don't support uint128_t types, so we can't just add one of those, so we need to use a byte pointer. Or we can add a bool DWARFFormValue::IsLargeConstant(const dw_form_t form) if we don't want to re-use the block stuff.

[walter-erquinigo]

I think that the block form will be the most convenient way to access this just because not all systems have uint128_t.

[clayborg]

We still need to add decoding of DW_FORM_data16 in DWARFFormValue::Extract...

[jasonmolenda]

We still need to add decoding of DW_FORM_data16 in DWARFFormValue::Extract...

It was added in 2018 by George Rimar as some DWARF5 support changes. In 2021 Jan Kratochvil added the entry to the g_form_sizes table along with some other fixes for DWARF5 too.

[walter-erquinigo]

Cancelled in favor of #113508 which adds tests