[all-commits] [llvm/llvm-project] 6795bf: [BOLT][RISCV] Handle CIE's produced by GNU as (#69...

[Job Noorman via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 6795bfce4ded6abf2403db62a5fec89a8bcb256a
      https://github.com/llvm/llvm-project/commit/6795bfce4ded6abf2403db62a5fec89a8bcb256a
  Author: Job Noorman <jnoorman at igalia.com>
  Date:   2023-10-20 (Fri, 20 Oct 2023)

  Changed paths:
    M bolt/lib/Core/BinaryFunction.cpp
    A bolt/test/RISCV/Inputs/cie-gnu.yaml
    A bolt/test/RISCV/cie-gnu.test

  Log Message:
  -----------
  [BOLT][RISCV] Handle CIE's produced by GNU as (#69578)

On RISC-V, GNU as produces the following initial instruction in CIE's:

```
DW_CFA_def_cfa_register: r2
```

While I believe it is technically illegal to use this instruction
without first using a `DW_CFA_def_cfa` (since the offset is undefined),
both `readelf` and `llvm-dwarfdump` accept this and implicitly set the
offset to 0.

In BOLT, however, this triggers an assert (in `CFISnapshot::advanceTo`)
as it (correctly) believes the offset is not set. This patch fixes this
by setting the offset to 0 whenever executing `DW_CFA_def_cfa_register`
while the offset is undefined.

Note that this is probably the simplest workaround but it has a
downside: while emitting CFI start, we check if the initial instructions
are contained within `MCAsmInfo::getInitialFrameState` and omit them if
they are. This will not be true for GNU CIE's (since they differ from
LLVM's) which causes an unnecessary `DW_CFA_def_cfa_register` to be
emitted.

While technically correct, it would probably be better to replace the
GNU CIE with the one used by LLVM to avoid this situation. This would
solve the problem this patch solves while also preventing unnecessary
CFI instructions. However, this is a bit trickier to implement correctly
so I propose to keep this for a later time.

Note on testing: the test creates a simple function with three basic
blocks and forces the CFI state of the last one to be different from the
others using an arbitrary CFI instruction. Then,
`--reorder-blocks=reverse` is used to force `CFISnapshot::advanceTo` to
be called. This causes an assert on the current main branch.