www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - v0.123 borks the vTable

reply "Kris" <fu bar.com> writes:
There's apparently been some notable changes in the way Interfaces are
handled within both v0.122 and v0.123. Code that compiled and executed
cleanly (and has done for ~10 months) breaks with both these builds.


dmd 0.121
-----------
# C:\D\mango\mango\io>dmd -c -I\d\mango Writer.d -v
# parse     Writer
# semantic  Writer
# semantic2 Writer
# semantic3 Writer
# code      Writer
# generating code for function 'this'
# generating code for function 'this'
# generating code for function 'error'
# generating code for function 'getBuffer'
# generating code for function 'setEncoder'
# generating code for function 'flush'
# generating code for function 'cr'
# generating code for function 'enableArrayPrefix'
# generating code for function 'length'
# generating code for function 'write'
# generating code for function 'encoder'
# generating code for function 'put'
# generating code for function 'put'
...
# generating code for function 'write'
# generating code for function '_staticCtor'



dmd 0.122
------------
# C:\D\mango\mango\io>dmd -c -I\d\mango Writer.d -v
# parse     Writer
# semantic  Writer
# Error: circular inheritance of interface



dmd 0.123
-----------
Same as 0.121, but tracing execution shows that calling through an interface
now goes off into la la land (actually, instead of going to the vtable it
went directly to the _printf C library routine!)


What happened, Walter?
May 19 2005
next sibling parent "Kris" <fu bar.com> writes:
DMD 0.124 has the same problem: compiles cleanly, but calls through
interfaces are borked.


"Kris" <fu bar.com> wrote in message news:d6il7f$2agd$1 digitaldaemon.com...
 There's apparently been some notable changes in the way Interfaces are
 handled within both v0.122 and v0.123. Code that compiled and executed
 cleanly (and has done for ~10 months) breaks with both these builds.


 dmd 0.121
 -----------
 # C:\D\mango\mango\io>dmd -c -I\d\mango Writer.d -v
 # parse     Writer
 # semantic  Writer
 # semantic2 Writer
 # semantic3 Writer
 # code      Writer
 # generating code for function 'this'
 # generating code for function 'this'
 # generating code for function 'error'
 # generating code for function 'getBuffer'
 # generating code for function 'setEncoder'
 # generating code for function 'flush'
 # generating code for function 'cr'
 # generating code for function 'enableArrayPrefix'
 # generating code for function 'length'
 # generating code for function 'write'
 # generating code for function 'encoder'
 # generating code for function 'put'
 # generating code for function 'put'
 ...
 # generating code for function 'write'
 # generating code for function '_staticCtor'



 dmd 0.122
 ------------
 # C:\D\mango\mango\io>dmd -c -I\d\mango Writer.d -v
 # parse     Writer
 # semantic  Writer
 # Error: circular inheritance of interface



 dmd 0.123
 -----------
 Same as 0.121, but tracing execution shows that calling through an

 now goes off into la la land (actually, instead of going to the vtable it
 went directly to the _printf C library routine!)


 What happened, Walter?

May 19 2005
prev sibling next sibling parent reply "Walter" <newshound digitalmars.com> writes:
"Kris" <fu bar.com> wrote in message news:d6il7f$2agd$1 digitaldaemon.com...
 What happened, Walter?

I don't know. But if you can make a small example, I can fix it, it'll go into the test suite, and it'll never happen again.
May 19 2005
parent reply "Kris" <fu bar.com> writes:
There's a second related segfault that I have an example for ~ there's four
layers of inheritance, with an embedded diamond.

Do you have an email I can send it to?



"Walter" <newshound digitalmars.com> wrote in message
news:d6ivqe$2jt9$1 digitaldaemon.com...
 "Kris" <fu bar.com> wrote in message

 What happened, Walter?

I don't know. But if you can make a small example, I can fix it, it'll go into the test suite, and it'll never happen again.

