Jump to page: 1 2
Thread overview
A Riddle: what is wrong with this code using std.array.Appender?
Mar 25, 2019
FeepingCreature
Mar 25, 2019
Andrea Fontana
Mar 25, 2019
Adam D. Ruppe
Mar 25, 2019
Andrea Fontana
Mar 25, 2019
user1234
Mar 25, 2019
Meta
Mar 25, 2019
Meta
Mar 25, 2019
H. S. Teoh
Mar 25, 2019
Meta
Mar 25, 2019
FeepingCreature
Mar 25, 2019
Daniel Kozak
March 25, 2019
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?

March 25, 2019
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
March 25, 2019
On Monday, 25 March 2019 at 15:44:08 UTC, Andrea Fontana wrote:
> Previously on dlang forum:

not the same!
March 25, 2019
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.
March 25, 2019
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 :)
March 25, 2019
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?
March 25, 2019
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
March 25, 2019
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.
March 25, 2019
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
March 25, 2019
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.
« First   ‹ Prev
1 2