optimization/4061: Optimization bug in arm-unknown-elf cross-build of gcc 3.1

optimization/4061: Optimization bug in arm-unknown-elf cross-build of gcc 3.1

[parag]

>Number:         4061
>Category:       optimization
>Synopsis:       Optimization bug in arm-unknown-elf cross-build of gcc 3.1
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    unassigned
>State:          open
>Class:          wrong-code
>Submitter-Id:   net
>Arrival-Date:   Sun Aug 19 01:06:01 PDT 2001
>Closed-Date:
>Last-Modified:
>Originator:     Parag Patel
>Release:        3.1 20010818 (experimental)
>Organization:
CodeGen, Inc.
>Environment:
System: SunOS blade 5.8 Generic_108528-08 sun4u sparc SUNW,Sun-Blade-100
Architecture: sun4
	Sun Blade 100 (Solaris 8)
	C compiler: gcc version 2.95.2 19991024 (release)
	(I also tried gcc 3.0 with identical results.)
	
host: sparc-sun-solaris2.8
build: sparc-sun-solaris2.8
target: arm-unknown-elf
configured with: ../../egcs/configure --with-newlib --with-gnu-ld --with-gnu-as --target=arm-unknown-elf --enable-languages=c
>Description:
	Compile the attached file with:
		arm-unknown-elf-gcc -Wall -I- -O -S $*
	The resulting code incorrectly rejects "hdr->htype"s with
	the correct value of "1".  However, swap the two lines (marked
	below with "XXX") and it works!

	This problem also occurs in gcc 2.95.3 with the same target.  I
	didn't tried 2.95.2 as that didn't support ELF, if I recall
	correctly.

>How-To-Repeat:
	Test file is appended below.  I've minimized as much as I could
	from the original code, and removed all #included files.  The
	diff of the good and bad assembly is also in the comment below.

	Reducing the code further creates longer diffs and I couldn't
	tell if I was exposing the problem or not.

	The bug originally surfaced in a cross-compiled firmware
	environment for a custom ARM-based evaluation system developed
	by Cogent Computers.  It should be possible to replicate the
	problem with similar code in any ARM7TDMI-based board.

	I would be happy to try out patches on my eval box, if this
	problem is not convenient to test and/or debug.

>Fix:
	Don't use the optimizer or flip the lines around. :)


/*
   Compile with "arm-unknown-elf-gcc -Wall -I- -O -S".


Reading specs from /usr/local/lib/gcc-lib/arm-unknown-elf/3.1/specs
Configured with: ../../egcs/configure --with-newlib --with-gnu-ld --with-gnu-as --target=arm-unknown-elf --enable-languages=c
Thread model: single
gcc version 3.1 20010818 (experimental)


--- xbad.s	Sat Aug 18 23:13:17 2001
+++ xgood.s	Sat Aug 18 23:13:07 2001
@@ -17,19 +17,17 @@
 	cmp	r3, #300
 	cmpge	r2, #2
 	bne	.L3
+	ldrb	r3, [ip, #1]	@ zero_extendqisi2
+	cmp	r3, #1
+	bne	.L3
 	ldrb	r2, [ip, #7]	@ zero_extendqisi2
 	ldrb	r3, [lr, #8]	@ zero_extendqisi2
 	cmp	r2, r3
 	bne	.L3
-	ldr	r2, [r0, #42]
-	bic	r2, r2, #-16777216
-	bic	r2, r2, #255
-	mov	r3, #393216
-	add	r3, r3, #256
-	cmp	r2, r3
+	ldrb	r2, [ip, #2]	@ zero_extendqisi2
+	cmp	r2, #6
 	bne	.L3
 	add	r0, r0, #70
-	mov	r2, #6
 	bl	memcmp
 	cmp	r0, #0
 	moveq	r0, #1
 */

typedef unsigned char uByte;
#define BYTE_SIZE		8				/* size of a Byte */
#define BYTE_MASK		0xFF			/* a Byte with all bits turned on */

typedef unsigned int uInt;

