Thread overview
[BUG] ? 'out' argument not being set correctly
Apr 15, 2004
Kris
Apr 16, 2004
Stewart Gordon
Apr 16, 2004
Ilya Minkov
Apr 17, 2004
Stewart Gordon
Apr 17, 2004
Ilya Minkov
Apr 16, 2004
Ilya Minkov
Apr 16, 2004
Kris
Apr 16, 2004
Kris
April 15, 2004
I've run into a very odd scenario whereby an 'out' parameter is not being set correctly:

        IReader get (out char[] x)
        {
                int     size;

                get(size);

                int read (void[] content)
                {
                        x = cast(char[]) content;
                        return content.length;
                }

                buffer.preserve (size, &read);
                return this;
        }

In the above case, out argument 'x' is not actually set properly (via the delegate). I've used this kind of thing before in D with the expected results, so I mangled the method to use a temporary instead:

        IReader get (out char[] x)
        {
                char[]  tmp;
                int     size;

                get(size);

                int read (void[] content)
                {
                        tmp = cast(char[]) content;
                        return content.length;
                }

                buffer.preserve (size, &read);
                x = tmp;
                return this;
        }

Now it works just as one would expect ...

Unfortunately, it's not easy to condense this one as it tends to always work in a simplified scenario <g>  The particular case involves several layers of inheritance, plus several interfaces;

Any ideas?

The (-g) assembly follow:

354:                  int read (void[] content)
00428DCC   enter       4,0
00428DD0   mov         dword ptr [ebp-4],eax
355:                  {
356:                          x = cast(char[]) content;
00428DD3   mov         edx,dword ptr [ebp+0Ch]
00428DD6   mov         eax,dword ptr [content]
00428DD9   mov         ecx,dword ptr [this]
00428DDC   mov         dword ptr [ecx+1Ch],eax
00428DDF   mov         dword ptr [ecx+20h],edx
357:                          return content.length;
00428DE2   mov         eax,dword ptr [content]
358:                  }
00428DE5   leave
00428DE6   ret         8

Seems to me that it's stuffing the 'content' somewhere into the 'this' object rather than through an out argument. Then again, I don't know what's actually stored at [this+0] ...

- Kris


April 16, 2004
Kris wrote:
> I've run into a very odd scenario whereby an 'out' parameter is not being
> set correctly:
> 
>         IReader get (out char[] x)
<snip>

That's an understatement.  On the whole, in/out/inout with arrays is broken, inconsistent, counter-intuitive or some linear combination of the three.

It seems that if it's in, it passes the (length, startAddress) tuple by
value, otherwise it passes a reference to this tuple.  But you're right, it seems a bug that out and inout behave identically.

