D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 3914 - Struct as argument that fits in register has member accessed wrong
Summary: Struct as argument that fits in register has member accessed wrong
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: x86 Windows
: P2 critical
Assignee: No Owner
URL:
Keywords: patch, wrong-code
Depends on:
Blocks:
 
Reported: 2010-03-09 10:54 UTC by Aldo Nunez
Modified: 2014-02-15 02:44 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Aldo Nunez 2010-03-09 10:54:31 UTC
Depending on the build options and where in a function the struct parameter is used, referencing one member will actually reference another one.

Given the following program:

import std.stdio;

struct SS
{
	char a;
	char b;
	char c;
	char d;
//	char e;
}

void A( SS ss1 )
{
C:	//writeln( ss1.a, ss1.b, ss1.c, ss1.d );

	char temp;

	temp = ss1.a;
	ss1.a = ss1.d;
	ss1.d = temp;

	temp = ss1.b;
	ss1.b = ss1.c;
	ss1.c = temp;

B:	//writeln( ss1.a, ss1.b, ss1.c, ss1.d );

	temp = ss1.a;
	ss1.a = ss1.d;
	ss1.d = temp;

	temp = ss1.b;
	ss1.b = ss1.c;
	ss1.c = temp;

A:	writeln( ss1.a, ss1.b, ss1.c, ss1.d );
}

void main()
{
	SS ss3;

	ss3.a = 'A';
	ss3.b = 'L';
	ss3.c = 'D';
	ss3.d = 'O';

	A( ss3 );
}

If lines A, B, and C are enabled in the following pattern along with the given build options, you get these results:

	(none)	-g	-rel	-rel-O	-O	-g -O
A	ALDO	ALDO	ALDO	ALDA	ALDA	ALDA
						
B, A	ODLA	ODLA	ODLA	ADLA	ADLA	ADLA
	ALDO	ALDO	ALDO	ALDA	ALDA	ALDA
						
C, B, A	ALDA	ALDO	ALDA	ALDA	ALDA	ALDA
	ODLA	ODLA	ODLA	ODLA	ODLA	ODLA
	ALDO	ALDO	ALDO	ALDO	ALDO	ALDO

In particular, notice that field 'd' starts off wrong.
If I uncomment field 'e', or change one of them to an int, the problem goes away.

At C, "ALDO" should be printed.
At B, "ODLA" should be printed.
At A, "ALDO" should be printed.
Comment 1 Don 2010-03-11 12:51:37 UTC
Here's a slightly reduced test case which doesn't require any compiler flags. 
Not a regression, fails even on DMD0.165.
It's a problem with variadic function parameters and fastpar arguments.
------------
struct SS {
    char a, b, c, d;
}

void show(char[] args...) {
    assert(args[0]=='A');
    assert(args[1]=='L');
    assert(args[2]=='D');
    assert(args[3]=='O');
}

void A( SS ss ) {
    show( ss.a, ss.b, ss.c, ss.d );
}

void main(){
    SS ss3;
    ss3.a = 'A';
    ss3.b = 'L';
    ss3.c = 'D';
    ss3.d = 'O';
    A( ss3 );
}
Comment 2 Don 2010-03-12 05:51:40 UTC
PATCH: The struct ss is passed in EAX. The backend wants ss.b. It sees that ss.b is already in EAX, so it doesn't reload it into AX. But AX actually contains ss.a.
Solution: Disable the optimisation in cod1.loaddata() if it's a subsequent member of the struct.

Index: cod1.c
===================================================================
--- cod1.c	(revision 413)
+++ cod1.c	(working copy)
@@ -3453,6 +3453,7 @@
     // See if we can use register that parameter was passed in
     if (regcon.params && e->EV.sp.Vsym->Sclass == SCfastpar &&
 	regcon.params & mask[e->EV.sp.Vsym->Spreg] &&
+	!(e->Eoper == OPvar && e->EV.sp.Voffset > 0) && // Must be at the base of that variable
 	sz <= REGSIZE)			// make sure no 'paint' to a larger size happened
     {
 	reg = e->EV.sp.Vsym->Spreg;

=========================
Reduced test case:
=========================
struct Snake {
    short a, b;
}

void venom(short dd)
{
  assert(dd == 'B');
}

void serpent( Snake ss ) {
  venom(ss.b);
}

void main(){
    Snake s;
    s.a = 'A';
    s.b = 'B';
    serpent( s );
}
Comment 3 Walter Bright 2010-03-12 21:59:21 UTC
changeset 416
Comment 4 Don 2010-04-09 19:15:20 UTC
Fixed DMD1.058 and DMD2.042.