www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 1069] New: No error on type mismatch with varargs

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1069

           Summary: No error on type mismatch with varargs
           Product: D
           Version: 1.009
          Platform: PC
        OS/Version: Windows
            Status: NEW
          Keywords: accepts-invalid
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla digitalmars.com
        ReportedBy: sean f4.ca


If a vararg function is defined like so:

    void add( void delegate()[] procs... )

the compiler will not complain when delegates are added without the leading
address qualifier, but an error will occur when the delegate is run in some
cases.  Here is an example:

    class C
    {
        this()
        {
            add( fn );
        }

        void fn()
        {
            printf( "hello\n" );
        }
    }

    class Elem
    {
        void delegate() proc;
        Elem next;
    }

    Elem first;

    void add( void delegate()[] procs... )
    {
        foreach( p; procs )
        {
            auto e = new Elem;
            e.proc = p;
            e.next = first;
            first = e;
        }
    }

    void call()
    {
        for( Elem e = first; e; e = e.next )
            e.proc();
    }

    void main()
    {
        auto val = new C;
        call();
    }

C:\code\src\d\test>dmd test
c:\bin\dmd\bin\..\..\dm\bin\link.exe test,,,user32+kernel32/noi;

C:\code\src\d\test>test
Access Violation

C:\code\src\d\test>


--
Mar 19 2007
next sibling parent reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1069





------- Comment #1 from fvbommel wxs.nl  2007-03-19 15:14 -------
Actually, I think the compiler may be right here. Your code invokes undefined
behavior but is otherwise legal and the compiler has every right to
successfully compile this into a program that crashes without complaining.

Let me explain:
What you're seeing is an unfortunate side-effect of the combination of lazy
[void delegate()] arguments with the fact that a function name doesn't evaluate
to a pointer/delegate to that function but to its return value (i.e. property
syntax).
Your 'add( fn );' adds an implicit delegate that calls this.dg() (because of
property syntax). (It's equivalent to 'add( { fn() } );')
Since this implicit delegate refers to the stack frame of the constructor, it's
invalid after the constructor returns. Your call() then calls an invalidated
delegate, and your program crashes.

So as you mentioned, to do what you want you should use 'add( &fn );'.
What you didn't realize was that the _reason_ this works is that you're then
passing a method delegate instead of an (anonymous) inner function delegate.


--
Mar 19 2007
parent reply Sean Kelly <sean f4.ca> writes:
d-bugmail puremagic.com wrote:

 What you're seeing is an unfortunate side-effect of the combination of lazy
 [void delegate()] arguments with the fact that a function name doesn't evaluate
 to a pointer/delegate to that function but to its return value (i.e. property
 syntax).


And here I thought lazy parameters had to be marked 'lazy'.  That's 
annoying.  Thanks :-)
Mar 19 2007
parent Frits van Bommel <fvbommel REMwOVExCAPSs.nl> writes:
Sean Kelly wrote:

 d-bugmail puremagic.com wrote:

 What you're seeing is an unfortunate side-effect of the combination of 
 lazy
 [void delegate()] arguments with the fact that a function name doesn't 
 evaluate
 to a pointer/delegate to that function but to its return value (i.e. 
 property
 syntax).


 
 And here I thought lazy parameters had to be marked 'lazy'.  That's 
 annoying.  Thanks :-)


This is the one exception where 'implicit lazy' was kept when the 
keyword was introduced.
Mar 19 2007
prev sibling next sibling parent reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1069


point14 magma.ca changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |point14 magma.ca




------- Comment #4 from point14 magma.ca  2007-03-19 23:28 -------
I have expanded the example to show a bit more of the problem an end user can
have (sorry I have tango hard-coded, just replace with printf):

import tango.io.Stdout;

void another() {
  Stdout("Another one\n");
}

class C
{   
    this()
    {   
        add(&fn );     // Needed here
        add(another);  // Not needed here!!!!
    }

    void fn()
    {   
        Stdout( "hello\n" );
    }
}

class Elem
{   
    void delegate() proc;
    Elem next;
}

Elem first;

