Bug 16980 – [REG2.072.0] wrong interface called

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
Linux
Creation time
2016-12-18T16:59:30Z
Last change time
2019-05-25T08:01:29Z
Keywords
wrong-code
Assigned to
No Owner
Creator
Sönke Ludwig

Comments

Comment #0 by sludwig — 2016-12-18T16:59:30Z
This bug is present in 2.072.0 and 2.072.1, but not on older compiler versions. The following code, instead of calling bar(), gets the vtable of the A interface and calls foo(). It just happens when the unrelated template instance (tinst) is present somewhere and if T is a template struct that calls bar() from its destructor. --- interface A { void foo(); } interface B { void bar(); } interface AB : A, B {} class C : AB { void foo() { assert(false, "Never called!"); } void bar() {} } struct T() { AB ab; ~this() { ab.bar(); // not OK, calls foo(); } } T!() tinst; // NOTE: the destructor of tinst is never run! void main() { T!() dst; dst.ab = new C; dst.ab.bar(); // OK, calls bar() } --- This is currently a blocker for the efforts of integrating the new revamped vibe.d core module with the rest of the library.
Comment #1 by code — 2016-12-27T14:40:11Z
Digger says https://github.com/dlang/dmd/pull/5500 is the culprit.
Comment #2 by code — 2016-12-29T00:43:48Z
For whatever reason the ab in the ab.bar() call of the dtor isn't casted to B (cast(B)dst.ab).bar(); // <- correct in main, offsets this (AB) by 8 byte ab.bar(); // <- broken, uses this (AB) w/o offset which is A, thus methods of B are called on A's vtbl should be (cast(B)ab).bar();
Comment #3 by code — 2016-12-29T01:11:45Z
Semantic for this.ab.bar() in the dtor runs first. The cast gets incorrectly optimized away, because the offset returned by cdto.isBaseOf(cdfrom, &offset) [¹] is 0 instead of 8. Maybe the struct size for one of the classes hasn't been finalized. [¹]: https://github.com/dlang/dmd/blob/3cbb4d8dee6c2be433a147996c445db0ccdc81db/src/optimize.d#L596
Comment #4 by code — 2016-12-29T13:14:27Z
So what happens is that the size finalization of aggregates (classes and structs) was moved to semantic2 in order to resolve various forward reference issues. The module level T!() tinst; declaration already triggers a FunctionDeclaration::semantic3 for the dtor during Module::semantic(). IIRC this is done for any template instances. What's missing is a call to ad.size/determineSize somewhere before determining the offset when the CastExp is optimized.
Comment #5 by github-bugzilla — 2016-12-29T16:59:03Z
Commits pushed to stable at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/bbd22804313fa37fe2848b7f3bc45f83f4ea8db8 fix Issue 16980 - wrong interface called - ensure that class/interface size is finalized before determining the interface offsets https://github.com/dlang/dmd/commit/6b294ff205772ef2ca0dfc3df90dc0b16c2a2772 Merge pull request #6383 from MartinNowak/fix16980 fix Issue 16980 - wrong interface called
Comment #6 by github-bugzilla — 2016-12-31T21:36:34Z
Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/bbd22804313fa37fe2848b7f3bc45f83f4ea8db8 fix Issue 16980 - wrong interface called https://github.com/dlang/dmd/commit/6b294ff205772ef2ca0dfc3df90dc0b16c2a2772 Merge pull request #6383 from MartinNowak/fix16980
Comment #7 by github-bugzilla — 2017-01-16T23:26:05Z
Commits pushed to newCTFE at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/bbd22804313fa37fe2848b7f3bc45f83f4ea8db8 fix Issue 16980 - wrong interface called https://github.com/dlang/dmd/commit/6b294ff205772ef2ca0dfc3df90dc0b16c2a2772 Merge pull request #6383 from MartinNowak/fix16980
Comment #8 by razvan.nitu1305 — 2018-12-27T12:02:45Z
Slightly modified example: interface A { void foo(); } interface B { void bar(); } interface AB : A, B {} class C : AB { void foo() { assert(false, "Never called!"); } void bar() {} } struct T() { AB ab; ~this() { ab.bar(); // not OK, calls foo(); } } static ~this() // calling the destructor still fails { tinst.__dtor(); } T!() tinst; // NOTE: the destructor of tinst is never run! void main() { T!() dst; dst.ab = new C; dst.ab.bar(); // OK, calls bar() }
Comment #9 by sahmi.soulaimane — 2019-05-25T08:01:29Z
> // calling the destructor still fails It fails because the field `ab` is not initialized, hence holds `null`. Try declaring `tinst` like this: ``` auto tinst = T!()(new C); ```