https://bugzilla.novell.com/show_bug.cgi?id=319930
User [EMAIL PROTECTED] added comment https://bugzilla.novell.com/show_bug.cgi?id=319930#c1 --- Comment #1 from Massimiliano Mantione <[EMAIL PROTECTED]> 2008-01-09 04:38:42 MST --- The test "bug-77127.exe" is failing again since r92011. Strictly speaking, the bug is not the same, it is a new one triggred by the C# compiler change and luckily discovered by the same test. But since it is related to "vtable inheritance", it can be handled here... Tracking the problem down, we see that the compiler change uses different names for property methods in the interface and in the class that implements it. The correct override is then specified in the ".override" section. In the test, Mono handles it correctly in the B class. However, when building the C vtable, the ".override" data is not visible anymore, and since the IA interface ends up in a different interface offset, its vtable section must be rebuilt. This rebuild does not reuse the correct method from the B vtable, so the wrong one is taken. IMHO, the obvious fix is to "inherit" all the interface vtable sections (copying them from the parent vtable to the new interface offset in the current vtable) before attempting to rebuild them: the rebuild should just add overrides. This would be the only way to get the correct method, because the ".override" data for class B is simply not visible anymore in the code when we build the vtable for class C. In doing so, I begun fighting a bit with the current code. My main issue is that the vtable building loop iterations are "guarded" by "if (vtable [io + l]) continue;" statements, and it definitely does not play well with the case when the method slots are pre-initialized. The current code has a very convoluted algorithm, like this: [1] Find the max vtable size, and init interface offsets. [2] Copy over (inherit) the parent vtable. [3] Foreach method in the ".override" section, if it is an interface method put it in the right slot *and* in the override hash table (only for the current class). [4] Foreach class "k", starting from the current towards its superclasses: [4.1] Foreach interface "ic" implemented by "k" (even if implemented indirectly, but *not* through class inheritance, only through interface inheritance): [4.1.1] If "k" is still the starting class: - Foreach method "im" in "ic": - Foreach method "cm" in "k": If the method names and signatures match, initialize the right slot. Else, if "ic" is outside of the "k" vtable, continue loop [4.1]. [4.1.2] Foreach method "im" in "ic": [4.1.2.1] If its slot is full, continue [4.1.2]. [4.1.2.2] Foreach class "k1" from "k" through all its superclasses: - Foreach method "cm" in "k1": If the fully qualified names and signatures of "im" and "cm" match, initialize the slot. [4.1.3] Foreach method "im" in "ic": [4.1.3.1] If its slot is full, continue [4.1.3]. [4.1.3.2] Foreach class "k1" like in [4.1.2.2]: - Foreach method "cm" in "k1": If the names-signatures of "im" and "cm" match, initialize the slot. [4.1.4] If class is not abstract, foreach method "im" in "ic": [4.1.4.1] If its slot is empty, foreach class "parent" from "class" through all its superclasses: [4.1.4.1.1] If its corresponding slot is full, copy it here. [4.1.4.1.2] IMHO, reading the code, a break statement (from loop [4.1.4.1]) is missing! After loop [4.1.4.1], if the slot is still empty, raise the loading error. [4.1.5] Foreach method "im" in "ic": If it's slot is -1, fix it (nobody knows why this is needed, or even why it can happen). [5] Foreach method "cm" in the current class: - If it in not "newslot", find its slot to override. - Else, give it a new slot. [6] A loop like [3], but for non-interface methods. [7] Apply the override hash table to the vtable. [8] Put the vtable in place (or, if it's identical, use the parent). The bad part, IMO, is section [4]. Perhaps it has been written in such a convoluted way because we did not have an array like "MonoClass.interfaces_packed", which collects all the implemented intefaces. The checks at [4.1.2.1] and [4.1.3.1] are the ones that cause me problems, but I think that the whole logic should be rewritten like this: [2] In section [2], besides copying the parent vtable, foreach interface "ic" implemented by "parent", if it has an interface offset larger than "parent->vtable_size", copy its vtable section at the correct interface offset inside the current vtable (this "inherits" the vtable section). [3] ... [4] Foreach interface "ic" implemented by the current class: [4.1] Foreach method "im" in "ic": [4.1.1] Foreach method "cm" in the current class: - If "cm" is a suitable override for "im" (by signature, fully qualified name, name or ".override" section), use it (potentially overwriting the slot written at [2]). [4.1.2] If the class is not abstract, check that the "im" slot is full (it must, because of [2]), otherwise raise the loading error. The simplicity comes mostly from the fact that in [2] and [4] we would loop on "MonoClass.interfaces_packed", which the previous code could not do. And this rewrite, beside making the code much more maintainable, would fix the bug. However, this is a tricky and risky change, even if we've got plenty of regression tests to check it... Comments? Am I missing something? -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. _______________________________________________ mono-bugs maillist - [email protected] http://lists.ximian.com/mailman/listinfo/mono-bugs