May 19 2005
parent "Walter" <newshound digitalmars.com> writes:
"Kris" <fu bar.com> wrote in message news:d6jdrj$2tca$1 digitaldaemon.com...
 There's a second related segfault that I have an example for ~ there's

 layers of inheritance, with an embedded diamond.

 Do you have an email I can send it to?

www.digitalmars.com/contact.html
May 20 2005
prev sibling parent reply "Kris" <fu bar.com> writes:
Here's a contrived example that aborts:

interface IWritable
{
        void write (IWriter writer);
}

interface IWriter
{
        IWriter put (IWritable x);
}

interface IPhantom : IWritable {}

interface INewline : IPhantom {}

class Newline : INewline
{
        void write (IWriter writer)
        {
                printf ("OK\n");
        }
}



// implementation

class Writer : IWriter
{
        IWriter put (IWritable x)
        {
                x.write (this);
                return this;
        }
}

class DisplayWriter : Writer
{
        alias Writer.put put;
}

class FlushWriter : DisplayWriter
{
        override IWriter put (IWritable x)
        {
               // have superclass handle the IWritable
                super.put (x);

                // flush output when we see a newline
                if (cast(Newline) x)
                   {
                   }

                return this;
        }
}


// a static instance
public static INewline NL;

static this ()
{
        NL = new Newline;
}


void test (IWriter w)
{
        // this works
        //w.put (new Newline);

        // this fails
        w.put (NL);
}


void main()
{
        test (new FlushWriter);
}

######################

C:\D\mango\mango\test>test
Object
Error: Access Violation

######################

This example actually calls _printf() directly instead of w.put(), which is
where the console output "Object" comes from.

There seems to be an issue with the use of a statically constructed NL.
Further, if the depth of interface inheritance is changed for NL (e.g.
derive directly from IWritable instead of IPhantom) then it doesn't break.
Nor does it break if you construct and pass a new Newline instance. BTW; I
recall this same bug appeared and was fixed in the 0.8x timeframe. I
couldn't reproduce an example at that time.

Any chance of an interim release to fix this, please?  Mango is completely
broken by this.


Finally, just an FYI: I noticed the codegen for interfaces has (recently)
become a bit bloated ~ here's a listing from the above code: there are tests
made on the Interface that used to occur, but were fixed sometime in the
middle of last year (they used to be performed when returning an Interface,
but that was cleaned up nicely). Now they appear to be back again?

Compiler flags are -release -g  (-O makes no real difference)

This first listing is converting an Interface to its superInterface
(INewline to IWritable):

67:   void test (IWriter w)
004020B8   enter       4,0
004020BC   mov         dword ptr [ebp-4],eax
68:   {
69:           // this works
70:           //w.put (new Newline);
71:
72:           // this fails
73:           w.put (NL);
004020BF   cmp         dword ptr [_D3bar2NLC3bar8INewline (00415760)],0
004020C6   je          _D3bar4testFC3bar7IWriterZv+1Ah (004020d2)
004020C8   mov         eax,[_D3bar2NLC3bar8INewline (00415760)]
004020CD   lea         ecx,[eax+8]
004020D0   jmp         _D3bar4testFC3bar7IWriterZv+1Ch (004020d4)
004020D2   xor         ecx,ecx
004020D4   push        ecx
004020D5   mov         eax,dword ptr [w]
004020D8   mov         edx,dword ptr [eax]
004020DA   call        dword ptr [edx+4]
74:   }
004020DD   leave
004020DE   ret

And this one is converting from an object into an implemented interface
(FlushWriter to IWriter):

78:   void main()
004020E0   enter       4,0
79:   {
80:           test (new FlushWriter);
004020E4   push        offset __Class_3bar11FlushWriter (00411354)
004020E9   call        __d_newclass (00402330)
004020EE   add         esp,4
004020F1   mov         dword ptr [_TMP5],eax
004020F4   test        eax,eax
004020F6   je          _Dmain+1Dh (004020fd)
004020F8   lea         eax,[eax+8]
004020FB   jmp         _Dmain+1Fh (004020ff)
004020FD   xor         eax,eax
004020FF   call        _D3bar4testFC3bar7IWriterZv (004020b8)
81:   }
00402104   leave
00402105   ret


