search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Wrong line information at Og #44978

Open Quuxplusone opened 4 years ago

Quuxplusone commented 4 years ago
Bugzilla Link PR46009
Status CONFIRMED
Importance P enhancement
Reported by Luca Massarelli (massarelli@diag.uniroma1.it)
Reported on 2020-05-20 09:02:59 -0700
Last modified on 2020-06-17 11:16:50 -0700
Version trunk
Hardware PC Linux
CC aprantl@apple.com, ditaliano@apple.com, jdevlieghere@apple.com, jeremy.morse.llvm@gmail.com, keith.walker@arm.com, llvm-bugs@lists.llvm.org, paul_robinson@playstation.sony.com, vsk@apple.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also 
Line 8 should not be hit.

$ cat a.c
int a,  b,  c ;
int main ()
{
    {int ui1 = 5,  ui2 = b;
       c =
          ui2 == 0 ?
          ui1 :
          (ui1 / ui2);
     }
}

$ clang -v
clang version 11.0.0 (https://github.com/llvm/llvm-project.git
268fa40daa151d3b4bff1df12b62e5dae94685d7)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/local/bin
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/7
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/7.5.0
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/8
Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/7.5.0
Candidate multilib: .;@m64
Selected multilib: .;@m64

$ lldb -v

$ lldb opt
(lldb) target create "opt"
Current executable set to 'opt' (x86_64).
(lldb) b main
Breakpoint 1: where = opt`main at a.c:4:26, address = 0x0000000000400480
(lldb) r
Process 65 launched: 'opt' (x86_64)
Process 65 stopped
* thread #1, name = 'opt', stop reason = breakpoint 1.1
    frame #0: 0x0000000000400480 opt`main at a.c:4:26
   1    int a,  b,  c ;
   2    int main ()
   3    {
-> 4        {int ui1 = 5,  ui2 = b;
   5           c =
   6              ui2 == 0 ?
   7              ui1 :
(lldb) s
Process 65 stopped
* thread #1, name = 'opt', stop reason = step in
    frame #0: 0x0000000000400486 opt`main at a.c:8:16
   5           c =
   6              ui2 == 0 ?
   7              ui1 :
-> 8              (ui1 / ui2);
   9         }
   10   }
