[SR-13449] Wrong method call when binary is built with optimizations

[lxbndr]

Previous ID	SR-13449
Radar	rdar://problem/68170038
Original Reporter	@lxbndr
Type	Bug
Status	Closed
Resolution	Done

Environment

Swift version 5.3-dev (LLVM 36f7fac60d2e676, Swift a5ca83a8b2d499f)
Target: x86_64-unknown-windows-msvc

Additional Detail from JIRA

Votes	1
Component/s	Compiler
Labels	Bug, OptimizedOnly, Optimizer
Assignee	None
Priority	Medium

md5: 205e769ae641e22368ccb8ccbb625245

blocks:

• TF-1314 Networking timeout

Issue Description:

Under specific conditions binary built with optimizations would call wrong method inside a class. This is the reason why network requests are not working in swift-corelibs-foundation at the moment.

Sample code have to be split in two sources, otherwise the issue is not reproduced.

// Grandchild.swift
class Grandchild: Child {}

// main.swift
class Root {}
class Child: Root {
  func doSomething() {
    print("-- \(type(of: self)) - \(#function) - will call doHomework()")
    doHomework()
    print("-- \(type(of: self)) - \(#function) - did call doHomework()")
  }   
  private func eatPopcorn() {
    print("-- \(type(of: self)) - \(#function)")
  }
  func playGames() {
    print("-- \(type(of: self)) - \(#function) - SHOULD NOT HAPPEN")
    eatPopcorn()
  }
  func doHomework() {
    print("-- \(type(of: self)) - \(#function)")
  }
}

Grandchild().doSomething()

Expected output:

-- Grandchild - doSomething() - will call doHomework()
-- Grandchild - doHomework()
-- Grandchild - doSomething() - did call doHomework()

Actual output:

-- Grandchild - doSomething() - will call doHomework()
-- Grandchild - playGames() - SHOULD NOT HAPPEN
-- Grandchild - eatPopcorn()
-- Grandchild - doSomething() - did call doHomework()

[compnerd]

+michellecasbon (JIRA User) has also seen this on Linux.

This might be related to @jckarter's change to enable vtable optimizations (#32359)

[compnerd]

Okay, so I had a chance to look at this. This is a pretty interesting bug and the change itself makes sense as the precipitating factor. Note that this

I have a slightly different test case than @lxbndr but its based on his work:

class C {}
class D: C {
 func f() { i() }
fileprivate func g() {
 print("g")
 fatalError("death")
 }
func h() { g() }
func i() { }
}
E().f()

class E: D {}

Compiling this as (on Linux):

swiftc -emit-executable main.swift support.swift -Osize -module-name M

results in the program terminating unexpectedly.

I believe that the reason for this is the VTable Pruning pass which interacts poorly with a very interesting set of things happening.

1. The test is multi-file with a class hierarchy split across multiple files.

2. The VTable Pruning pass kicks in and removes the VWT entry for g

3. The local function implementation uses the VWT for the parent type that everything is derived from, which gets the offset incorrect.

During execution, breaking at f we see the following:

(gdb) disassemble
Dump of assembler code for function $s1M1DC1fyyF:
   0x000055555555515b <+0>:     mov    0x0(%r13),%rax
=> 0x000055555555515f <+4>:     jmpq   *0x50(%rax)

Which seems like the reasonable thing given that its a VWT dispatch.

Taking a look at the type metadata in use:

(gdb) x/12xg $rax
0x55555555b158 <$s1M1ECN>:       0x0000000000000000 0x000055555555b0e8
0x55555555b168 <$s1M1ECN+16>:    0x0000000000000002 0x0000000700000010
0x55555555b178 <$s1M1ECN+32>:    0x0000001000000070 0x00005555555555c8
0x55555555b188 <$s1M1ECN+48>:    0x0000000000000000 0x0000555555555390
0x55555555b198 <$s1M1ECN+64>:    0x00005555555551b5 0x0000555555555162
0x55555555b1a8 <$s1M1ECN+80>:    0x0000555555555333 0x0000555555555339
(gdb) x/i 0x0000555555555390
   0x0000555555555390 <$s1M1ECACycfC>:   push   %r13
(gdb) x/i 0x00005555555551b5
   0x00005555555551b5 <$s1M1DC1fyyF>:    mov    0x0(%r13),%rax
(gdb) x/i 0x0000555555555162
   0x0000555555555162 <$s1M1DC11g33_ECFA201E680F09F769E21A64A6CF7A0FLLyyF>:   push    %r15
(gdb) x/i 0x0000555555555333
   0x0000555555555333 <$s1M1DC1hyyF>:    push   %rax
(gdb) x/i 0x0000555555555339
   0x0000555555555339 <$s1M1DC1iyyF>:    retq

Okay, so, the type metadata dump is pretty useful here, lets decompose it (via some cheating):

        .global $s1M1ECN
        .hidden $s1M1ECN
.set $s1M1ECN, ($s1M1ECNMf)+16
        .type $s1M1ECMf,@object
        .data
        .p2align 3
$s1M1ECMf:
        .quad ($S1M1ECfD)
        .quad ($sBoWV)
        .quad 0                                                 ; +00 isa
        .quad ($s1M1DCN)                                        ; +08 super
        .long 2                                                 ; +16 flags
        .long 0                                                 ; +20 instance address point
        .long 16                                                ; +24 instance size
        .short 7                                                ; +28 instance alignment mask
        .short 0                                                ; +30 reserved
        .long 112                                               ; +32 class object size
        .long 16                                                ; +36 class object address point
        .quad ($s1M1ECMn)                                       ; +40 nominal type descriptor
        .quad 0                                                 ; +48 parent
        .quad ($s1M1ECACycfC)                                   ; +56 init
        .quad ($s1M1DC1fyyF)                                    ; +64 f
        .quad ($s1M1DC11g33_ECFA201E680F09F769E21A64A6CF7A0FLLyyF) ; +72 g
        .quad ($s1M1DC1hyyF)                                    ; +80 h
        .quad ($s1M1DC1iyyF)                                    ; +88 i
        .size $s1M1ECMf, 112

Since we index at +80 we end up calling h instead of i.

[compnerd]

Note that because E does not override the method, the compiler can statically tell the dispatch statically. It is prohibited by doing so by default as E is in a separate file. Using -wmo will allow the static dispatch to be realized and restores this test. However, the test case is intended to identify the problem that exists in the non-WMO case or the case that there is dynamic dispatch. The difference in the VWT between the two types results in the dispatch failing.

[lxbndr]

Fixed in #33770. Just did re-check on LLVM 9c1e12989ad359e, Swift 9685179e5fa933b.

Should I close it now? @compnerd

[compnerd]

There is a small follow up to fix something that I thought I had done (I accidentally had it disabled for fileprivate), but this is complete beyond that. Should be okay to close.