www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 5212] New: Safer typesafe variadics

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

           Summary: Safer typesafe variadics
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: bearophile_hugs eml.cc



With DMD 2.050 this D2 program usually asserts at run-time:


class Foo {
    int[] args;
    this(int[] args_...) {
        args = args_;
    }
}
Foo foo() {
    return new Foo(1, 2, 3); // passes stack data to Foo
}
void main() {
    assert(foo().args == [1, 2, 3]);
}


That's the root cause of a bug found by a person in the D.learn group:
http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D.learn&article_id=22697

Just adding a dup inside the this() avoids the bug. Also, the bug is avoided if
Foo receives arguments like this:

Foo foo() {
    return new Foo([1, 2, 3]);
}

So this nature of typesafe variadic arguments is bug-prone. There are various
ways to avoid this problem.

One possible solution is to modify the semantics of this kind of arguments
pass, so the code inside the function foo always see an args array allocated on
the heap (it may perform a run-time test, and dup the given arguments only if
the data is on the stack, so in the second example it doesn't dup and saves a
little of RAM and running time).

To restore the original efficient semantics of the typesafe variadics a "scope"
attribute may be added (as the one used to avoid a closure):

class Foo {
    int[] args;
    this(scope int[] args_...) { // unsafe but fast
        args = args_;
    }
}

This is safer than the current semantics because, as usual in D design, the
safe way is the built-in one and the faster one is on request.

This is just one of the possible ways to avoid this problem.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 13 2010
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5212




12:27:42 PST ---

 Just adding a dup inside the this() avoids the bug. Also, the bug is avoided if
 Foo receives arguments like this:
 
 Foo foo() {
     return new Foo([1, 2, 3]);
 }


Is it valid to pass them byref here?
It's a nice performance trick, but... shouldn't arguments be on the stack?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 13 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5212


nfxjfg gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nfxjfg gmail.com



Ideally, the compiler would to proper data flow analysis and raise an error if
the reference to the variadic parameter array "escapes". Closures actually do
something similar, although that case seems to be simpler.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 13 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5212




13:47:14 PST ---


 Just adding a dup inside the this() avoids the bug. Also, the bug is avoided if
 Foo receives arguments like this:
 
 Foo foo() {
     return new Foo([1, 2, 3]);
 }


 
 Is it valid to pass them byref here?
 It's a nice performance trick, but... shouldn't arguments be on the stack?


Steven has pointed out that one can discern between variadic arguments and an
array by creating an overload for array.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 13 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5212




13:49:42 PST ---
A quote from Denis:

 I find a bit strange that the compiler accepts f([1,2,3])
 when the declaration reads eg void f(int[] ints...).


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 13 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5212




Extra note:

It's a problem of perception: typesafe variadic arguments don't look like
normal function arguments that you know are usually on the stack, they look
like dynamic arrays, and in D most dynamic arrays are allocated on the heap
(it's easy and useful to take a dynamic-array-slice of a stack allocated array,
but in this case the code shows that the slice doesn't contain heap data).

If your function has a signature similar to this one:

void foo(int[3] arr...) {

It's not too much hard to think that 'arr' is on the stack. But dynamic arrays
don't give that image:

void foo(int[] arr...) {

This is why I think it's better for the compiler to test if the arr data is on
the stack, and dup it otherwise (unless a 'scope' is present, in this case both
the test and allocation aren't present).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 15 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5212




12:11:46 PST ---
Variadic arguments are arguments, and arguments are on the stack.
T[] is not a dynamic array, it's a slice. Messing python idioms into D, while
those idioms were not implemented in D, is a bug-prone way of thinking
(misunderstanding).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 16 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5212




One of the many posts of the relative thread:
http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D&article_id=122157





 Variadic arguments are arguments, and arguments are on the stack.


A dynamic array argument is an argument, it is on the stack, and most times its
contents are on the heap. But not always.



 T[] is not a dynamic array, it's a slice.


This is not significant. What is significant is where the memory it points is
located.



 Messing python idioms into D, while
 those idioms were not implemented in D, is a bug-prone way of thinking
 (misunderstanding).


Python wasn't part of this discussion.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 16 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5212


hsteoh quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hsteoh quickfur.ath.cx



At the very least, the compiler should be improved to perform proper data flow
analysis for these kinds of cases, and detect the escaping reference to the
on-stack array.

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




P.S. This bug still happens in latest git (dmd 2.061). Here's another test
case:

        import std.conv;
        import std.stdio;

        class C {
                real[] val;
                this(real[] v...) {
                        val = v;
                }
                override string toString() {
                        return to!string(val);
                }
        }
        C f() {
                return new C(1.0);
        }
        void main() {
                auto e = f();
                writeln(e);
        }

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


Artem Borisovskiy <kolos80 bk.ru> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
                 CC|                            |kolos80 bk.ru
           Severity|enhancement                 |normal


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


Martin Nowak <code dawg.eu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |maxim maxim-fomin.ru



*** Issue 9527 has been marked as a duplicate of this issue. ***

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 17 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5212


Martin Nowak <code dawg.eu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |code dawg.eu
            Summary|Safer typesafe variadics    |no escape analysis for
                   |                            |typesafe variadic function
                   |                            |arguments



Without escape analysis these functions are not memory  safe.
Maybe we should think about this in conjunction with scope ref,


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 17 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5212




This topic was discussed again in D.learn:

http://forum.dlang.org/thread/whlxkcbvkldtmfezucts forum.dlang.org

An improvement of my idea is that when you use "scope":

this(scope int[] args_...) {

The compiler doesn't perfom the dup (and doesn't allow you to escape the data).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 19 2013