Skip to content

SIL: Verify kinds of vtable entries. #32171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 4, 2020
Merged

SIL: Verify kinds of vtable entries. #32171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/SIL/IR/SILDeclRef.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,12 @@ bool SILDeclRef::requiresNewVTableEntry() const {
if (derivativeFunctionIdentifier)
if (derivativeFunctionRequiresNewVTableEntry(*this))
return true;
if (cast<AbstractFunctionDecl>(getDecl())->needsNewVTableEntry())
if (!hasDecl())
return false;
auto fnDecl = dyn_cast<AbstractFunctionDecl>(getDecl());
if (!fnDecl)
return false;
if (fnDecl->needsNewVTableEntry())
return true;
return false;
}
Expand Down
67 changes: 65 additions & 2 deletions lib/SIL/Verifier/SILVerifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5360,8 +5360,22 @@ void SILProperty::verify(const SILModule &M) const {
void SILVTable::verify(const SILModule &M) const {
if (!verificationEnabled(M))
return;

for (auto &entry : getEntries()) {

// Compare against the base class vtable if there is one.
const SILVTable *superVTable = nullptr;
auto superclass = getClass()->getSuperclassDecl();
if (superclass) {
for (auto &vt : M.getVTables()) {
if (vt.getClass() == superclass) {
superVTable = &vt;
break;
}
}
}

for (unsigned i : indices(getEntries())) {
auto &entry = getEntries()[i];

// All vtable entries must be decls in a class context.
assert(entry.Method.hasDecl() && "vtable entry is not a decl");
auto baseInfo =
Expand Down Expand Up @@ -5403,6 +5417,55 @@ void SILVTable::verify(const SILModule &M) const {
"vtable entry for " + baseName + " must be ABI-compatible",
*entry.Implementation);
}

// Validate the entry against its superclass vtable.
if (!superclass) {
// Root methods should not have inherited or overridden entries.
bool validKind;
switch (entry.TheKind) {
case Entry::Normal:
case Entry::NormalNonOverridden:
validKind = true;
break;

case Entry::Inherited:
case Entry::Override:
validKind = false;
break;
}
assert(validKind && "vtable entry in root class must not be inherited or override");
Copy link
Contributor

@gottesmm gottesmm Jun 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continue and eliminate the else? E.x.:

if (...) {
   assert(validKind, ...)
   continue;
}

if (superVTable) {
   ...
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why I am suggesting this is that it makes it clearer that the if statement will never cause any code beneath it to be evaluated. Without this, I have to scan after the else if to see if that code may be run after the first if completes. In contrast, with the early continue like this I now have a nice invariant I can use when scanning the code below. Take it or leave it ; ).

} else if (superVTable) {
// Validate the entry against the matching entry from the superclass
// vtable.

const Entry *superEntry = nullptr;
for (auto &se : superVTable->getEntries()) {
if (se.Method.getOverriddenVTableEntry() == entry.Method.getOverriddenVTableEntry()) {
superEntry = &se;
break;
}
}

switch (entry.TheKind) {
case Entry::Normal:
case Entry::NormalNonOverridden:
assert(!superEntry && "non-root vtable entry must be inherited or override");
break;

case Entry::Inherited:
break;

case Entry::Override:
if (!superEntry)
break;

// The superclass entry must not prohibit overrides.
assert(superEntry->TheKind != Entry::NormalNonOverridden
&& "vtable entry overrides an entry that claims to have no overrides");
// TODO: Check the root vtable entry for the method too.
break;
}
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/TypeCheckDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,10 @@ NeedsNewVTableEntryRequest::evaluate(Evaluator &evaluator,
if (!isa<ClassDecl>(dc))
return true;

// Destructors always use a fixed vtable entry.
if (isa<DestructorDecl>(decl))
return false;

assert(isa<FuncDecl>(decl) || isa<ConstructorDecl>(decl));

// Final members are always be called directly.
Expand Down
17 changes: 14 additions & 3 deletions test/SIL/Parser/basic.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1688,11 +1688,22 @@ bb0:
return %1 : $()
}

class Foo2: Foo {}

// CHECK-LABEL: sil_vtable Foo {
// CHECK: #Foo.subscript!getter: {{.*}} : @Foo_subscript_getter
// CHECK: #Foo.subscript!setter: {{.*}} : @Foo_subscript_setter [override]
// CHECK: #Foo.subscript!getter: {{.*}} : @Foo_subscript_getter [nonoverridden]
// CHECK: #Foo.subscript!setter: {{.*}} : @Foo_subscript_setter
// CHECK: }
sil_vtable Foo {
#Foo.subscript!getter: @Foo_subscript_getter
#Foo.subscript!getter: @Foo_subscript_getter [nonoverridden]
#Foo.subscript!setter: @Foo_subscript_setter
}

// CHECK-LABEL: sil_vtable Foo2 {
// CHECK: #Foo.subscript!getter: {{.*}} : @Foo_subscript_getter [inherited]
// CHECK: #Foo.subscript!setter: {{.*}} : @Foo_subscript_setter [override]
// CHECK: }
sil_vtable Foo2 {
#Foo.subscript!getter: @Foo_subscript_getter [inherited]
#Foo.subscript!setter: @Foo_subscript_setter [override]
}
16 changes: 8 additions & 8 deletions test/SILOptimizer/basic-callee-printer.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ sil_vtable private_base {
}

sil_vtable private_derived {
#private_base.foo: @private_derived_foo
#private_base.foo: @private_derived_foo [override]
}

sil_vtable internal_base {
Expand All @@ -349,8 +349,8 @@ sil_vtable internal_base {
}

sil_vtable internal_derived {
#internal_base.foo: @internal_derived_foo
#internal_base.bar: @internal_derived_bar
#internal_base.foo: @internal_derived_foo [override]
#internal_base.bar: @internal_derived_bar [override]
}

sil_vtable public_base {
Expand All @@ -360,9 +360,9 @@ sil_vtable public_base {
}

sil_vtable public_derived {
#public_base.foo: @public_derived_foo
#public_base.bar: @public_derived_bar
#public_base.baz: @public_derived_baz
#public_base.foo: @public_derived_foo [override]
#public_base.bar: @public_derived_bar [override]
#public_base.baz: @public_derived_baz [override]
}

private protocol private_proto_1 {
Expand Down Expand Up @@ -659,8 +659,8 @@ sil_vtable SomeItem {
}

sil_vtable SomeChildItem {
#SomeItem.init!allocator: @SomeChildItem_allocator
#SomeItem.init!initializer: @SomeChildItem_initializer
#SomeItem.init!allocator: @SomeChildItem_allocator [override]
#SomeItem.init!initializer: @SomeChildItem_initializer [override]
#SomeChildItem.deinit!deallocator: @SomeChildItem_destructor
}

Expand Down
2 changes: 1 addition & 1 deletion test/SILOptimizer/dead_func_init_method.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ sil_vtable Base {
}

sil_vtable Derived {
#Base.init!allocator: @DerivedInit
#Base.init!allocator: @DerivedInit [override]
}


Expand Down
8 changes: 4 additions & 4 deletions test/SILOptimizer/devirt_access.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ sil_vtable X {
}

sil_vtable Y {
#X.ping: @_TFC14devirt_access21X4pingfS0_FT_Si // devirt_access2.X.ping (devirt_access2.X)() -> Swift.Int
#X.init!initializer: @_TFC14devirt_access21YcfMS0_FT_S0_ // devirt_access2.Y.init (devirt_access2.Y.Type)() -> devirt_access2.Y
#X.ping: @_TFC14devirt_access21X4pingfS0_FT_Si [inherited]
#X.init!initializer: @_TFC14devirt_access21YcfMS0_FT_S0_ [override]
}

sil_vtable A {
Expand All @@ -161,6 +161,6 @@ sil_vtable A {
}

sil_vtable B {
#A.ping: @_TFC14devirt_access21B4pingfS0_FT_Si // devirt_access2.B.ping (devirt_access2.B)() -> Swift.Int
#A.init!initializer: @_TFC14devirt_access21BcfMS0_FT_S0_ // devirt_access2.B.init (devirt_access2.B.Type)() -> devirt_access2.B
#A.ping: @_TFC14devirt_access21B4pingfS0_FT_Si [override]
#A.init!initializer: @_TFC14devirt_access21BcfMS0_FT_S0_ [override]
}
8 changes: 4 additions & 4 deletions test/SILOptimizer/devirt_access_ownership.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ sil_vtable X {
}

sil_vtable Y {
#X.ping: @_TFC14devirt_access21X4pingfS0_FT_Si // devirt_access2.X.ping (devirt_access2.X)() -> Swift.Int
#X.init!initializer: @_TFC14devirt_access21YcfMS0_FT_S0_ // devirt_access2.Y.init (devirt_access2.Y.Type)() -> devirt_access2.Y
#X.ping: @_TFC14devirt_access21X4pingfS0_FT_Si [inherited]
#X.init!initializer: @_TFC14devirt_access21YcfMS0_FT_S0_ [override]
}

sil_vtable A {
Expand All @@ -157,6 +157,6 @@ sil_vtable A {
}

sil_vtable B {
#A.ping: @_TFC14devirt_access21B4pingfS0_FT_Si // devirt_access2.B.ping (devirt_access2.B)() -> Swift.Int
#A.init!initializer: @_TFC14devirt_access21BcfMS0_FT_S0_ // devirt_access2.B.init (devirt_access2.B.Type)() -> devirt_access2.B
#A.ping: @_TFC14devirt_access21B4pingfS0_FT_Si [override]
#A.init!initializer: @_TFC14devirt_access21BcfMS0_FT_S0_ [override]
}
4 changes: 2 additions & 2 deletions test/SILOptimizer/devirt_access_serialized.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@ sil_vtable X {
}

sil_vtable Y {
#X.ping: @_TFC14devirt_access21X4pingfS0_FT_Si // devirt_access2.X.ping (devirt_access2.X)() -> Swift.Int
#X.init!initializer: @_TFC14devirt_access21YcfMS0_FT_S0_ // devirt_access2.Y.init (devirt_access2.Y.Type)() -> devirt_access2.Y
#X.ping: @_TFC14devirt_access21X4pingfS0_FT_Si [inherited]
#X.init!initializer: @_TFC14devirt_access21YcfMS0_FT_S0_ [inherited]
}
4 changes: 2 additions & 2 deletions test/SILOptimizer/devirt_access_serialized_ownership.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,6 @@ sil_vtable X {
}

sil_vtable Y {
#X.ping: @_TFC14devirt_access21X4pingfS0_FT_Si // devirt_access2.X.ping (devirt_access2.X)() -> Swift.Int
#X.init!initializer: @_TFC14devirt_access21YcfMS0_FT_S0_ // devirt_access2.Y.init (devirt_access2.Y.Type)() -> devirt_access2.Y
#X.ping: @_TFC14devirt_access21X4pingfS0_FT_Si [inherited]
#X.init!initializer: @_TFC14devirt_access21YcfMS0_FT_S0_ [override]
}
13 changes: 8 additions & 5 deletions test/SILOptimizer/devirt_override.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ class MD5 : Hash {
}


sil @_TFC4test4HashcfMS0_FT_S0_ : $@convention(method) (@owned Hash) -> @owned Hash

sil @_TFC4test3MD5cfMS0_FT_S0_ : $@convention(method) (@owned MD5) -> @owned MD5 {
bb0(%0 : $MD5):
release_value %0 : $MD5
Expand Down Expand Up @@ -64,14 +66,15 @@ bb0:
sil_vtable Hash {
#Hash.update: @_TFC9hash2_new4Hash6updatefS0_FT3MsgVs5UInt83CntSi_T_
#Hash.hash: @_TFC4test4Hash4hashfS0_FT3MsgVs5UInt8_T_
#Hash.init!initializer: @_TFC4test4HashcfMS0_FT_S0_
}

sil_vtable MD5 {
// vtable should be keyed with the least-derived method, otherwise
// devirtualizer will choose the wrong function ref.
#Hash.hash: @_TFC4test3MD54hashfS0_FT3MsgVs5UInt8_T_
#MD5.init!initializer: @_TFC4test3MD5cfMS0_FT_S0_
#Hash.update: @_TFC9hash2_new4Hash6updatefS0_FT3MsgVs5UInt83CntSi_T_
#Hash.hash: @_TFC4test3MD54hashfS0_FT3MsgVs5UInt8_T_ [override]
#Hash.init!initializer: @_TFC4test3MD5cfMS0_FT_S0_ [override]
#Hash.update: @_TFC9hash2_new4Hash6updatefS0_FT3MsgVs5UInt83CntSi_T_ [override]
}

class C {
Expand Down Expand Up @@ -127,9 +130,9 @@ sil_vtable C {
}

sil_vtable D {
#C.doIt: @_TFC1b1D4doItfS0_FT_T_
#C.doIt: @_TFC1b1D4doItfS0_FT_T_ [override]
}

sil_vtable E {
#C.doIt: @_TFC1b1E4doItfS0_FT_T_
#C.doIt: @_TFC1b1E4doItfS0_FT_T_ [override]
}
8 changes: 4 additions & 4 deletions test/SILOptimizer/devirt_override_ownership.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ sil_vtable Hash {
sil_vtable MD5 {
// vtable should be keyed with the least-derived method, otherwise
// devirtualizer will choose the wrong function ref.
#Hash.hash: @_TFC4test3MD54hashfS0_FT3MsgVs5UInt8_T_
#Hash.hash: @_TFC4test3MD54hashfS0_FT3MsgVs5UInt8_T_ [override]
#MD5.init!initializer: @_TFC4test3MD5cfMS0_FT_S0_
#Hash.update: @_TFC9hash2_new4Hash6updatefS0_FT3MsgVs5UInt83CntSi_T_
#Hash.update: @_TFC9hash2_new4Hash6updatefS0_FT3MsgVs5UInt83CntSi_T_ [override]
}

class C {
Expand Down Expand Up @@ -129,9 +129,9 @@ sil_vtable C {
}

sil_vtable D {
#C.doIt: @_TFC1b1D4doItfS0_FT_T_
#C.doIt: @_TFC1b1D4doItfS0_FT_T_ [override]
}

sil_vtable E {
#C.doIt: @_TFC1b1E4doItfS0_FT_T_
#C.doIt: @_TFC1b1E4doItfS0_FT_T_ [override]
}
6 changes: 3 additions & 3 deletions test/SILOptimizer/devirt_speculative.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ sil_vtable Base {
}

sil_vtable Sub {
#Base.foo: @_TSubFooFun
#Base.exit: @Sub_exit
#Base.foo: @_TSubFooFun [override]
#Base.exit: @Sub_exit [override]
}

sil @test_objc_ancestry : $@convention(thin) (@guaranteed Base) -> () {
Expand Down Expand Up @@ -97,7 +97,7 @@ sil_vtable Base2 {
}

sil_vtable Sub2 {
#Base2.foo: @_TSub2FooFun
#Base2.foo: @_TSub2FooFun [override]
}

sil @test_objc_ancestry2 : $@convention(thin) (@guaranteed Base2) -> () {
Expand Down
26 changes: 13 additions & 13 deletions test/SILOptimizer/devirt_try_apply.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -848,43 +848,43 @@ sil_vtable D1 {
}

sil_vtable D3 {
#D1.f1: @_TFC16devirt_try_apply2D32f1fzT_CS_1A// devirt_try_apply.D3.f1 () throws -> devirt_try_apply.A
#D1.f1: @_TFC16devirt_try_apply2D32f1fzT_CS_1A [override] // devirt_try_apply.D3.f1 () throws -> devirt_try_apply.A
}


sil_vtable Base {
#Base.foo: @_TFC16devirt_try_apply4Base3foofS0_FzT_GSqVs5Int32_
#Base.boo1: @_TFC16devirt_try_apply4Base4boo1fS0_FzT_S0_
#Base.boo2: @_TFC16devirt_try_apply4Base4boo2fS0_FzT_GSqS0__
#Base.deinit!deallocator: @_TFC16devirt_try_apply4BaseD
#Base.init!initializer: @_TFC16devirt_try_apply4BasecfMS0_FT_S0_
#Base.deinit!deallocator: @_TFC16devirt_try_apply4BaseD
}

sil_vtable Derived1 {
#Base.foo: @_TFC16devirt_try_apply8Derived13foofS0_FzT_GSqVs5Int32_
#Base.boo1: @_TFC16devirt_try_apply8Derived14boo1fS0_FzT_S0_
#Base.boo2: @_TFC16devirt_try_apply8Derived14boo2fS0_FzT_GSqS0__
#Base.init!initializer: @_TFC16devirt_try_apply8Derived1cfMS0_FT_S0_
#Base.foo: @_TFC16devirt_try_apply8Derived13foofS0_FzT_GSqVs5Int32_ [override]
#Base.boo1: @_TFC16devirt_try_apply8Derived14boo1fS0_FzT_S0_ [override]
#Base.boo2: @_TFC16devirt_try_apply8Derived14boo2fS0_FzT_GSqS0__ [override]
#Base.init!initializer: @_TFC16devirt_try_apply8Derived1cfMS0_FT_S0_ [override]
#Derived1.deinit!deallocator: @_TFC16devirt_try_apply8Derived1D
}

sil_vtable Derived2 {
#Base.foo: @_TTVFC16devirt_try_apply8Derived23foofS0_FzT_Vs5Int32
#Base.boo1: @_TFC16devirt_try_apply8Derived24boo1fS0_FzT_S0_
#Base.boo2: @_TFC16devirt_try_apply8Derived24boo2fS0_FzT_S0_
#Base.init!initializer: @_TFC16devirt_try_apply8Derived2cfMS0_FT_S0_
#Base.foo: @_TTVFC16devirt_try_apply8Derived23foofS0_FzT_Vs5Int32 [override]
#Base.boo1: @_TFC16devirt_try_apply8Derived24boo1fS0_FzT_S0_ [override]
#Base.boo2: @_TFC16devirt_try_apply8Derived24boo2fS0_FzT_S0_ [override]
#Base.init!initializer: @_TFC16devirt_try_apply8Derived2cfMS0_FT_S0_ [override]
#Derived2.deinit!deallocator: @_TFC16devirt_try_apply8Derived2D
}

sil_vtable CP1 {
#CP1.foo: @_TFC16devirt_try_apply3CP13foofS0_FzT_Vs5Int32
#CP1.deinit!deallocator: @_TFC16devirt_try_apply3CP1D
#CP1.init!initializer: @_TFC16devirt_try_apply3CP1cfMS0_FT_S0_
#CP1.deinit!deallocator: @_TFC16devirt_try_apply3CP1D
}

sil_vtable CP2 {
#CP1.foo: @_TFC16devirt_try_apply3CP23foofS0_FzT_Vs5Int32
#CP1.init!initializer: @_TFC16devirt_try_apply3CP2cfMS0_FT_S0_
#CP1.foo: @_TFC16devirt_try_apply3CP23foofS0_FzT_Vs5Int32 [override]
#CP1.init!initializer: @_TFC16devirt_try_apply3CP2cfMS0_FT_S0_ [override]
#CP2.deinit!deallocator: @_TFC16devirt_try_apply3CP2D
}

Expand Down
Loading