void add( void delegate()[] procs... )
{   
    foreach( p; procs )
    {   
        auto e = new Elem;
        e.proc = p;
        e.next = first;
        first = e;
    }
}

void call()
{   
    for( Elem e = first; e; e = e.next )
        e.proc();
}

void main()
{   
    auto val = new C;
    add(another);     // Not here!
    call();
}

This code works because by trial and error I have put the "&" in the right
places. Can this kind of segfault be avoided be detecting it at the compile
stage?

Regardless of the answer, I think a diagram (or more details) of what Frits
explains in the first reply would help me understand. Thanks.


--
Mar 19 2007
parent Frits van Bommel <fvbommel REMwOVExCAPSs.nl> writes:
d-bugmail puremagic.com wrote:

 I have expanded the example to show a bit more of the problem an end user can
 have (sorry I have tango hard-coded, just replace with printf):
 


[snip]

     this()
     {   
         add(&fn );     // Needed here


Correct


         add(another);  // Not needed here!!!!


This happens to look like it does what it should, but in reality it 
still ads a delegate that will be invalid after the constructor returns. 
That delegate just happens not to reference the stackframe of the 
constructor, and thus not crash, because all it does is call a free 
function.
It's still undefined behavior, but undefined behavior doesn't 
necessarily means it'll crash (just that it *may* crash, or worse, and 
if it does it's your fault).

Also, adding a & won't help in this case, as it'll be a function pointer 
instead of a delegate...


     }


[snip]

 void main()
 {   
     auto val = new C;
     add(another);     // Not here!


Correct, but only because the implicitly formed delegate literal happens 
not to go out of scope before it's last called. If for instance the 
module had a 'static ~this()' that would call 'call()' after main ended, 
it'd be undefined behavior.


     call();
 }
 
 This code works because by trial and error I have put the "&" in the right
 places.


That's not really the right strategy...
Just because it doesn't crash doesn't mean it does what you think it 
does, or that it won't crash the next time you upgrade/switch compilers 
(or just change which command-line switches to give to your current one)...


 Can this kind of segfault be avoided be detecting it at the compile
 stage?


At least partially, yes. Google for "escape analysis" (Mostly applied to 
pointers, which have essentially the same problem when pointing to local 
variables).
It may not possible to catch 100% of all cases (without false positives) 
though, and it might be a lot of work to implement (not that I'm an 
expert in this).


 Regardless of the answer, I think a diagram (or more details) of what Frits
 explains in the first reply would help me understand. Thanks.


I'm not really good with diagrams.
Just remember: don't use delegates to local functions/delegate literals 
(including the implicit kind discussed here), pointers to local 
variables or references to 'scope' objects after the function they were 
created in returns.
Mar 20 2007
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1069





------- Comment #6 from point14 magma.ca  2007-04-11 22:13 -------
In the context of the example above, here is another flavor:

class C
{   
    this()
    {   
        add({Stdout("inline").newline;})
    }

... [rest as above]

Would this add a delegate that will be invalid after the constructor returns
and thus I should avoid doing it?
If yes, is this something that would ever be changed in D such that is would be
"okay" to do?

Thanks.


--
Apr 11 2007
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1069


yebblies <yebblies gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yebblies gmail.com
           Platform|x86                         |All
            Version|1.009                       |D1
         OS/Version|Windows                     |All
           Severity|normal                      |enhancement


--- Comment #7 from yebblies <yebblies gmail.com> 2012-01-30 14:25:06 EST ---
This is working as designed, although it is bug prone.
It might be possible for the compiler to convert a delegate that calls a member
function into a delegate to the member function, avoiding this bug.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2012
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1069


timon.gehr gmx.ch changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |timon.gehr gmx.ch
         Resolution|                            |WORKSFORME


--- Comment #8 from timon.gehr gmx.ch 2012-01-31 09:41:11 PST ---
(In reply to comment #7)

 This is working as designed, although it is bug prone.
 It might be possible for the compiler to convert a delegate that calls a member
 function into a delegate to the member function, avoiding this bug.


It was not working as designed, but is now. The current behavior (DMD 2.057) is
what Sean expects. This is because implicit conversions to void delegate() have
been scrapped. Safe to close now.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 31 2012