llvm / llvm/llvm-project

[BOLT] `.debug_loclists` expressions keep stale `BaseTypeRef` operands after rewriting

Open
#214,721 1 comment 0 reactions 0 assignees View on GitHub
BOLT
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

## Description

When BOLT rewrites `.debug_loclists`, it translates the PC ranges of every location list entry but copies each entry's DWARF expression **byte for byte**. Such an expression may carry an operand that points into `.debug_info`: `Encoding::BaseTypeRef` - the type operand of `DW_OP_regval_type`, `DW_OP_convert`, `DW_OP_deref_type`, `DW_OP_const_type`, `DW_OP_xderef_type`, `DW_OP_reinterpret` - is a CU-relative DIE offset. BOLT rebuilds `.debug_info` in the same run, so the referenced `DW_TAG_base_type` DIE ends up at a different offset, while the copied expression still holds the input one.

## DWARF 5 references

*DWARF Debugging Information Format, Version 5* (February 13, 2017):

- **§2.5.1.1–§2.5.1.3** - the type operand of `DW_OP_const_type`, `DW_OP_regval_type`, `DW_OP_deref_type` and `DW_OP_xderef_type` is *"the offset of a debugging information entry in the current compilation unit, which must be a* `DW_TAG_base_type` *entry"*. A stale offset can land on an unrelated DIE.
- **§2.5.1.6** - `DW_OP_convert` / `DW_OP_reinterpret` take the same offset "or value 0 which represents the generic type"; only a non-zero operand must resolve to a `DW_TAG_base_type` entry, so a `0` is left as is, here and in `cloneExpression()`.
- **§2.6.2 → §2.6.1** - a *bounded* location list entry holds a ULEB length followed by a location description, and §2.6.1.1.2 defines that as *"a non-empty DWARF expression (see Section 2.5)"*. The rules above therefore hold in `.debug_loclists` exactly as they do for an `exprloc` attribute.

## Environment
- **BOLT:** `llvm-bolt` (`BOLT version:` bd6adfedc776c07caf158e59367d9c246c933510), correlated fixes:
- https://github.com/llvm/llvm-project/issues/213758 - descriptors for `DW_OP_const_type`, `DW_OP_deref_type`, `DW_OP_xderef_type`, `DW_OP_reinterpret`, plus handling of a `BaseTypeRef` that has operands after it. Without them the prescan bails out and those expressions keep a stale reference. invalid base_type ref will be reduced but still > 0.
- https://github.com/llvm/llvm-project/issues/208082 - widening and back-patching the length prefix of a nested `DW_OP_entry_value` block. Without it, widening a reference inside such a block leaves the length stale, i.e. this patch would move that defect into `.debug_loclists`

- **Target arch:** AArch64 and x86_64
- **Compiler:** g++ (GCC) 14.2.0
- **`llvm-dwarfdump`:** built from the same tree as the `llvm-bolt` above. This matters for the dumps below: #213758 adds the `DW_OP_const_type` descriptor to the shared expression parser, so a dumper without it prints `` for the `scaled` expression instead of decoding it.
- **Binutils:** GNU readelf (GNU Binutils) 2.46 - for the single `--debug-dump=loc` check under *Verification*. `llvm-dwarfdump` accepts a stale `DW_OP_entry_value` length prefix without a word, so readelf is what shows that this patch does not reintroduce #208082 inside `.debug_loclists`.

## Reproducer

`main.cpp`:

```cpp
volatile double sink;
volatile long isink;

__attribute__((noinline)) double callee(double a, double b) {
sink = a + b;
return sink;
}

__attribute__((noinline)) int caller(double x) { return (int)callee(x, 1.0); }

__attribute__((noinline)) int use(int v) {
isink = v;
return v;
}

__attribute__((noinline)) double typed_const(double base, int n) {
double scaled = base * 2.5;
use(n);
return scaled + sink;
}

__attribute__((noinline)) double converted(int n) {
double d = (double)n;
float f = (float)(d * 2.0);
return callee(d, f) + f;
}

int main(int argc, char **) {
return caller(1.0) + (int)typed_const(argc, argc) + (int)converted(argc);
}
```

