Bug 6178 – Struct inside the AA are not init correctly
Status
RESOLVED
Resolution
FIXED
Severity
blocker
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2011-06-19T12:39:00Z
Last change time
2013-09-27T02:05:01Z
Keywords
pull, wrong-code
Assigned to
nobody
Creator
jsancio
Comments
Comment #0 by jsancio — 2011-06-19T12:39:02Z
Comment #1 by jsancio — 2011-06-19T12:45:41Z
Err -- Hit submit without a description...
I am going to assume that this was not already filed. If so please accept my apologies.
If you create an AA that points to a struct the is not initialized properly. In other word it is not the same as S.init. If this stays that way in can completely confuse the dtor. For example:
import std.stdio;
struct Bug
{
this(this) { writefln("%s postblit ctor called", var); }
void opAssign(Bug rhs) { writefln("%s opAssign called", var); }
~this() { writefln("%s dtor called", var); }
int var = 0;
}
void main()
{
Bug[int] map;
{
map[0] = Bug(1);
}
}
This yields the following output (you need -O or you will hit bug 6177):
dmd -w -O map_test.d && ./map_test
3 opAssign called
1 dtor called
3 dtor called
Where in the world is 3 coming from? You output may be slightly different.
Comment #2 by jsancio — 2011-06-19T12:47:14Z
Moving it to druntime since it probably belongs there.
Comment #3 by jsancio — 2011-06-19T13:20:15Z
I am getting around this issue with:
enum : uint { TOKEN = 987654321 }
struct Bug
{
~this()
{
if(token == TOKEN) { ...}
}
private uint token = TOKEN;
}
Comment #4 by kennytm — 2011-06-19T14:56:10Z
With an opAssign, an assignment to an AA struct will become something like
Bug __aatmp1234 = void; // <---
__aatmp1234.opAssign(1);
map.set(0, __aatmp1234);
map[0].___postblit();
the '3' is likely due to the '<---' line. See bug 2451.
Comment #5 by andrei — 2012-01-18T08:40:19Z
Update - with the dmd from the head the example causes
Internal error: backend/cgcs.c 162
If the destructor is commented out, the printed message on my machine is
-2084965984 opAssign called
Comment #6 by jsancio — 2012-01-18T18:22:55Z
Andrei,
Do you still get the compiler internal error with -0? See bug 6177.
Comment #7 by andrei — 2012-01-19T07:11:42Z
(In reply to comment #6)
> Andrei,
>
> Do you still get the compiler internal error with -0? See bug 6177.
I don't understand the question.
Comment #8 by hsteoh — 2012-03-16T08:10:42Z
I think he meant -O, not -0.
Comment #9 by clugdbug — 2012-12-13T09:35:46Z
On Linux, I see the bug only with 32 bits, it works OK with 64 bits.
With -m64 and -m64 -O, I get
0 opAssign called
1 dtor called
0 dtor called
whereas with -m32 I get
-142997715 opAssign called
1 dtor called
-142997715 dtor called
and with -m32 -O
2 opAssign called
1 dtor called
2 dtor called
Comment #10 by maxim — 2012-12-13T09:59:34Z
*** Issue 9084 has been marked as a duplicate of this issue. ***
Comment #11 by lovelydear — 2012-12-28T05:58:45Z
Different output for different compilers:
http://dpaste.dzfl.pl/c94eea76
Success with GDC 2.060 both 32/64 bits
0 opAssign called
1 dtor called
0 dtor called
Fails identically with LDC 2.060 32 and 64 bits
-1 opAssign called
1 dtor called
-1 dtor called
Success with DMD 2.060 32 bits, fails in 64 bits
Compilation error with Git HEAD 32 bits and failure in 64 bits
Comment #12 by dmitry.olsh — 2013-01-15T14:19:41Z
I've just hit another case of this bug on Win32.
It's a major impediment for new std.uni as it may result in memery corruption
if a set of code points is written to AA like this:
//this one calls destructor twice and no postblit and causes memory corruption
props["Set"] = CodepointSet(a1, a2+1);
//While this one is fine (it calls postblit):
auto set = CodepointSet(a1, a2+1);
props["Set"] = set;
There is no opAssign only postblit + destructor.
The problem is that in the real world the destructor is tricky and
expects at least T.init or some valid object else it blows up quite nasty.
Comment #13 by maxim — 2013-01-16T01:18:30Z
Well, seems in 2.061 the behavior was changed.
Without any options, the output is like:
-1154412584 opAssign called (big value)
1 dtor called
-1154412584 dtor called
With -release or -noboundscheck the output is more robust:
0 opAssign called
1 dtor called
0 dtor called
With -O option output is:
32638 opAssign called // close to signed short int max
1 dtor called
32638 dtor called
Comment #14 by dmitry.olsh — 2013-01-16T01:46:18Z
(In reply to comment #13)
> Well, seems in 2.061 the behavior was changed.
>
> Without any options, the output is like:
>
> -1154412584 opAssign called (big value)
> 1 dtor called
> -1154412584 dtor called
>
> With -release or -noboundscheck the output is more robust:
>
> 0 opAssign called
> 1 dtor called
> 0 dtor called
>
> With -O option output is:
>
> 32638 opAssign called // close to signed short int max
> 1 dtor called
> 32638 dtor called
This is shitty but I'll use -release for the moment. BTW it seems
to require both module with struct *and* the one where AA is used to be compiled wiht -release.
Comment #15 by maxim — 2013-01-16T02:18:45Z
(In reply to comment #14)
> This is shitty but I'll use -release for the moment. BTW it seems
> to require both module with struct *and* the one where AA is used to be
> compiled wiht -release.
Than this a part of a bigger shit.
/* Known as a problem of filling newly created space of AA array
with T.init before assigning actual object.
If operation is interrupted, this leads to AA array containing
"orphan" T.init objects by no reason. Was reported in BZ.
*/
import std.stdio;
int foo()
{
throw new Exception("");
}
int[int] array;
void main()
{
try
{
array[1] = foo();
}
catch(Exception e)
{
}
writeln(array);
}
Compiling with -O => [1:0]
Compiling with -release => []
Compiling with -noboundscheck => []
So, it appears that there is not only bug with AA assignment, but the bug depends on compiler options.
Comment #16 by k.hara.pg — 2013-04-04T03:11:17Z
(In reply to comment #15)
> Than this a part of a bigger shit.
>
> /* Known as a problem of filling newly created space of AA array
> with T.init before assigning actual object.
> If operation is interrupted, this leads to AA array containing
> "orphan" T.init objects by no reason. Was reported in BZ.
> */
> import std.stdio;
>
> int foo()
> {
> throw new Exception("");
> }
>
> int[int] array;
>
> void main()
> {
> try
> {
> array[1] = foo();
> }
> catch(Exception e)
> {
>
> }
> writeln(array);
> }
>
> Compiling with -O => [1:0]
> Compiling with -release => []
> Compiling with -noboundscheck => []
>
> So, it appears that there is not only bug with AA assignment, but the bug
> depends on compiler options.
It was bug 3825, and has already fixed in 2.063 (git head).
Comment #17 by k.hara.pg — 2013-09-07T10:05:52Z
*** Issue 5234 has been marked as a duplicate of this issue. ***
Comment #18 by k.hara.pg — 2013-09-07T10:06:57Z
*** Issue 7503 has been marked as a duplicate of this issue. ***
Comment #19 by k.hara.pg — 2013-09-07T10:07:18Z
*** Issue 10356 has been marked as a duplicate of this issue. ***
Comment #20 by k.hara.pg — 2013-09-07T10:37:10Z
(In reply to comment #4)
> With an opAssign, an assignment to an AA struct will become something like
>
> Bug __aatmp1234 = void; // <---
> __aatmp1234.opAssign(1);
> map.set(0, __aatmp1234);
> map[0].___postblit();
>
> the '3' is likely due to the '<---' line. See bug 2451.
This is the root cause. If the value struct in AA has opAssign method, it would be called on uninitialized object.
For such structs, AA value setting should do additional work to distinguish initializing and assignment.
struct S1 {
void opAssign(S1 rhs) {}
}
S1[int] aa1;
aa1[1] = S1(); // [*]
I think the line at [*] should behave as follows:
1. If the key doesn't exist in aa yet, so it should work as initializing.
2. If the key already exists in aa, so it should work as assignment
(== opAssign call).
Comment #21 by k.hara.pg — 2013-09-08T03:28:06Z
(In reply to comment #20)
> For such structs, AA value setting should do additional work to distinguish
> initializing and assignment.
>
> struct S1 {
> void opAssign(S1 rhs) {}
> }
>
> S1[int] aa1;
> aa1[1] = S1(); // [*]
>
> I think the line at [*] should behave as follows:
> 1. If the key doesn't exist in aa yet, so it should work as initializing.
> 2. If the key already exists in aa, so it should work as assignment
> (== opAssign call).
https://github.com/D-Programming-Language/dmd/pull/2539https://github.com/D-Programming-Language/phobos/pull/1554
Comment #22 by github-bugzilla — 2013-09-14T13:52:10Z