#define IP_ADDR_SIZE		4
#define MAC_ADDR_SIZE		6
#define MAC_BUFFER_SIZE		1514
#define MAC_HEADER_SIZE		14
#define MAC_HEADER_OFFSET	0
#define IP_HEADER_SIZE		20
#define IP_HEADER_OFFSET	(MAC_HEADER_OFFSET + MAC_HEADER_SIZE)
#define UDP_HEADER_SIZE		8
#define UDP_HEADER_OFFSET	(IP_HEADER_OFFSET + IP_HEADER_SIZE)
#define UDP_PACKET_OFFSET	(UDP_HEADER_OFFSET + UDP_HEADER_SIZE)

#define BOOT_REQUEST	1
#define BOOT_REPLY		2

#define FILE_NAME_SIZE		128

struct bootp_packet
{
	uByte op;				/* either BOOT_REQUEST or BOOT_REPLY */
	uByte htype;			/* hardware type; 1 = 10Mb Ethernet */
	uByte hlen;				/* hardware addr length; 6 = 10Mb Ethernet*/
	uByte hops;				/* client set to zero */
	uByte xid[4];			/* random transaction ID number */
	uByte secs[2];			/* seconds elapsed since attempting to boot */
	uByte flags[2];			/* option flags - BROADCAST */
	uByte ciaddr[IP_ADDR_SIZE];		/* client IP addr, if known */
	uByte yiaddr[IP_ADDR_SIZE];		/* "your" IP addr, returned by server */
	uByte siaddr[IP_ADDR_SIZE];		/* server's IP addr, returned by server */
	uByte giaddr[IP_ADDR_SIZE];		/* gateway's IP addr; cross-gateway boot */
	uByte chaddr[16];			/* client hardware address, ie MAC addr */
	uByte sname[64];			/* optional server host name */
	char file[FILE_NAME_SIZE];	/* null-term'd boot file, returned by server */
	uByte options[64];			/* optional data - may be up to max pkt size */
};
#define BOOTP_PKT_SIZE		300


struct self
{
	uByte mac_addr[MAC_ADDR_SIZE];			/* our ether addr */
	uInt xid;								/* unique transaction ID */
};
typedef struct self Self;


int
get_bootp_reply(void *e, Self *s, uByte *packet, int len)
{
	struct bootp_packet *hdr =
			(struct bootp_packet*)(packet + UDP_PACKET_OFFSET);

	/* make sure this boot reply is intended for us */
	if (hdr->op != BOOT_REPLY ||
			len < BOOTP_PKT_SIZE ||
		/* XXX - swap the following two lines, and it works - XXX */
			hdr->xid[3] != (s->xid & BYTE_MASK) ||
			hdr->htype != 1 ||
		/* XXX - swap the previous two lines, and it works - XXX */
			hdr->hlen != MAC_ADDR_SIZE ||
			memcmp(hdr->chaddr, s->mac_addr, MAC_ADDR_SIZE) != 0)
	{
		return 0;
	}
	
	return 1;
}
>Release-Note:
>Audit-Trail:
>Unformatted:

Re: optimization/4061: Optimization bug in arm-unknown-elf cross-build of gcc 3.1

[rearnsha]

Synopsis: Optimization bug in arm-unknown-elf cross-build of gcc 3.1

Responsible-Changed-From-To: unassigned->rearnsha
Responsible-Changed-By: rearnsha
Responsible-Changed-When: Mon Aug 20 07:44:20 2001
Responsible-Changed-Why:
    mine
State-Changed-From-To: open->closed
State-Changed-By: rearnsha
State-Changed-When: Mon Aug 20 07:44:20 2001
State-Changed-Why:
    Not a bug.  You are assuming that because your structure contains
    only char type elements that it will be only byte-aligned.
    This assumption is incorrect on the ARM, where structures are
    always word-aligned by default.
    
    You can fix the problem by adding __attribute__((packed)) at the end
    of your structure declaration.  See the manual for more details.

http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view&pr=4061&database=gcc