2773 – ICE(go.c) array assignment through a pointer, only with -O.

D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 2773 - ICE(go.c) array assignment through a pointer, only with -O.
Summary: ICE(go.c) array assignment through a pointer, only with -O.
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D1 (retired)
Hardware: x86 Windows
: P2 critical
Assignee: Walter Bright
URL:
Keywords: ice-on-valid-code, patch
Depends on:
Blocks:
Reported: 2009年03月31日 10:57 UTC by ponce
Modified: 2014年04月18日 09:12 UTC (History)
1 user (show)

See Also:


Attachments
Reduced test case. (339 bytes, text/plain)
2009年04月03日 14:10 UTC, Unknown W. Brackets
Details
Workaround patch: make endless iteration not an error. (1.92 KB, patch)
2009年04月03日 21:22 UTC, Unknown W. Brackets
Details | Diff
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description ponce 2009年03月31日 10:57:57 UTC
When compiling with dmd -O -inline -release, dmd hangs up and I get the following error message. 
I did'nt succeeded in isolating the problem. The bug appear only with all three arguments (-O -inline -release).
Internal Error : ..\ztc\go.c 243
The corresponding line in the back-end source code is :
243 : assert(++iter < 200);	/* infinite loop check		 */
Comment 1 Walter Bright 2009年04月01日 14:00:31 UTC
I cannot do anything with this without a reproducible example.
But I suggest that to work around, split your large function into a couple smaller ones.
Comment 2 Unknown W. Brackets 2009年04月03日 14:10:50 UTC
Created attachment 312 [details] 
Reduced test case.
Attached is a reduced test case (can't spend time on it now but wanted to attach this.)
Compile with:
dmd -O -inline -release -c 2773_reduced.d
Bug occurs in DMD 1.041 and DMD 2.026, most likely in 2.027 as well.
-[Unknown]
Comment 3 Gide Nwawudu 2009年04月03日 16:23:19 UTC
The error is different in DMD 2.027.
dmd -O -inline -release -c 2773_reduced.d
Internal error: ..\ztc\go.c 243
Comment 4 Unknown W. Brackets 2009年04月03日 21:22:33 UTC
Created attachment 313 [details] 
Workaround patch: make endless iteration not an error.
Please note: this patch does not fix this bug.
I suggest dropping the assert for iter, and instead treating it the same as the clock timeout. While this makes this class of bug less discoverable, I propose that it's better that it compiles - at least some functions will be optimized.
Regarding this bug - it keeps moving an equation to optimize it, so each time the loop runs it's got more changes. Unfortunately, the innards of the backend are still a bit beyond me, so I can't see why.
Note that the second call to pointer.clear() can be a call to another method, on the same struct, as long as that method does something (anything.)
Also, reducing the size of the static array below 3 (or making it dynamic) solves it, but a bigger one still dies. Changing the code within clear() to any other operation also solves it. And the struct has to be within a class, returned from a method.
-[Unknown]
Comment 5 Don 2009年05月20日 06:49:00 UTC
Here's a slightly reduced test case, which only requires -O (doesn't need -inline -release), and ICEs on both D1 and D2. On D2, you still get an ICE if you remove all references to the class, and just set S2773* pointer = &dat.
struct S2773{
 int[4] data;	
}
S2773 dat;
class C2773 {
 S2773* get()	{ return &dat; }
}
void main()
{
 C2773 inst = null;
 S2773* pointer = inst.get();
 pointer.data[] = 0;
 pointer.data[] = 0;
}
Comment 6 Don 2009年08月31日 08:23:36 UTC
This isn't a regression. It failed on DMD1.022 as well, which was released in mid-2007, almost 2 years before this bug report.
Comment 7 Don 2009年10月05日 03:12:06 UTC
Even simpler test case, this one fails on both D1.033 and 1.048.
int[4]* get()	{ return null; }
void main()
{
 int[4]* p = get();
 (*p)[] = 0;
 (*p)[] = 0;
}
Comment 8 Don 2009年10月06日 00:17:14 UTC
An even simpler test case shows something interesting: it happens only when there's an array assignment of 0, followed by another use of the same variable.
An array assignment to 0 is an OPmemset operation in the backend.
int* get() { return null; }
void main(){
 int* p = get();
 p[0..3] = 0; // must be = 0!! Doesn't happen with any other number.
 p[0] = 7;
}
ANALYSIS:
This is an OPmemset fight! In the optimisation loop, there's a localize step which rearranges the assignment, and there's a canonicalize step which sets it back the way it was before....
cgelem.c, elmemxxx() line 702 replaces ((e op= v) op e2) with ((e op=v), (e op e2))
ie, (p = get()), p) memset 0. ---> ((p = get()), p memset 0.
glocal.c, local_exp() replaces
p = get(); p memset 0; ---> (p = get(), p) memset 0
So it just keeps going round in circles.
The fight can be broken up by changing cgelem.c elmemxxx() line 701 
to avoid doing the first replacement:
-			if (e1->Eoper == OPcomma || OTassign(e1->Eoper))
+			if (e1->Eoper == OPcomma)
This probably isn't correct, there may be somewhere this particular canonicalisation is important. But without the DMC test suite, I can't tell.
(Note that the comments in the code only refer to the OPcomma transformation, not the assignment one, so I presume the assignment was a later addition).
Comment 9 Walter Bright 2009年10月13日 14:10:07 UTC
Fixed dmd 1.049 and 2.034


AltStyle によって変換されたページ (->オリジナル) /