(lldb) di
opt`main:
    0x400480 <+0>:  movl   0x200ba6(%rip), %ecx      ; b
->  0x400486 <+6>:  movl   $0x5, %eax
    0x40048b <+11>: testl  %ecx, %ecx
    0x40048d <+13>: je     0x400493                  ; <+19> at a.c:5:10
    0x40048f <+15>: xorl   %edx, %edx
    0x400491 <+17>: idivl  %ecx
    0x400493 <+19>: movl   %eax, 0x200b97(%rip)      ; c
    0x400499 <+25>: xorl   %eax, %eax
    0x40049b <+27>: retq
(lldb) s
Process 65 stopped
* thread #1, name = 'opt', stop reason = step in
    frame #0: 0x000000000040048b opt`main at a.c:6:15
   3    {
   4        {int ui1 = 5,  ui2 = b;
   5           c =
-> 6              ui2 == 0 ?
   7              ui1 :
   8              (ui1 / ui2);
   9         }
(lldb) s
Process 65 stopped
* thread #1, name = 'opt', stop reason = step in
    frame #0: 0x0000000000400493 opt`main at a.c:5:10
   2    int main ()
   3    {
   4        {int ui1 = 5,  ui2 = b;
-> 5           c =
   6              ui2 == 0 ?
   7              ui1 :
   8              (ui1 / ui2);
(lldb) s
Process 65 stopped
* thread #1, name = 'opt', stop reason = step in
    frame #0: 0x0000000000400499 opt`main at a.c:10:1
   7              ui1 :
   8              (ui1 / ui2);
   9         }
-> 10   }
Quuxplusone commented 4 years ago 
Confirmed on

davide@Davides-iMac-Pro bin % ./clang --version
clang version 11.0.0 (https://github.com/llvm/llvm-project
31984063678ef523b940160ac9d49fcb87c1e2ac)
Target: x86_64-apple-darwin20.0.0
Thread model: posix
InstalledDir: /Users/davide/work/build/bin/.
Quuxplusone commented 4 years ago 
BISECT: running pass (78) Shrink Wrapping analysis on function (main)
BISECT: running pass (79) Control Flow Optimizer on function (main)

This is the pass responsible for the bug.
Quuxplusone commented 4 years ago 
MIR after BranchFolding:

(lldb) p MF.dump()
# Machine code for function main: NoPHIs, TracksLiveness, NoVRegs
Frame Objects:
  fi#-1: size=8, align=16, fixed, at location [SP-8]

bb.0.entry:
  successors: %bb.2(0x30000000), %bb.1(0x50000000); %bb.2(37.50%), %bb.1(62.50%)

  frame-setup PUSH64r killed $rbp, implicit-def $rsp, implicit $rsp
  CFI_INSTRUCTION def_cfa_offset 16
  CFI_INSTRUCTION offset $rbp, -16
  $rbp = frame-setup MOV64rr $rsp
  CFI_INSTRUCTION def_cfa_register $rbp
  DBG_VALUE 5, $noreg, !"ui1", !DIExpression(), debug-location !23; ../../build/bin/a.c:0 line no:4
  renamable $ecx = MOV32rm $rip, 1, $noreg, @b, $noreg, debug-location !24 :: (dereferenceable load 4 from @b, !tbaa !25); ../../build/bin/a.c:4:26
  DBG_VALUE $ecx, $noreg, !"ui2", !DIExpression(), debug-location !23; ../../build/bin/a.c:0 line no:4
  $eax = MOV32ri 5, debug-location !31; ../../build/bin/a.c:8:16
  TEST32rr renamable $ecx, renamable $ecx, implicit-def $eflags, debug-location !29; ../../build/bin/a.c:6:15
  JCC_1 %bb.2, 4, implicit $eflags, debug-location !30; ../../build/bin/a.c:6:11
[...]
Quuxplusone commented 4 years ago

Before HoistCommonCodeInSuccs:

bb.0.entry:
  successors: %bb.1(0x30000000), %bb.2(0x50000000); %bb.1(37.50%), %bb.2(62.50%)

  frame-setup PUSH64r killed $rbp, implicit-def $rsp, implicit $rsp
  CFI_INSTRUCTION def_cfa_offset 16
  CFI_INSTRUCTION offset $rbp, -16
  $rbp = frame-setup MOV64rr $rsp
  CFI_INSTRUCTION def_cfa_register $rbp
  DBG_VALUE 5, $noreg, !"ui1", !DIExpression(), debug-location !23; ../../build/bin/a.c:0 line no:4
  renamable $ecx = MOV32rm $rip, 1, $noreg, @b, $noreg, debug-location !24 :: (dereferenceable load 4 from @b, !tbaa !25); ../../build/bin/a.c:4:26
  DBG_VALUE $ecx, $noreg, !"ui2", !DIExpression(), debug-location !23; ../../build/bin/a.c:0 line no:4
  TEST32rr renamable $ecx, renamable $ecx, implicit-def $eflags, debug-location !29; ../../build/bin/a.c:6:15
  JCC_1 %bb.2, 5, implicit killed $eflags, debug-location !30; ../../build/bin/a.c:6:11

bb.1:
; predecessors: %bb.0
  successors: %bb.3(0x80000000); %bb.3(100.00%)

  renamable $eax = MOV32ri 5
  JMP_1 %bb.3

bb.2.cond.false:
; predecessors: %bb.0
  successors: %bb.3(0x80000000); %bb.3(100.00%)
  liveins: $ecx
  $eax = MOV32ri 5, debug-location !31; ../../build/bin/a.c:8:16
  $edx = MOV32r0 implicit-def dead $eflags, debug-location !31; ../../build/bin/a.c:8:16
  IDIV32r killed renamable $ecx, implicit-def $eax, implicit-def dead $edx, implicit-def dead $eflags, implicit $eax, implicit killed $edx, debug-location !31; ../../build/bin/a.c:8:16

So it's the hoisting that screws us over. :|

Quuxplusone commented 4 years ago

The question here is: should we always drop locations when hoisting? Or, more generally, should we have splice dropping the locations?

Quuxplusone commented 4 years ago

Proposed patch here:

https://reviews.llvm.org/D80602

Quuxplusone commented 4 years ago

Back to this. What Vedant suggests I think it's correct and I'll try implementing it now.