www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - A Riddle: what is wrong with this code using std.array.Appender?

reply FeepingCreature <feepingcreature gmail.com> writes:
class Class
{
     Appender!(int[]) app = null;
}

This is the most evil bug I've seen this year yet.

Hint: Appender is a struct, not a class. So what does "= null" 
do, when it appears as a default initializer?
Mar 25 2019
next sibling parent reply Andrea Fontana <nospam example.org> writes:
On Monday, 25 March 2019 at 14:58:08 UTC, FeepingCreature wrote:

 class Class
 {
     Appender!(int[]) app = null;
 }

 This is the most evil bug I've seen this year yet.

 Hint: Appender is a struct, not a class. So what does "= null" 
 do, when it appears as a default initializer?


Previously on dlang forum:
https://forum.dlang.org/post/suezctpobjjoanyummjm forum.dlang.org
Mar 25 2019
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Monday, 25 March 2019 at 15:44:08 UTC, Andrea Fontana wrote:

 Previously on dlang forum:


not the same!
Mar 25 2019
parent Andrea Fontana <nospam example.org> writes:
On Monday, 25 March 2019 at 15:45:56 UTC, Adam D. Ruppe wrote:

 On Monday, 25 March 2019 at 15:44:08 UTC, Andrea Fontana wrote:

 Previously on dlang forum:


 not the same!


Yep, but same topic :)
Mar 25 2019
prev sibling next sibling parent user1234 <user12324 12.de> writes:
On Monday, 25 March 2019 at 14:58:08 UTC, FeepingCreature wrote:

 class Class
 {
     Appender!(int[]) app = null;
 }

 This is the most evil bug I've seen this year yet.

 Hint: Appender is a struct, not a class. So what does "= null" 
 do, when it appears as a default initializer?


My bet is that null is converted to int[]

///
class Class
{
     enum int[] initializer = null;
     Appender!(int[]) app0 = initializer;
     Appender!(int[]) app1 = null;
     this()
     {
         assert(app0 == app1);
     }
}

void main()
{
     new Class;
}
///

because Appender ctor can take a int[] i.e a range, no only a 
single elem.
Mar 25 2019
prev sibling next sibling parent reply Meta <jared771 gmail.com> writes:
On Monday, 25 March 2019 at 14:58:08 UTC, FeepingCreature wrote:

 class Class
 {
     Appender!(int[]) app = null;
 }

 This is the most evil bug I've seen this year yet.

 Hint: Appender is a struct, not a class. So what does "= null" 
 do, when it appears as a default initializer?


I can't see the bug in this minimal example:

import std.array: Appender;

class Class
{
     Appender!(int[]) app = null;
}

void main()
{
     auto c = new Class();
     import std.stdio;
     writeln(c.app.data); //Prints "[]"
     c.app ~= 10;
     writeln(c.app.data); //Prints "[10]"
}

What's the problem?
Mar 25 2019
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 3/25/19 1:06 PM, Meta wrote:

 On Monday, 25 March 2019 at 14:58:08 UTC, FeepingCreature wrote:

 class Class
 {
     Appender!(int[]) app = null;
 }

 This is the most evil bug I've seen this year yet.

 Hint: Appender is a struct, not a class. So what does "= null" do, 
 when it appears as a default initializer?


 
 I can't see the bug in this minimal example:
 
 import std.array: Appender;
 
 class Class
 {
      Appender!(int[]) app = null;
 }
 
 void main()
 {
      auto c = new Class();
      import std.stdio;
      writeln(c.app.data); //Prints "[]"
      c.app ~= 10;
      writeln(c.app.data); //Prints "[10]"
 }
 
 What's the problem?


I have a feeling it's an aliasing thing -- like every app member in 
every class points at the same IMPL struct.

-Steve
Mar 25 2019
parent reply Meta <jared771 gmail.com> writes:
On Monday, 25 March 2019 at 17:32:13 UTC, Steven Schveighoffer 
wrote:

 I can't see the bug in this minimal example:
 
 import std.array: Appender;
 
 class Class
 {
      Appender!(int[]) app = null;
 }
 
 void main()
 {
      auto c = new Class();
      import std.stdio;
      writeln(c.app.data); //Prints "[]"
      c.app ~= 10;
      writeln(c.app.data); //Prints "[10]"
 }
 
 What's the problem?


 I have a feeling it's an aliasing thing -- like every app 
 member in every class points at the same IMPL struct.

 -Steve


