[BOLT] `--update-debug-sections`: `DW_AT_low_pc` of removed code is either zeroed or left pointing into the archived copy, decided by the attribute form
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
## Description
The same `DW_AT_low_pc`, on the same DIE, describing the same removed instruction, is **overwritten with** `0` when the attribute uses `DW_FORM_addr` and **left holding the input address** when it uses `DW_FORM_addrx`.
`0` is BOLT's marker for deleted code (`dwarf5-deleted-range.s` asserts it on `DW_AT_low_pc`), so the `addrx` result is the deviation, and the worse of the two: the kept address now denotes `.bolt.org.text`, the archived copy of the original bytes, so rather than being marked as gone the DIE points at code that can never execute again.
## Environment
- **BOLT:** `llvm-bolt` built from `bd6adfedc776c07caf158e59367d9c246c933510`
- **Tested with Compilers:** `clang++ 23.0.0git` (built from the same tree) and `g++ (GCC) 14.2.0`
- `llvm-dwarfdump`**:** built from the same tree.
## Reproducer
The reproducer needs a block BOLT will delete, and `asm goto` produces one: the only edge to the block lives inside the asm template, so the emitted machine code is a plain `nop`, BOLT's CFG reconstruction finds the block unreachable and removes it (`BOLT-INFO: UCE removed 1 blocks`). This is the same situation the two `*-deleted-range.s` tests create by hand-editing `je` into `jmp`, expressed in C++ instead of in an edited `.s` file.
```cpp
// main.cpp
volatile int sink;
__attribute__((noinline)) int opaque(int v) { return v * 3 + 1; }
static inline __attribute__((always_inline)) int inlined_helper(int a) {
int t = opaque(a);
return t + opaque(t);
}
__attribute__((noinline)) int live(int x) {
asm goto("nop" : : : : dead);
return x + 1;
dead: // a DW_TAG_label in the block UCE removes
sink = inlined_helper(x);
return sink;
}
int main(int argc, char **) { return live(argc) & 1; }
```
One source file, built several ways. The two halves of the defect are the two **form** branches, so showing both means making the same source emit `DW_AT_low_pc` in both forms. With clang that is `-gdwarf-5` against `-gdwarf-4`; with GCC it needs a split-DWARF build
```bash
# clang: -gdwarf-5 gives DW_FORM_addrx, -gdwarf-4 gives DW_FORM_addr, same file
clang -gdwarf-5 -O3 -gz=none main.cpp -o main5 -no-pie -Wl,-q
clang -gdwarf-4 -O3 -gz=none main.cpp -o main4 -no-pie -Wl,-q
llvm-bolt main5 -o main5.bolt --update-debug-sections
llvm-bolt main4 -o main4.bolt --update-debug-sections
# GCC: plain -gdwarf-5 gives DW_FORM_addr, only the split build below gives DW_FORM_addrx
g++ -gdwarf-5 -O3 -gz=none main.cpp -o mgcc -no-pie -Wl,-q
llvm-bolt mgcc -o mgcc.bolt --update-debug-sections
g++ -gdwarf-5 -gsplit-dwarf -O3 -gz=none -c main.cpp -o split.o
g++ split.o -o split -no-pie -gz=none -Wl,-q
llvm-bolt split -o split.bolt --update-debug-sections
```
## Analysis
```bash
llvm-dwarfdump --show-form --name=live --show-children main4
llvm-dwarfdump --show-form --name=live --show-children main4.bolt
llvm-dwarfdump --show-form --name=live --show-children main5
llvm-dwarfdump --show-form --name=live --show-children main5.bolt
```
Excerpt of the input (`main4`), the two DIEs of interest:
```
0x000000af: DW_TAG_label
DW_AT_name [DW_FORM_strp] ("dead")
DW_AT_decl_file [DW_FORM_data1] ("/workspace/main.cpp")
DW_AT_decl_line [DW_FORM_data1] (15)
DW_AT_decl_column [DW_FORM_data1] (1)
DW_AT_low_pc [DW_FORM_addr] (0x0000000000401127)
0x000000bf: DW_TAG_inlined_subroutine
DW_AT_abstract_origin [DW_FORM_ref4] (0x00000055 "_ZL14inlined_helperi")
DW_AT_low_pc [DW_FORM_addr] (0x0000000000401126)
DW_AT_high_pc [DW_FORM_data4] (0x00000013)
DW_AT_call_file [DW_FORM_data1] ("/workspace/main.cpp")
DW_AT_call_line [DW_FORM_data1] (16)
DW_AT_call_column [DW_FORM_data1] (10)
```
Both DIEs sit inside the same removed block: the inlined instance covers `[0x401126, 0x401139)` and the label sits at `0x401127`, inside it. `DW_AT_low_pc` of each, before and after the rewrite:
| DIE | form | main4 | main4.bolt | main5 | main5.bolt |
| --- | --- | --- | --- | --- | --- |
| `DW_TAG_label` ("dead") | `addr` / `addrx` | `0x401127` | `0x0` | `0x401127` | **`0x401127`** (!) |
| `DW_TAG_inlined_subroutine` | `addr` / `addrx` | `0x401126` | `0x0` | `0x401126` | `0x0` |
`main4` is `-gdwarf-4`, so both attributes use `DW_FORM_addr`; `main5` is `-gdwarf-5`, so both use `DW_FORM_addrx`. Only one cell does not move: the label in `main5.bolt`. Every other output cell is replaced by the deleted-code marker `0`, including the very same label when its form is `DW_FORM_addr`.
> NOTE Ignore the inlined instance's `DW_AT_high_pc` here: it went from the offset `0x13` to `0x00401139`, an absolute address in a length field, which is [#217966](https://github.com/llvm/llvm-project/issues/217966).
`.bolt.org.text` spans `[0x401020, 0x40115b)` and the live `.text` starts at `0x800000`, so the label keeps an address that now denotes the archived copy of code that will never execute again, while its neighbour - one byte away, same form, same removed block, but a tag with a code path of its own - is marked as deleted.
GCC
With clang the form changes together with the DWARF version, so a reader can object that the version is what matters. GCC separates the two: both builds are `-gdwarf-5`, the label lands at `0x401138` in each, and the form is switched by `-gsplit-dwarf` alone.
```bash
llvm-dwarfdump --show-form --name=dead mgcc mgcc.bolt
llvm-dwarfdump --show-form --name=dead split.dwo split.dwo.dwo
llvm-dwarfdump --debug-addr split split.bolt
```
| build | form | input | output |
| --- | --- | --- | --- |
| `mgcc` / `mgcc.bolt` | `DW_FORM_addr` | `0x401138` | `0x0` |
| `split` / `split.bolt` | `DW_FORM_addrx` | index `14` → `0x401138` | index `4` → **`0x401138`** (!) |
A `.dwo` carries no address table of its own, so both `.dwo` dumps print `` and the value has to be read from the executable's `.debug_addr`:
```
.debug_addr contents (split.bolt):
Addrs: [
0x0000000000404014
0x000000000080001c
0x0000000000800021
0x000000000080012c
0x0000000000401138 <- index 4, the label
0x0000000000000000 <- the table carries a zero happily
0x0000000000800126
]
```
The table was rebuilt from scratch - 18 entries in, 7 out, with the four separate input entries holding `0x401138` collapsed into this single one - and the value still came through unchanged. Output was relocated to `0x8000xx` or zeroed, and `0x404014` is a `.bss` address that never moves; `0x401138` is the only pre-rewrite code address left. BOLT did not overlook the table; it re-emitted this address into it.
## Root cause
`DW_TAG_label` has no `case` of its own, so it falls into the generic "any tag carrying `DW_AT_low_pc`" branch, together with every other tag that has no dedicated one - unlike the neighbour above, `DW_TAG_inlined_subroutine`, which is handled at line 1266 and zeroed through that path instead. Both form branches here do the same job, but they disagree on what a missing translation means:
```cpp
// bolt/lib/Rewrite/DWARFRewriter.cpp:1567-1595
} else if (LowPCAttrInfo) {
...
uint64_t NewAddress = 0;
if (const BinaryFunction *Function =
BC.getBinaryFunctionContainingAddress(Address))
NewAddress = Function->translateInputToOutputAddress(Address);
...
if (Form == dwarf::DW_FORM_addrx ||
Form == dwarf::DW_FORM_GNU_addr_index) {
const uint32_t Index = AddressWriter.getIndexFromAddress(
NewAddress ? NewAddress : Address, Unit); // keeps the input address
...
} else {
DIEBldr.replaceValue(Die, LowPCAttrInfo.getAttribute(),
LowPCAttrInfo.getForm(),
DIEInteger(NewAddress)); // writes the zero
}
```
The two branches differ only in that ternary. What `0` means there is decided by the callee, and it is narrower than it looks:
```cpp
// bolt/lib/Core/BinaryFunction.cpp:4651-4683
uint64_t BinaryFunction::translateInputToOutputAddress(uint64_t Address) const {
if (isFolded())
return 0; // folded away by ICF
// If the function hasn't changed return the same address.
if (!isEmitted())
return Address; // <- input address, not zero
...
const BinaryBasicBlock *BB = getBasicBlockContainingOffset(Offset);
if (!BB)
return 0; // block was removed
```
A zero *out of* this function means the code is gone, and the input address then denotes the archived copy - which is precisely the value the `addrx` fallback puts back:
| situation | `NewAddress` | `DW_FORM_addr` writes | `DW_FORM_addrx` writes |
| ------------------------------------------- | ------------- | ----------------------------- | --------------------------------------------------- |
| function kept in place (`!isEmitted()`) | input address | input address - correct | input address - correct |
| code removed (block deleted - the reproducer; or ICF-folded) | `0` | `0` - the deleted-code marker | **input address - a pointer into** `.bolt.org.text` |
The two branches agree for as long as the code survives, and part company the moment it does not: the one signal the callee emits to say "this instruction is gone" is the one signal the fallback is written to discard.
## Proposed fix
Start from the input address and let the callee overwrite it with its own answer, zero included. The fallback then has nothing left to restore and goes with it, which removes the form-dependence.
```diff
if (getLowPC(*Die, Unit, Address, SectionIndex)) {
- uint64_t NewAddress = 0;
+ // Start from the input address, so that NewAddress ends up zero only
+ // when translateInputToOutputAddress() says so: it returns the input
+ // address unchanged for a function that was not emitted, and zero
+ // only when the code is gone.
+ uint64_t NewAddress = Address;
if (const BinaryFunction *Function =
BC.getBinaryFunctionContainingAddress(Address)) {
NewAddress = Function->translateInputToOutputAddress(Address);
@@
if (Form == dwarf::DW_FORM_addrx ||
Form == dwarf::DW_FORM_GNU_addr_index) {
- const uint32_t Index = AddressWriter.getIndexFromAddress(
- NewAddress ? NewAddress : Address, Unit);
+ const uint32_t Index =
+ AddressWriter.getIndexFromAddress(NewAddress, Unit);
```
The label's `DW_AT_low_pc`, measured on the reproducer with proposed fix applied to `bd6adfe`:
| build | without the fix | with the fix |
| ------------------------------------ | -------------------------------- | ---------------- |
| clang `-gdwarf-5`, `DW_FORM_addrx` | `0x401127`, inside archived code | `0x0` |
| clang `-gdwarf-4`, `DW_FORM_addr` | `0x0` | `0x0`, unchanged |
| GCC `-gsplit-dwarf`, `DW_FORM_addrx` | `0x401138`, inside archived code | `0x0` |
> NOTE In the split-DWARF build the stale value leaves the address table altogether - `.debug_addr` drops from 7 entries to 6 and the output no longer holds any address inside `.bolt.org.text`.
**Important.** Why not the narrower edit, dropping the fallback and keeping `NewAddress = 0`? Because in a **third situation** the lookup finds no `BinaryFunction`, `translateInputToOutputAddress()` is never called, and `NewAddress` still holds its initializer. That `0` means the opposite of the callee's - nothing moved, so the input address is still right - and the two are indistinguishable. There the `addr` branch already destroys a valid address, and the fallback is the only thing keeping `addrx` correct; dropping it would make both write the zero. Initialising from `Address` removes that second meaning instead, so every `0` reaching either branch came out of the translation. That situation is reasoned from the code during that investigation - a label always sits inside some `BinaryFunction`, so the reproducer above does not reach it. Recommendation would be that all these cases should be covered by lit tests.
Contributor guide
Research direction
Start in bolt/lib/Rewrite/DWARFRewriter.cpp at the generic DW_AT_low_pc handling, then read BinaryFunction::translateInputToOutputAddress in bolt/lib/Core/BinaryFunction.cpp. Run the clang or GCC reproducer with llvm-bolt and llvm-dwarfdump, and verify that removed-code DW_FORM_addrx values become 0 without breaking addresses for code with no BinaryFunction or for unmodified functions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100