```
$ g++ -gdwarf-5 -O3 -gz=none main.cpp -o main
$ llvm-bolt main -o main.bolt --update-debug-sections
```

`llvm-dwarfdump` reports the matches in alphabetical order, so each pair of blocks below reads `f` (`DW_OP_convert`, a `float` narrowed from double arithmetic), then `scaled` (`DW_OP_const_type`, the typed constant `2.5`), then `x` (`DW_OP_regval_type`, an entry value). The `DW_AT_type` line printed next to each expression shows where the referenced base type DIE actually moved.

```
$ llvm-dwarfdump --debug-info --name x --name scaled --name f main | grep -E 'DW_AT_name|DW_AT_type|DW_AT_location|DW_OP_'
DW_AT_name ("f")
DW_AT_type (0x00000031 "float")
DW_AT_location (0x00000021:
[0x00000000004011b8, 0x00000000004011c4): DW_OP_regval_type XMM0 (0x0000002a) "double", DW_OP_regval_type XMM0 (0x0000002a) "double", DW_OP_plus, DW_OP_convert (0x00000031) "float", DW_OP_stack_value
[0x00000000004011c4, 0x00000000004011c8): DW_OP_reg18 XMM1
[0x00000000004011c8, 0x00000000004011cc): DW_OP_regval_type XMM0 (0x0000002a) "double", DW_OP_regval_type XMM0 (0x0000002a) "double", DW_OP_plus, DW_OP_convert (0x00000031) "float", DW_OP_stack_value)
DW_AT_name ("scaled")
DW_AT_type (0x0000002a "double")
DW_AT_location (0x0000006d:
[0x0000000000401198, 0x00000000004011a9): DW_OP_reg17 XMM0
[0x00000000004011a9, 0x00000000004011aa): DW_OP_entry_value(DW_OP_regval_type XMM0 (0x0000002a) "double"), DW_OP_const_type (0x0000002a) "double" 0x8 0x00 0x00 0x00 0x00 0x00 0x00 0x04 0x40, DW_OP_mul, DW_OP_stack_value)
DW_AT_name ("x")
DW_AT_type (0x0000002a "double")
DW_AT_location (0x00000096:
[0x0000000000401160, 0x000000000040116c): DW_OP_reg17 XMM0
[0x000000000040116c, 0x0000000000401172): DW_OP_entry_value(DW_OP_regval_type XMM0 (0x0000002a) "double"), DW_OP_stack_value)

$ llvm-dwarfdump --debug-info --name x --name scaled --name f main.bolt | grep -E 'DW_AT_name|DW_AT_type|DW_AT_location|DW_OP_'
DW_AT_name ("f")
DW_AT_type (0x0000003a "float")
DW_AT_location (indexed (0x1) loclist = 0x0000002a:
[0x00000000004011b8, 0x00000000004011c4): DW_OP_regval_type XMM0 , DW_OP_regval_type XMM0 , DW_OP_plus, DW_OP_convert , DW_OP_stack_value
[0x00000000004011c4, 0x00000000004011c8): DW_OP_reg18 XMM1
[0x00000000004011c8, 0x00000000004011cc): DW_OP_regval_type XMM0 , DW_OP_regval_type XMM0 , DW_OP_plus, DW_OP_convert , DW_OP_stack_value)
DW_AT_name ("scaled")
DW_AT_type (0x00000033 "double")
DW_AT_location (indexed (0x3) loclist = 0x00000060:
[0x0000000000401198, 0x00000000004011a9): DW_OP_reg17 XMM0
[0x00000000004011a9, 0x00000000004011aa): DW_OP_entry_value(DW_OP_regval_type XMM0 ), DW_OP_const_type 0x8 0x00 0x00 0x00 0x00 0x00 0x00 0x04 0x40, DW_OP_mul, DW_OP_stack_value)
DW_AT_name ("x")
DW_AT_type (0x00000033 "double")
DW_AT_location (indexed (0x4) loclist = 0x0000007e:
[0x0000000000401160, 0x000000000040116c): DW_OP_reg17 XMM0
[0x000000000040116c, 0x0000000000401172): DW_OP_entry_value(DW_OP_regval_type XMM0 ), DW_OP_stack_value)
```

