<html>
    <head>
      <base href="https://bugs.llvm.org/">
    </head>
    <body><table border="1" cellspacing="0" cellpadding="8">
        <tr>
          <th>Bug ID</th>
          <td><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [DebugInfo@O2] LiveDebugValues misses stack restores, creates immortal variable locations"
   href="https://bugs.llvm.org/show_bug.cgi?id=42772">42772</a>
          </td>
        </tr>

        <tr>
          <th>Summary</th>
          <td>[DebugInfo@O2] LiveDebugValues misses stack restores, creates immortal variable locations
          </td>
        </tr>

        <tr>
          <th>Product</th>
          <td>libraries
          </td>
        </tr>

        <tr>
          <th>Version</th>
          <td>trunk
          </td>
        </tr>

        <tr>
          <th>Hardware</th>
          <td>PC
          </td>
        </tr>

        <tr>
          <th>OS</th>
          <td>Linux
          </td>
        </tr>

        <tr>
          <th>Status</th>
          <td>NEW
          </td>
        </tr>

        <tr>
          <th>Keywords</th>
          <td>wrong-debug
          </td>
        </tr>

        <tr>
          <th>Severity</th>
          <td>normal
          </td>
        </tr>

        <tr>
          <th>Priority</th>
          <td>P
          </td>
        </tr>

        <tr>
          <th>Component</th>
          <td>Common Code Generator Code
          </td>
        </tr>

        <tr>
          <th>Assignee</th>
          <td>unassignedbugs@nondot.org
          </td>
        </tr>

        <tr>
          <th>Reporter</th>
          <td>jeremy.morse.llvm@gmail.com
          </td>
        </tr>

        <tr>
          <th>CC</th>
          <td>aprantl@apple.com, chackz0x12@gmail.com, greg.bedwell@sony.com, llvm-bugs@lists.llvm.org, orlando.hyams@sony.com, paul.robinson@am.sony.com, stephen.tozer@sony.com, vsk@apple.com, Wolfgang_Pieb@playstation.sony.com
          </td>
        </tr>

        <tr>
          <th>Blocks</th>
          <td>38768
          </td>
        </tr></table>
      <p>
        <div>
        <pre>The spill/restore recognition code in LiveDebugValues has an unexpected
feedback effect that makes it faulty. I've been using LLVM/clang r366789,
compiling the program below with "-O2 -g", and can generate some invalid
locations.

The code below is designed to force spills and restores in the "f" function
(inspired by live-debug-values-restore.mir). It works thus:
 * The first FORCE_SPILL pushes a and b onto the stack
 * The inspection of "bees" forces there to be control flow
 * The second FORCE_SPILL forces "quux" onto the stack, and nothing else 
   because a and b are out of liveness.

For the reasons I'll explain below, the stack-restore of of "a" and "b"'s
locations isn't recognised, and their spilt location propagates to the end of
the function. quux is later spilt to one of the same spill slots, making one of
the arguments report an incorrect value.

-------->8--------
#define FORCE_SPILL() \
  __asm volatile("" : : : \
                   "rax", "rbx", "rcx", "rdx", "rsi", "rdi", "rbp", "r8", \
                   "r9", "r10", "r11", "r12", "r13", "r14", "r15")

volatile int bees = 0;

__attribute__((noinline))
long int sum(long int a, long int b)
{
  return a + b;
}

__attribute__((noinline))
long int f(long int a, long int b) {
  FORCE_SPILL();

  if (bees == 0)
    bees += 1;

  int quux = sum(a, b);
  FORCE_SPILL();
  return quux;
}

int
main()
{
  return f(1, 2);
}
--------8<--------

Stepping through in gdb, "b" reports the value "3" on the return-line of the f
function, which is incorrect. Here's the location of b according to
llvm-dwarfdump:

-------->8--------

 DW_TAG_formal_parameter
    DW_AT_location      (0x00000037
       [0x400490,  0x4004a3): DW_OP_reg4 RSI
       [0x4004a3,  0x4004e0): DW_OP_breg7 RSP+8)
    DW_AT_name  ("b")
    DW_AT_decl_file     ("test.c")
    DW_AT_decl_line     (15)
    DW_AT_type  ("long int")
--------8<--------

Note the stack-location runs to the end of the function. The location is never
restored, and "quux" is eventually spilt to the same place.

The spill and restore implementation of r352642 [0] works totally fine inside a
single basic block. With multiple blocks, it should work like this: If a value
is inside a "spill" location at the end of a block, it's propagated to other
blocks in the usual dataflow way [1]. When processing another basic block, the
set of currently-valid variable locations is rebuilt by interpreting the
DBG_VALUEs at the start of the block in the transferDebugValue method. This
clause [2] in transferDebugValue recognises DBG_VALUE locations that are in
spill slots, and records its location in the OpenRanges class accordingly.

The problem is this: because LiveDebugValues runs after PrologEpilog, no
DBG_VALUEs refer to stack slots, nor can they. In fact, [2] is dead code [3],
putting a trap instruction inside the code block and building clang-3.4 never
hits the trap. Instead, all stack-spilt DBG_VALUEs are of this form:

  DBG_VALUE $rsp, $noreg, !123, !DIExpression(DW_OP_constu, 8, DW_OP_minus)

Which is interpreted as a register location (it's based on $rsp after all) and
recorded as a "RegisterKind" in the VarLoc class. It'll never match this
recogniser [4] for stack spills, and thus will never be restored.

If a variable is never restored to a register (because of this fault; or on X86
perhaps it's referenced via a memory operand), and at the source level the
variable is never assigned again, the spill location lives to the end of the
function. This risks it silently going out of liveness, and the register
allocator spilling some other value onto the stack location.

This is exactly what happens with the example code above: 'b' is spilt; control
flow happens; b's restore is never recognised; then 'quux' is spilt on top of
b's stack slot.

~

Fixing this is hard: IMO digging around in a DIExpression to work out whether
something is a spill or not is a Bad Plan (TM). Not doing that would then
involve storing additional information in LiveDebugValues about what's a spill
location, and what isn't, which then might need algorithm changes.

We would need to store the original DIExpressions before a spill; as it stands,
the spill restorer [5] uses an empty DIExpression on restore, which I have
another ticket to open about.

[0] <a href="https://reviews.llvm.org/rL352642">https://reviews.llvm.org/rL352642</a>
[1]
<a href="https://llvm.org/docs/SourceLevelDebugging.html#livedebugvalues-expansion-of-variable-locations">https://llvm.org/docs/SourceLevelDebugging.html#livedebugvalues-expansion-of-variable-locations</a>
[2]
<a href="https://github.com/llvm/llvm-project/blob/12aca5de026bd15663596c392ac828f8078bca6b/llvm/lib/CodeGen/LiveDebugValues.cpp#L569">https://github.com/llvm/llvm-project/blob/12aca5de026bd15663596c392ac828f8078bca6b/llvm/lib/CodeGen/LiveDebugValues.cpp#L569</a>
[3] (added by me!)
[4]
<a href="https://github.com/llvm/llvm-project/blob/12aca5de026bd15663596c392ac828f8078bca6b/llvm/lib/CodeGen/LiveDebugValues.cpp#L857">https://github.com/llvm/llvm-project/blob/12aca5de026bd15663596c392ac828f8078bca6b/llvm/lib/CodeGen/LiveDebugValues.cpp#L857</a>
[5]
<a href="https://github.com/llvm/llvm-project/blob/12aca5de026bd15663596c392ac828f8078bca6b/llvm/lib/CodeGen/LiveDebugValues.cpp#L696">https://github.com/llvm/llvm-project/blob/12aca5de026bd15663596c392ac828f8078bca6b/llvm/lib/CodeGen/LiveDebugValues.cpp#L696</a></pre>
        </div>
      </p>

        <div id="referenced">
          <hr style="border: 1px dashed #969696">
          <b>Referenced Bugs:</b>
          <ul>
              <li>
                [<a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [meta][DebugInfo] Umbrella bug for poor debug experiences"
   href="https://bugs.llvm.org/show_bug.cgi?id=38768">Bug 38768</a>] [meta][DebugInfo] Umbrella bug for poor debug experiences
              </li>
          </ul>
        </div>
        <br>

      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are on the CC list for the bug.</li>
      </ul>
    </body>
</html>