This kind of thing used to be performed much more efficiently, and still is
when returning an Object as an Interface; as this partial listing from the
FlushWriter method shows:

53:                   return this;
00402084   mov         eax,dword ptr [this]
00402087   add         eax,8
54:           }
0040208A   leave
0040208B   ret         4

Back last year, this kind of return statement had the same odd tests as seen
in the other listings. Thankfully, those were cleaned up nicely. Can you
clean up these ones too, please?






"Kris" <fu bar.com> wrote in message news:d6il7f$2agd$1 digitaldaemon.com...
 There's apparently been some notable changes in the way Interfaces are
 handled within both v0.122 and v0.123. Code that compiled and executed
 cleanly (and has done for ~10 months) breaks with both these builds.


 dmd 0.121
 -----------
 # C:\D\mango\mango\io>dmd -c -I\d\mango Writer.d -v
 # parse     Writer
 # semantic  Writer
 # semantic2 Writer
 # semantic3 Writer
 # code      Writer
 # generating code for function 'this'
 # generating code for function 'this'
 # generating code for function 'error'
 # generating code for function 'getBuffer'
 # generating code for function 'setEncoder'
 # generating code for function 'flush'
 # generating code for function 'cr'
 # generating code for function 'enableArrayPrefix'
 # generating code for function 'length'
 # generating code for function 'write'
 # generating code for function 'encoder'
 # generating code for function 'put'
 # generating code for function 'put'
 ...
 # generating code for function 'write'
 # generating code for function '_staticCtor'



 dmd 0.122
 ------------
 # C:\D\mango\mango\io>dmd -c -I\d\mango Writer.d -v
 # parse     Writer
 # semantic  Writer
 # Error: circular inheritance of interface



 dmd 0.123
 -----------
 Same as 0.121, but tracing execution shows that calling through an

 now goes off into la la land (actually, instead of going to the vtable it
 went directly to the _printf C library routine!)


 What happened, Walter?

May 19 2005
next sibling parent "Walter" <newshound digitalmars.com> writes:
Thanks. I'll have a look.
May 19 2005
prev sibling parent reply "Walter" <newshound digitalmars.com> writes:
"Kris" <fu bar.com> wrote in message news:d6j1tk$2lbr$1 digitaldaemon.com...
 Here's a contrived example that aborts:

I've found the problem and fixed it.
 Finally, just an FYI: I noticed the codegen for interfaces has (recently)
 become a bit bloated ~ here's a listing from the above code: there are

 made on the Interface that used to occur, but were fixed sometime in the
 middle of last year (they used to be performed when returning an

 but that was cleaned up nicely). Now they appear to be back again?

Those tests are needed when an offset needs to be added to the 'this' pointer to get a pointer to the right vtbl[]. If the 'this' is null, it shouldn't get an offset added.
May 20 2005
next sibling parent reply "Kris" <fu bar.com> writes:
Right; but isn't that trying to salvage a use-case that's already broken? I
mean, you're working with a invalid null instance at that point - yes?


"Walter" <newshound digitalmars.com> wrote in message
news:d6lacb$1fbd$2 digitaldaemon.com...
 "Kris" <fu bar.com> wrote in message

 Here's a contrived example that aborts:

I've found the problem and fixed it.
 Finally, just an FYI: I noticed the codegen for interfaces has


 become a bit bloated ~ here's a listing from the above code: there are

 made on the Interface that used to occur, but were fixed sometime in the
 middle of last year (they used to be performed when returning an

 but that was cleaned up nicely). Now they appear to be back again?

Those tests are needed when an offset needs to be added to the 'this' pointer to get a pointer to the right vtbl[]. If the 'this' is null, it shouldn't get an offset added.

