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

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 12 16:17:20 PDT 2017


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


More information about the llvm-commits mailing list