[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Thanks a lot for so detailed answer, it helps!

So we need to parse a FPO program and to convert it in a DWARF expression too. The problem here (in the DIA case) is that I don't know how to retrieve the required FPO range (we have a symbol context when creating a variable, but it seems that it doesn't contain enough information). As for the raw PDB case, can the same `S_LOCAL` have several `S_DEFRANGE_FRAMEPOINTER_REL` records for several ranges? If so, then the problem exists for the raw PDB case too, but there's a solution: we can combine processing of all `S_DEFRANGE_FRAMEPOINTER_REL` records in the single DWARF expression (and call the required FPO program depending on current `eip`).

The interesting moment here is that MSVC doesn't emit correct information for locals. For the program `aaa.cpp`:

  void bar(char a, short b, int c) { }
  
  void __fastcall foo(short arg_0, float arg_1) {
    char loc_0 = 'x';
    double __declspec(align(8)) loc_1 = 0.5678;
    bar(1, 2, 3);
  }
  
  int main(int argc, char *argv[]) {
    foo(1111, 0.1234);
    return 0;
  }

compiled with `cl /Zi /Oy aaa.cpp` we have the next disassembly of `foo`:

  push    ebp
  mov     ebp, esp
  and     esp, 0FFFFFFF8h
  sub     esp, 10h
  mov     [esp+4], cx     ; arg_0
  mov     byte ptr [esp+3], 'x' ; loc_0
  movsd   xmm0, ds:__real at 3fe22b6ae7d566cf
  movsd   qword ptr [esp+8], xmm0 ; loc_1
  push    3               ; c
  push    2               ; b
  push    1               ; a
  call    j_?bar@@YAXDFH at Z ; bar(char,short,int)
  add     esp, 0Ch
  mov     esp, ebp
  pop     ebp
  retn    4

but MSVC emits the next info about locals:

  296 | S_GPROC32 [size = 44] `foo`
        parent = 0, end = 452, addr = 0001:23616, code size = 53
        type = `0x1003 (void (short, float))`, debug start = 14, debug end = 47, flags = has fp
  340 | S_FRAMEPROC [size = 32]
        size = 16, padding size = 0, offset to padding = 0
        bytes of callee saved registers = 0, exception handler addr = 0000:0000
        local fp reg = VFRAME, param fp reg = EBP
        flags = has async eh | opt speed
  372 | S_REGREL32 [size = 20] `arg_0`
        type = 0x0011 (short), register = ESP, offset = 4294967284
  392 | S_REGREL32 [size = 20] `arg_1`
        type = 0x0040 (float), register = EBP, offset = 8
  412 | S_REGREL32 [size = 20] `loc_1`
        type = 0x0041 (double), register = ESP, offset = 4294967288
  432 | S_REGREL32 [size = 20] `loc_0`
        type = 0x0070 (char), register = ESP, offset = 4294967283
  452 | S_END [size = 4]

so neither LLDB nor Visual Studio show valid values (except for `arg_1`).

In https://reviews.llvm.org/D53086#1260787, @zturner wrote:

> You didn't include it here, but I notice the test file just writes `clang-cl /Zi`.  Should we be passing `-m32` or `-m64`?  Currently, the test just runs with whatever architecture happens to be set via the VS command prompt.  The behavior here is different on x86 and x64 so perhaps it requires separate tests.


The problem is that x64 LLDB can't debug x86 applications on Windows (and vice versa), so I rely here on the fact that tests are run in the same VS command prompt as LLVM was built. But yes, it's still possible to run tests for x86 LLVM under the x64 VS command prompt, so this one will fail. I think we can somehow guard this when I'll implement `compiler_config` you have mentioned in https://reviews.llvm.org/D52618.

In https://reviews.llvm.org/D53086#1260685, @stella.stamenova wrote:

> Could you document that in the test?


Sure, thanks!


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53086