Whichever option you use, you get read/write access to the array, and
the original array is modified in place.  The only difference seems to
be that, if within the function you change the length of the array, or
reassign it altogether, then it isn't reassigned back at the caller's
end.  (Of course, if a length change means moving it in memory altogether, then that's an as-yet-untested nother matter....)

The way I would do it is to allow the length and contents to have
separate in/out/inout specifiers.  The specifier for the length would be
in the [].

The various declarations would effectively be saying:

in char[in]
	Give me an array to look at
inout char[in]
	Give me the size and some starting data, and I'll change the
	data if I like
inout char[inout]
	Give me an array, and I'll do what I like with it
out char[in]
	Give me the size, and I'll give you the data
out char[inout]
	Give me a starting size, and I'll give you the data, changing
	the size if I want
out char[out]
	I'll give you an array from scratch

What do people think?  I might go into more detail soon.  Of course, defaults and backward compatibility are going to be issues to address whatever happens....

Stewart.

-- 
My e-mail is valid but not my primary mailbox.  Please keep replies on
on the 'group where everyone may benefit.

April 16, 2004
Stewart Gordon schrieb:
> What do people think?  I might go into more detail soon.  Of course, defaults and backward compatibility are going to be issues to address whatever happens....

All that you wrote here has *nothing* to do with the bug that Kris described. Besides, it is not wrong for out to behave the same way as inout. It may be implemented the same or differently.

-eye
April 16, 2004
Kris schrieb:

> Seems to me that it's stuffing the 'content' somewhere into the 'this'
> object rather than through an out argument. Then again, I don't know what's
> actually stored at [this+0] ...

I'm not very good at understanding what's happenning here. What i can say, is that you have an inner function, which probably treats x as a local variable of a surrounding function. Inner functions must get something like a reference to a struct, where some local variables of a surrounding function are stored. This struct is a part of stack frame of a surrounding function. Anyway, this definately needs some more special handling for outs, beyond trivial - either the compiler should put references to them into the struct, or some other way, i don't know.

BTW, what does "buffer.preserve (size, &read);" do? It looks very fishy to me. I can't imagine how it would allow you to make any use of an adress of inner function - since it uses it's parent's stack and may be called only from it!

-eye
April 16, 2004
"Ilya Minkov" <minkov@cs.tum.edu> wrote in message news:c5paf6$17v8$1@digitaldaemon.com...
> BTW, what does "buffer.preserve (size, &read);" do? It looks very fishy to me. I can't imagine how it would allow you to make any use of an adress of inner function - since it uses it's parent's stack and may be called only from it!

The &read is passed as a delegate argument, defined: int delegate (void[]). I understand that's the way to do it, and the compiler doesn't like it without the & ...

You are right about the stack-frame being stuffed into, and retrieved from, the containing Object. I misread the [] indirection in the original post.


April 16, 2004
(Please note that I'm using the pre-release v0.83, but saw this in v0.82
also).

Here's a standalone example that exposes the bug:

class Token
{
        private char[] value;

        char[] getValue()
        {
                return value;
        }

        void setValue(char[] value)
        {
                this.value = value;
        }

        char[] toString()
        {
                return value;
        }
}


class Cookie : Token
{
        private char[] name;

        char[] getName()
        {
                return name;
        }

        void setName(char[] name)
        {
                this.name = name;
        }

        void parse (CookieParser parser)
        {
                char[] test;

                parser.next (this, test);
                printf ("name: '%.*s' , value '%.*s', test '%.*s'\n",
getName(), getValue(), test);
        }
}


class Scanner
{
        int next (int delegate (char[]) dg)
        {
                return dg ("some input text");
        }
}

class CookieParser : Scanner
{
        void next (Cookie cookie, out char[] test)
        {
                // here's the delegate callback ...
                int work (char[] input)
                {
                       // this never makes it to the outside world; rather,
it
                       // overwrites some vTable entries such that the
following
                       // cookie.setX() toss an Access Violation
                        test = "from within delegate";

                        //cookie.setName ("name");
                        //cookie.setValue ("value");
                        return -1;
                }

                test = "before delegate";
                super.next (&work);
        }
}


int main(char[][] args)
{
        CookieParser parser = new CookieParser();
        Cookie cookie = new Cookie();

        cookie.parse (parser);
        return 0;
}

The printf() output confirms that the 'out' assignment from within the
delegate never makes it to the outside world and, more telling, it trashes
the vTable such that those following cookie.setName() and cookie.setValue()
methods point at invalid addresses.

Here's the disassembly, with cookie.setName() uncommented:

1419:                 {
1420:                         test = "from within delegate";
00402020   mov         edx,dword ptr ds:[40F314h]
00402026   mov         eax,[0040F310]
0040202B   mov         ecx,dword ptr [this]
0040202E   mov         dword ptr [ecx+0Ch],eax
00402031   mov         dword ptr [ecx+10h],edx

1421:                         cookie.setName ("name");
00402034   push        dword ptr ds:[40F324h]
0040203A   push        dword ptr ds:[40F320h]
00402040   mov         eax,dword ptr [ecx+10h]
00402043   mov         ebx,dword ptr [eax]
00402045   call        dword ptr [ebx+28h]
00402048   mov         eax,0FFFFFFFFh

1422:                         //cookie.setValue ("value");
1423:                         return -1;
1424:                 }
0040204D   pop         ebx
0040204E   leave
0040204F   ret         8

You can see that address for cookie.setName() is being extracted via the same location [ecx + 10h] that the assignment to 'test' is being written to <g>

- Kris



"Kris" <someidiot@earthlink.dot.dot.dot.net> wrote in message news:c5mnd7$b0e$1@digitaldaemon.com...
> I've run into a very odd scenario whereby an 'out' parameter is not being set correctly:
>
>         IReader get (out char[] x)
>         {
>                 int     size;
>
>                 get(size);
>
>                 int read (void[] content)
>                 {
>                         x = cast(char[]) content;
>                         return content.length;
>                 }
>
>                 buffer.preserve (size, &read);
>                 return this;
>         }
>
> In the above case, out argument 'x' is not actually set properly (via the delegate). I've used this kind of thing before in D with the expected results, so I mangled the method to use a temporary instead:
>
>         IReader get (out char[] x)
>         {
>                 char[]  tmp;
>                 int     size;
>
>                 get(size);
>
>                 int read (void[] content)
>                 {
>                         tmp = cast(char[]) content;
>                         return content.length;
>                 }
>
>                 buffer.preserve (size, &read);
>                 x = tmp;
>                 return this;
>         }
>
> Now it works just as one would expect ...
>


April 17, 2004
Ilya Minkov wrote:
> Stewart Gordon schrieb:
> 
>> What do people think?  I might go into more detail soon.  Of course, defaults and backward compatibility are going to be issues to address whatever happens....
> 
> All that you wrote here has *nothing* to do with the bug that Kris described.

All that you've quoted, yes.  All that I wrote, no.

> Besides, it is not wrong for out to behave the same way as inout. It may be implemented the same or differently.

How do you work that out?  My understanding is that default initialisers in D are supposed to be either consistently defined or consistently non-existent.  And so this should apply to out parameters as much as normal declarations.

Stewart.

-- 
My e-mail is valid but not my primary mailbox.  Please keep replies on on the 'group where everyone may benefit.
April 17, 2004
Stewart Gordon schrieb:

> Ilya Minkov wrote:
> 
>> Stewart Gordon schrieb:
>>
>>> What do people think?  I might go into more detail soon.  Of course, defaults and backward compatibility are going to be issues to address whatever happens....
>>
>> All that you wrote here has *nothing* to do with the bug that Kris described.
> 
> All that you've quoted, yes.  All that I wrote, no.

Believe me i read through it. And you have really not read (or understood) Kris's post. He has found a subtle implementation bug - possibly the compiler allowes s something that is technically not possible - but it has *nothing* to do with "brokenness" of inout. I think it's a bug of delegate, but i need to dig deeper in their implementation to really grip the matter. It looks as if inner function confuses self object and stack frame, but i'm not at all sure how it works in a delegate.

I don't like the idea of separating in/out specification for array dimension and contents, but i'll explain later when i have time why.

>> Besides, it is not wrong for out to behave the same way as inout. It may be implemented the same or differently.
> 
> How do you work that out?  My understanding is that default initialisers in D are supposed to be either consistently defined or consistently non-existent.  And so this should apply to out parameters as much as normal declarations.

Yes, you are right about initializers, i missed that. I was thinking of passing implementation aspect. When combining out/inout and inner functions, it gets slightly more spooky and bug prone on the side of the compiler.

-eye