In all three the PC ranges were translated and `DW_AT_type` followed its DIE - `double` from `0x2a` to `0x33`, `float` from `0x31` to `0x3a` - while every reference *inside* an expression kept the input offset and now lands in an unrelated DIE.

Detection - the number of `.debug_info` lines still carrying a stale operand:

```
$ llvm-dwarfdump --debug-info main | grep -c 'invalid base_type ref'
0

$ llvm-dwarfdump --debug-info main.bolt | grep -c 'invalid base_type ref'
6
```

`llvm-dwarfdump --verify` reports the same DIEs, as `error: DIE contains invalid DWARF expression`. It is not used as the criterion here because on this binary it also reports an unrelated class of file-index errors (https://github.com/llvm/llvm-project/issues/209184), so it ends with `Errors detected.` even once the operands are correct.

## Root cause

`.debug_info` expressions are relocated in two stages. `DIEBuilder::cloneExpression()` emits each `BaseTypeRef` as a fixed-width 4-byte ULEB during cloning (`INIT`, still holding the input offset), the expression is queued in `LocWithReferencesToProcess`, and `DIEBuilder::updateReferences()` re-runs `cloneExpression()` in the `PATCH` stage once DIE offsets are known:

`bolt/lib/Core/DIEBuilder.cpp`
```cpp
// Handling references in location expressions.
for (LocWithReference &LocExpr : getState().LocWithReferencesToProcess) {
...
cloneExpression(Data, Expr, LocExpr.U, Buffer, CloneExpressionStage::PATCH);
```

The location-list path has no equivalent. `translateInputToOutputLocationList()` carries `Entry.Expr` through unchanged, and `writeDWARF5LocList()`, called from `DebugLoclistWriter::addList()`, serialises it verbatim:

`bolt/lib/Core/DebugData.cpp`
```cpp
auto writeExpression = [&](uint32_t Index) -> void {
const DebugLocationEntry &Entry = LocList[Index];
encodeULEB128(Entry.Expr.size(), LocBodyStream);
LocBodyStream << StringRef(
reinterpret_cast(Entry.Expr.data()), Entry.Expr.size());
};
```

## Proposed Fix

Three parts mirroring the `.debug_info` design: widen every reference to a fixed-width 4-byte ULEB before the expression is serialised, remember where the expression landed, then rewrite the references in place once DIE offsets are final. Fixed width means the patch never shifts a byte, so the entry's expression-length ULEB stays valid.

### 1. Expose the walk `cloneExpression()` already performs

No second expression rewriter is needed: `cloneExpression()` already widens `BaseTypeRef` operands. It only needs to be callable on a raw expression buffer - a thin public wrapper, with `CloneExpressionStage` moved to the public section:

```diff
--- a/bolt/include/bolt/Core/DIEBuilder.h
+++ b/bolt/include/bolt/Core/DIEBuilder.h
@@ -80,6 +80,24 @@

enum class ProcessingType { DWARF4TUs, DWARF5TUs, CUs };

+ enum class CloneExpressionStage { INIT, PATCH };
+
+ /// Re-encodes every Encoding::BaseTypeRef operand of the location expression
+ /// in \p ExprBytes as a fixed-width 4-byte ULEB: in the INIT stage holding
+ /// the input offset, in the PATCH stage the final offset of the referenced
+ /// DIE. The emitted size is the same in both stages, so a PATCH pass can
+ /// overwrite an already serialised expression in place.
+ ///
+ /// Used by the .debug_loclists path, whose expressions are serialised long
+ /// before DIE offsets exist: they are widened when the location list is built
+ /// and patched once DIEBuilder::finish() has assigned the final offsets.
+ ///
+ /// Returns false and leaves \p ExprBytes untouched when the expression holds
+ /// no such reference.
+ bool relocateLocExprBaseTypeRefs(DWARFUnit &U,
+ SmallVectorImpl &ExprBytes,
+ CloneExpressionStage Stage);
+
private:
/// Contains information so that we we can update references in locexpr after
/// we calculated all the final DIE offsets.
@@ -168,7 +186,6 @@
const DWARFAbbreviationDeclaration::AttributeSpec AttrSpec,
const DWARFFormValue &Val);

- enum class CloneExpressionStage { INIT, PATCH };
/// Clone an attribute in expression format. \p OutputBuffer will hold the
/// output content.
/// Returns true if Expression contains a reference.
```

```diff
--- a/bolt/lib/Core/DIEBuilder.cpp
+++ b/bolt/lib/Core/DIEBuilder.cpp
@@ -855,6 +855,34 @@
return DoesContainReference;
}

+bool DIEBuilder::relocateLocExprBaseTypeRefs(
+ DWARFUnit &U, SmallVectorImpl &ExprBytes,
+ CloneExpressionStage Stage) {
+ if (ExprBytes.empty())
+ return false;
+
+ // cloneExpression() reads from Data while writing to a separate buffer, so
+ // the input has to be kept apart from the output.
+ const SmallVector Input(ExprBytes.begin(), ExprBytes.end());
+ DataExtractor Data(Input, U.isLittleEndian());
+ DWARFExpression Expr(Data, U.getAddressByteSize(), U.getFormParams().Format);
+
+ // cloneExpression() stops at the first operation it cannot decode, so its
+ // output would be a shortened expression. Unlike a DIE attribute, a location
+ // list entry can keep the bytes it came in with, which is preferable to
+ // dropping the tail of the expression.
+ for (const DWARFExpression::Operation &Op : Expr)
+ if (Op.isError())
+ return false;
+
+ SmallVector Output;
+ if (!cloneExpression(Data, Expr, U, Output, Stage))
+ return false;
+
+ ExprBytes.assign(Output.begin(), Output.end());
+ return true;
+}
+
void DIEBuilder::cloneBlockAttribute(
DIE &Die, DWARFUnit &U,
const DWARFAbbreviationDeclaration::AttributeSpec AttrSpec,
```

### 2. Widen and record at serialisation time

`writeExpression()` in `writeDWARF5LocList()` is the single place every DWARF 5 location expression is serialised, and `LocBodyBuffer.size()` before the write gives the body position. `writeDWARF5LocList()` is a free function, so the record vector is threaded through as a parameter from `DebugLoclistWriter::addList()`, which owns it. The record keeps the **input** bytes, not the emitted ones, so the patch pass resolves the references from exactly the bytes the `.debug_info` path sees. This matters: `cloneExpression()` leaves an unresolvable reference as `0`, so a patch pass reading the *emitted* bytes would resolve that `0` to the unit DIE and produce a valid-looking reference to the wrong DIE.

```diff
--- a/bolt/lib/Core/DebugData.cpp
+++ b/bolt/lib/Core/DebugData.cpp
@@ -679,7 +679,8 @@
DebugBufferVector &LocBodyBuffer,
std::vector &RelativeLocListOffsets,
DWARFUnit &CU,
- raw_svector_ostream &LocBodyStream) {
+ raw_svector_ostream &LocBodyStream,
+ std::vector &BaseTypeRefExprs) {

replaceLocValbyForm(DIEBldr, Die, AttrInfo, dwarf::DW_FORM_loclistx,
NumberOfEntries);
@@ -693,9 +694,19 @@

auto writeExpression = [&](uint32_t Index) -> void {
const DebugLocationEntry &Entry = LocList[Index];
- encodeULEB128(Entry.Expr.size(), LocBodyStream);
- LocBodyStream << StringRef(
- reinterpret_cast(Entry.Expr.data()), Entry.Expr.size());
+ // A BaseTypeRef operand holds the offset of a DIE that has no final offset
+ // yet, so widen every one to a fixed-width 4-byte ULEB now and record where
+ // the expression landed; applyBaseTypeRefs() writes the offsets in once
+ // DIEBuilder::finish() knows them. Widening here keeps the patch from
+ // shifting any byte, so the length prefix written below stays valid.
+ SmallVector Expr(Entry.Expr);
+ const bool HasBaseTypeRef = DIEBldr.relocateLocExprBaseTypeRefs(
+ CU, Expr, DIEBuilder::CloneExpressionStage::INIT);
+ encodeULEB128(Expr.size(), LocBodyStream);
+ if (HasBaseTypeRef)
+ BaseTypeRefExprs.push_back({LocBodyBuffer.size(), Entry.Expr});
+ LocBodyStream << StringRef(reinterpret_cast(Expr.data()),
+ Expr.size());
};
for (unsigned I = 0; I < LocList.size();) {
if (emitWithBase;

+/// A serialised location expression holding BaseTypeRef operands that still
+/// wait for the final DIE offsets. The INPUT expression is kept rather than the
+/// emitted one, so the patch pass resolves the references from the same bytes
+/// the .debug_info path does. BodyOffset is relative to the start of this CU's
+/// location-list body.
+struct BaseTypeRefExpr {
+ uint64_t BodyOffset;
+ SmallVector InputExpr;
+};
+
/// References a row in a DWARFDebugLine::LineTable by the DWARF
/// Context index of the DWARF Compile Unit that owns the Line Table and the row
/// index. This is tied to our IR during disassembly so that we can later update
@@ -662,6 +678,8 @@
std::unique_ptr LocBodyStream;
std::vector RelativeLocListOffsets;
uint32_t NumberOfEntries{0};
+ /// Serialised expressions awaiting the final DIE offsets, in body order.
+ std::vector BaseTypeRefExprs;
};

/// Abstract interface for classes that apply modifications to a binary string.
```

### 3. Patch once DIE offsets are final

By the time this runs, `finalizeDWARF5()` has already assembled the writer's `LocBuffer` as header, then a loclist offset array of `NumberOfEntries` 4-byte entries, then the body - so the body starts at a position the writer can recompute. Both stages widen identically, so the patched expression occupies exactly the bytes the emitted one does:

```diff
--- a/bolt/include/bolt/Core/DebugData.h
+++ b/bolt/include/bolt/Core/DebugData.h
@@ -633,6 +643,12 @@
/// Writes out locations in to a local buffer and applies debug info patches.
void finalize(DIEBuilder &DIEBldr, DIE &Die) override;

+ /// Writes the final offset of every referenced DIE into the BaseTypeRef
+ /// operands of the location expressions serialised by this writer. Must run
+ /// after DIEBuilder::finish() has assigned the final DIE offsets, and while
+ /// \p Bldr -- the builder that owns those DIEs -- is still alive.
+ void applyBaseTypeRefs(DIEBuilder &Bldr);
+
/// Returns CU ID.
/// For Skeleton CU it is a CU Offset.
/// For DWO CU it is a DWO ID.
```

```diff
--- a/bolt/lib/Core/DebugData.cpp
+++ b/bolt/lib/Core/DebugData.cpp
@@ -784,6 +795,29 @@
clearList(*LocBodyBuffer);
}

+void DebugLoclistWriter::applyBaseTypeRefs(DIEBuilder &Bldr) {
+ if (BaseTypeRefExprs.empty())
+ return;
+
+ // finalizeDWARF5() has already assembled LocBuffer as header, then a loclist
+ // offset array of NumberOfEntries 4-byte entries, then the body.
+ const uint64_t BodyBase =
+ getDWARF5RngListLocListHeaderSize() +
+ static_cast(NumberOfEntries) * sizeof(uint32_t);
+ for (const BaseTypeRefExpr &E : BaseTypeRefExprs) {
+ SmallVector Expr(E.InputExpr);
+ if (!Bldr.relocateLocExprBaseTypeRefs(
+ CU, Expr, DIEBuilder::CloneExpressionStage::PATCH))
+ continue;
+ // Both stages widen identically, so the patched expression occupies exactly
+ // the bytes the emitted one does.
+ assert(BodyBase + E.BodyOffset + Expr.size() <= LocBuffer->size() &&
+ "patched location expression is out of the loclist buffer");
+ llvm::copy(Expr, LocBuffer->begin() + BodyBase + E.BodyOffset);
+ }
+ clearList(BaseTypeRefExprs);
+}
+
void DebugLoclistWriter::finalize(DIEBuilder &DIEBldr, DIE &Die) {
if (DwarfVersion >= 5)
finalizeDWARF5(DIEBldr, Die);
```

Called from `finalizeCompileUnits()` immediately after `DIEBlder.finish()`:

```diff
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -2009,6 +2009,17 @@
}
DIEBlder.setUnitOffsetBases(CUOffset);
DIEBlder.finish();
+ // DIE offsets are final now, so the BaseTypeRef operands of the location
+ // expressions can be filled in. It has to happen here rather than in
+ // makeFinalLocListsSection(): the DIEs whose offsets are needed belong to
+ // this bucket's builder, which is released as soon as the bucket is merged.
+ for (DWARFUnit *CU : CUs) {
+ auto LocIt = LocListWritersByCU.find(CU->getOffset());
+ if (LocIt == LocListWritersByCU.end())
+ continue;
+ if (auto *LocListWriter = dyn_cast(LocIt->second.get()))
+ LocListWriter->applyBaseTypeRefs(DIEBlder);
+ }
{
std::lock_guard Lock(DebugNamesUpdateMutex);
DIEBlder.updateDebugNamesTable();
```

## Verification

Applied on `bd6adfedc776` together with the two correlated fixes listed under *Environment*:

```
$ llvm-dwarfdump --debug-info --name x --name scaled --name f main.bolt | grep -E 'DW_AT_name|DW_AT_type|DW_AT_location|DW_OP_'
DW_AT_name ("f")
DW_AT_type (0x0000003a "float")
DW_AT_location (indexed (0x1) loclist = 0x0000002a:
[0x00000000004011b8, 0x00000000004011c4): DW_OP_regval_type XMM0 (0x00000033) "double", DW_OP_regval_type XMM0 (0x00000033) "double", DW_OP_plus, DW_OP_convert (0x0000003a) "float", DW_OP_stack_value
[0x00000000004011c4, 0x00000000004011c8): DW_OP_reg18 XMM1
[0x00000000004011c8, 0x00000000004011cc): DW_OP_regval_type XMM0 (0x00000033) "double", DW_OP_regval_type XMM0 (0x00000033) "double", DW_OP_plus, DW_OP_convert (0x0000003a) "float", DW_OP_stack_value)
DW_AT_name ("scaled")
DW_AT_type (0x00000033 "double")
DW_AT_location (indexed (0x3) loclist = 0x00000078:
[0x0000000000401198, 0x00000000004011a9): DW_OP_reg17 XMM0
[0x00000000004011a9, 0x00000000004011aa): DW_OP_entry_value(DW_OP_regval_type XMM0 (0x00000033) "double"), DW_OP_const_type (0x00000033) "double" 0x8 0x00 0x00 0x00 0x00 0x00 0x00 0x04 0x40, DW_OP_mul, DW_OP_stack_value)
DW_AT_name ("x")
DW_AT_type (0x00000033 "double")
DW_AT_location (indexed (0x4) loclist = 0x0000009f:
[0x0000000000401160, 0x000000000040116c): DW_OP_reg17 XMM0
[0x000000000040116c, 0x0000000000401172): DW_OP_entry_value(DW_OP_regval_type XMM0 (0x00000033) "double"), DW_OP_stack_value)
```

Every operand now resolves to the DIE its own `DW_AT_type` points at - `0x33` is the `DW_TAG_base_type` with `DW_AT_name ("double")` and `DW_AT_byte_size (0x08)`, `0x3a` is `("float")` with `DW_AT_byte_size (0x04)` - including the operands nested inside `DW_OP_entry_value`.

```
$ llvm-dwarfdump --debug-info main.bolt | grep -c 'invalid base_type ref'
0

$ readelf --debug-dump=loc main.bolt | grep -c 'whilst reading LEB'
0
```

The `readelf` check is what confirms the widening did not desynchronise any enclosing `DW_OP_entry_value` length: `llvm-dwarfdump` accepts a stale length prefix silently, so it reports nothing for that failure mode.

Contributor guide

Open the contributing guide

Research direction

Start with cloneExpression() and the reference-processing state in bolt/lib/Core/DIEBuilder.cpp, then trace translateInputToOutputLocationList() and writeDWARF5LocList() in bolt/lib/Core/DebugData.cpp. Compare the .debug_info relocation path with the .debug_loclists path and verify the reproducer using llvm-dwarfdump and GNU readelf. Done means rewritten loclists retain valid BaseTypeRef operands and the stale-reference count is zero without reintroducing entry_value length errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.