May 20 2005
parent reply "Walter" <newshound digitalmars.com> writes:
"Kris" <fu bar.com> wrote in message news:d6lfa6$1j0r$1 digitaldaemon.com...
 Right; but isn't that trying to salvage a use-case that's already broken?

 mean, you're working with a invalid null instance at that point - yes?

Casting a null class reference to an interface should produce a null, not a small offset value.
May 20 2005
parent "Kris" <fu bar.com> writes:
"Walter" <newshound digitalmars.com> wrote in message
news:d6lg1n$1jrk$1 digitaldaemon.com...
 "Kris" <fu bar.com> wrote in message

 Right; but isn't that trying to salvage a use-case that's already


 I
 mean, you're working with a invalid null instance at that point - yes?

Casting a null class reference to an interface should produce a null, not

 small offset value.

Arguably so. But then what should one do with -release? This can lead to quite a bit of bloat, and the extra jumps don't help any with pipeline bubbles in todays CPUs ... seems a shame to really penalize the usage of interfaces in this manner, especially with code that (a) has no null object references, or (b) does explicit checks for them in the most appropriate places, such as DbC; (c) the library function used for casting should do this check, but inline code? with full optimization enabled? We have to wonder ... The codegen used to be a whole lot better in these cases. For example, here's the codegen distinction between where CR is an object and where CR is an interface ~ CR implements an interface in both cases, which is expected by the called method: 2449: Stdout.put (CR); 0041ECD3 push dword ptr [_D5mango2io6Writer2CRC5mango2io5model7IWriter14INewlineWriter (00550e38)] 0041ECD9 mov eax,[_D5mango2io6Stdout6StdoutC5mango2io6Stdout13ConsoleWriter (005512b0)] 0041ECDE mov ecx,dword ptr [eax] 0041ECE0 call dword ptr [ecx+38h] 2449: Stdout.put (CR); 0041F093 cmp dword ptr [_D5mango2io6Writer2CRC5mango2io6Writer13NewlineWriter (00551e38)],0 0041F09A je _D8unittest14testTextWriterFZv+1Ah (0041f0a6) 0041F09C mov eax,[_D5mango2io6Writer2CRC5mango2io6Writer13NewlineWriter (00551e38)] 0041F0A1 lea ecx,[eax+10h] 0041F0A4 jmp _D8unittest14testTextWriterFZv+1Ch (0041f0a8) 0041F0A6 xor ecx,ecx 0041F0A8 push ecx 0041F0A9 mov eax,[_D5mango2io6Stdout6StdoutC5mango2io6Stdout13ConsoleWriter (005522b0)] 0041F0AE mov edx,dword ptr [eax] 0041F0B0 call dword ptr [edx+38h] These used to be *almost* identical. Should people avoid using interfaces, since their usage inccurs two wholly unecessary jumps for every function call that accepts one?
May 20 2005
prev sibling parent reply "Kris" <fu bar.com> writes:
Shall I test it against Mango?

(I just sent an email that has a return address)


"Walter" <newshound digitalmars.com> wrote in message
news:d6lacb$1fbd$2 digitaldaemon.com...
 "Kris" <fu bar.com> wrote in message

 Here's a contrived example that aborts:

I've found the problem and fixed it.
 Finally, just an FYI: I noticed the codegen for interfaces has


 become a bit bloated ~ here's a listing from the above code: there are

 made on the Interface that used to occur, but were fixed sometime in the
 middle of last year (they used to be performed when returning an

 but that was cleaned up nicely). Now they appear to be back again?

Those tests are needed when an offset needs to be added to the 'this' pointer to get a pointer to the right vtbl[]. If the 'this' is null, it shouldn't get an offset added.

May 20 2005
parent "Walter" <newshound digitalmars.com> writes:
"Kris" <fu bar.com> wrote in message news:d6lfpg$1jiu$1 digitaldaemon.com...
 Shall I test it against Mango?

I want to look at the package problem first.
May 20 2005