Ouch, you're right.

import std.array: Appender;

class Class
{
     Appender!(int[]) app = null;
}

void testAppender(Class c)
{
     import std.stdio;
     writeln(c.app.data);
     c.app ~= 10;
     writeln(c.app.data);
}

void main()
{
     auto c1 = new Class();
     auto c2 = new Class();
     testAppender(c1);  //Prints [] and [10]
     testAppender(c2); //Prints [10] and [10, 10]
}

I find that a bit strange, since you'd think that Appender would 
initialize its payload on the first append; and it seems like it 
does if you look at the code (in Appender.ensureAddable). I'm not 
sure how it shakes out that the two Appenders end up sharing the 
same memory.
Mar 25 2019
next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Mon, Mar 25, 2019 at 06:21:12PM +0000, Meta via Digitalmars-d wrote:

 On Monday, 25 March 2019 at 17:32:13 UTC, Steven Schveighoffer wrote:


[...]

 I have a feeling it's an aliasing thing -- like every app member in
 every class points at the same IMPL struct.




[...]

 I find that a bit strange, since you'd think that Appender would
 initialize its payload on the first append; and it seems like it does
 if you look at the code (in Appender.ensureAddable). I'm not sure how
 it shakes out that the two Appenders end up sharing the same memory.


This is pretty bad; two Appenders sharing the same memory could
potentially be exploited to break immutable.


T

-- 
In a world without fences, who needs Windows and Gates? -- Christian Surchi
Mar 25 2019
parent Meta <jared771 gmail.com> writes:
On Monday, 25 March 2019 at 18:39:26 UTC, H. S. Teoh wrote:

 On Mon, Mar 25, 2019 at 06:21:12PM +0000, Meta via 
 Digitalmars-d wrote:

 On Monday, 25 March 2019 at 17:32:13 UTC, Steven Schveighoffer 
 wrote:


 [...]

 I have a feeling it's an aliasing thing -- like every app 
 member in every class points at the same IMPL struct.




 [...]

 I find that a bit strange, since you'd think that Appender 
 would initialize its payload on the first append; and it seems 
 like it does if you look at the code (in 
 Appender.ensureAddable). I'm not sure how it shakes out that 
 the two Appenders end up sharing the same memory.


 This is pretty bad; two Appenders sharing the same memory could 
 potentially be exploited to break immutable.


 T


Yes, this part of the language *needs* to be changed if D is 
going to claim that it's memory safe. Either things such as:

struct Test
{
     Object o = new Object();
}

needs to be disallowed, or the semantics need to be changed.
Mar 25 2019
prev sibling parent FeepingCreature <feepingcreature gmail.com> writes:
On Monday, 25 March 2019 at 18:21:12 UTC, Meta wrote:

 Ouch, you're right.

 import std.array: Appender;

 class Class
 {
     Appender!(int[]) app = null;
 }

 void testAppender(Class c)
 {
     import std.stdio;
     writeln(c.app.data);
     c.app ~= 10;
     writeln(c.app.data);
 }

 void main()
 {
     auto c1 = new Class();
     auto c2 = new Class();
     testAppender(c1);  //Prints [] and [10]
     testAppender(c2); //Prints [10] and [10, 10]
 }

 I find that a bit strange, since you'd think that Appender 
 would initialize its payload on the first append; and it seems 
 like it does if you look at the code (in 
 Appender.ensureAddable). I'm not sure how it shakes out that 
 the two Appenders end up sharing the same memory.


Yep, that's correct. If you look at the AST analysis in 
run.dlang, what ends up happening is the constructor for Appender 
is evaluated statically as a side effect of "= null" becoming "= 
Appender(null)" and produces a static initializer referencing a 
shared global payload.
Mar 25 2019
prev sibling parent Daniel Kozak <kozzi11 gmail.com> writes:
On Mon, Mar 25, 2019 at 4:00 PM FeepingCreature via Digitalmars-d <
digitalmars-d puremagic.com> wrote:


 class Class
 {
      Appender!(int[]) app = null;
 }

 This is the most evil bug I've seen this year yet.

 Hint: Appender is a struct, not a class. So what does "= null"
 do, when it appears as a default initializer?


Yeah I remember this bug. But to be honest I believe it is already fixed,
but few days ago I still write something like this to be sure:
class Class
{
     Appender!(int[]) app;

    this()
    {
        app = appender!(int[]);
    }
}
Mar 25 2019