[llvm] r307729 - [IPO] Temporarily rollback r307215.

[Davide Italiano via llvm-commits]

On Wed, Jul 12, 2017 at 4:17 PM, Davide Italiano <davide at freebsd.org> wrote:
> On Tue, Jul 11, 2017 at 4:10 PM, Davide Italiano via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: davide
>> Date: Tue Jul 11 16:10:17 2017
>> New Revision: 307729
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=307729&view=rev
>> Log:
>> [IPO] Temporarily rollback r307215.
>>
>> [GlobalOpt] Remove unreachable blocks before optimizing a function.
>> While the change is presumably correct, it exposes a latent bug
>> in DI which breaks on of the CFI checks. I'll analyze it further
>> and try to understand what's going on.
>>
>
> After Matt's report and the revert I looked at this bug at length today.
> The CFI tests are failing because `sanstats` is not actually able to
> map back to the correct line number (see, e.g. the test called
> stats.cpp in compiler-rt/test/cfi/cross-dso/)
>
> i.e. in the working case we have:
>
> /home/davide/work/llvm/projects/compiler-rt/test/cfi/cross-dso/stats.cpp:23
> vcall.cfi cfi-vcall 37
>
> and in the broken case we have:
>
> /home/davide/work/llvm/projects/compiler-rt/test/cfi/cross-dso/stats.cpp:0
> vcall.cfi cfi-vcall 37
>
> The difference in IR with and without this patch for @vcall.cfi is the
> following:
>
> -  br label %cfi.slowpath, !dbg !20
> + br i1 false, label %cfi.cont, label %cfi.slowpath, !dbg !20, !prof
> !21, !nosanitize !2
>
> In the "broken" case (i.e. with the globalOpt patch) we transform a
> branch on a known condition to an unconditional branch:
>
> The two IR dumps look like:
>
> WORKING:
>
>   vtable = load void (%struct.a*)**, void (%struct.a*)*** %1, align 8, !dbg !20
>   call void @__sanitizer_stat_report(...), !dbg !20
>   %2 = bitcast void (%struct.a*)** %vtable to i8*, !dbg !20, !nosanitize !2
>   br i1 false, label %cfi.cont, label %cfi.slowpath, !dbg !20, !prof
> !21, !nosanitize !2
>
> !20 = !DILocation(line: 23, column: 6, scope: !9)
>
> and
>
> BROKEN:
>
>   %vtable = load void (%struct.A*)**, void (%struct.A*)*** %1, align 8, !dbg !20
>   call void @__sanitizer_stat_report(...), !dbg !20
>   %2 = bitcast void (%struct.A*)** %vtable to i8*, !dbg !20, !nosanitize !2
>   br label %cfi.slowpath, !dbg !20
>
> !20 = !DILocation(line: 23, column: 6, scope: !9)
>
> (this information lives up to the end of the optimizer so I don't
> think it's a bug in the middle-end). Note, the `!prof` and
> `!nosanitize` metadata attached to the `br` instruction are stripped
> as well but I'm not sure it matters here.
>
>
> The code generated looks like:
> WORKING:
>
>     206e:       4c 8b 33                mov    (%rbx),%r14
>     2071:       48 89 c7                mov    %rax,%rdi
>     2074:       e8 d7 f0 ff ff          callq  1150 <__sanitizer_stat_report>
>     2079:       31 c0                   xor    %eax,%eax
>     207b:       a8 01                   test   $0x1,%al
>     207d:       75 12                   jne    2091 <vcall.cfi+0x41>
>     207f:       48 bf 12 14 8e 46 2e    movabs $0x6133c22e468e1412,%rdi
>
> and line table:
>
> Address            Line   Column File   ISA Discriminator Flags
> ------------------ ------ ------ ------ --- ------------- -------------
> 0x000000000000206e     23      6      1   0             0
> 0x000000000000207f      0      6      1   0             0
> 0x0000000000002089     23      6      1   0             0
>
> BROKEN:
>
>     2071:       48 89 c7                mov    %rax,%rdi
>     2074:       e8 d7 f0 ff ff          callq  1150 <__sanitizer_stat_report>
>     2079:       48 bf 12 14 8e 46 2e    movabs $0x6133c22e468e1412,%rdi
>
> Address            Line   Column File   ISA Discriminator Flags
> ------------------ ------ ------ ------ --- ------------- -------------
> 0x000000000000206e     23      6      1   0             0
> 0x0000000000002079      0      6      1   0             0
> 0x0000000000002083     23      6      1   0             0
>
>
> `sanstats` asks for 0x2079.
> So it just seems to me without this optimization we were able to get
> the correct line number because there were some instructions mapped to
> the same line before the `movabs`, which then are removed when we fold
> the branch so we map back to "0". I'm not entirely sure who's at fault
> here. Thoughts?
>
> --
> Davide

Note, I don't know how this whole machinery works (at least not in
detail ;) so I could be completely off, but I expected `sanstats` to
ask the symbolication of the call, i.e.

    2074:       e8 d7 f0 ff ff          callq  1150 <__sanitizer_stat_report>

`0x2074` instead of `0x2079`.

Thanks,

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare