Configure spec: Configured with: /p3/jim/GCC/gcc-3.4.0/configure --prefix=/usr/local/EQLGCC_v340/mips64el --enable-languages=c,c++ --target=mips64el-elf --with-newlib --with-arch=sb1 --with-tune=sb1 --with-gnu-as --with-gnu-ld Thread model: single The following trivial test program is miscompiled in a way that causes a crash: struct foo { short x; struct foo *p __attribute__((packed)); short z; }; struct foo *list; int walklist(struct foo *p) { int t = 0; while (p) { t += p->x; p = p->p; } return t; } The generated code looks like this: .file 1 "test.c" .section .mdebug.abiO64 .previous .text .align 2 .globl walklist .ent walklist walklist: .frame $sp,0,$31 # vars= 0, regs= 0/0, args= 0, gp= 0 .mask 0x00000000,0 .fmask 0x00000000,0 .set noreorder .set nomacro beq $4,$0,$L6 move $3,$0 $L4: lh $2,0($4) lwl $4,5($4) lwr $4,2($4) bne $4,$0,$L4 addu $3,$3,$2 $L6: j $31 move $2,$3 .set macro .set reorder .end walklist .comm list,4,4 The error is that an unaligned load is a two-instruction sequence, so you CANNOT use the same register as source and destination. The code as shown will crash when it comes to the null pointer at the end of the list. The above is with -O1; with -O2 the code is reordered slightly but the code is wrong in the same way.
The processor selection doesn't actually matter; I tried the compile with march/mtune set to sr71000, r5000, rm9000, and r4000; all generate the same broken code.
GCC 3.4.1 in the 20040618 snapshot has the same bug.
Mea culpa. Working on a fix.
Created attachment 6624 [details] Proposed patch Please try the attached patch. I'll test it tomorrow (once the regression tests for 16144 are complete).
Subject: Bug 16176 CVSROOT: /cvs/gcc Module name: gcc Changes by: rsandifo@gcc.gnu.org 2004-06-25 18:24:52 Modified files: gcc : ChangeLog gcc/config/mips: mips.c gcc/testsuite : ChangeLog Added files: gcc/testsuite/gcc.c-torture/execute: 20040625-1.c Log message: PR target/16176 * config/mips/mips.c (mips_expand_unaligned_load): Use a temporary register for the destination of the lwl or ldl. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.4145&r2=2.4146 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/mips/mips.c.diff?cvsroot=gcc&r1=1.421&r2=1.422 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.3898&r2=1.3899 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/20040625-1.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
Sorry for the delayed response. This patch appears to fix the problem.
Patch applied: http://gcc.gnu.org/ml/gcc-patches/2004-06/msg02085.html Mark, you know what I'm going to ask ;) Is this OK for 3.4.1? Or should it wait until 3.4.2? Perhaps the patch isn't quite as obvious as the one you approved earlier today. But IMO it _is_ still obvious. And safe. The bug here is also much more serious, since it only shows up at run time (rather than link time). I suppose every patch carries some amount of risk. But like I say, I think the risk in this case is very small, and I think it's obvious that the new code is theoretically more correct. From that POV, I'd imagine the risk is more of introducing an ICE than introducing another wrong code bug. Err... couldn't decide whether to delete that last paragraph or not ;)
Subject: Re: [3.4 Regression] Miscompilation of unaligned data in MIPS backend (SB1 flavor) rsandifo at gcc dot gnu dot org wrote: > ------- Additional Comments From rsandifo at gcc dot gnu dot org 2004-06-25 18:42 ------- > Patch applied: > > http://gcc.gnu.org/ml/gcc-patches/2004-06/msg02085.html > > Mark, you know what I'm going to ask ;) Is this OK for 3.4.1? Or should > it wait until 3.4.2? OK for 3.4.1.
Subject: Bug 16176 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: rsandifo@gcc.gnu.org 2004-06-28 13:58:45 Modified files: gcc : ChangeLog gcc/config/mips: mips.c gcc/testsuite : ChangeLog Added files: gcc/testsuite/gcc.c-torture/execute: 20040625-1.c Log message: PR target/16176 * config/mips/mips.c (mips_expand_unaligned_load): Use a temporary register for the destination of the lwl or ldl. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.524&r2=2.2326.2.525 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/mips/mips.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.362.4.12&r2=1.362.4.13 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3389.2.217&r2=1.3389.2.218 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/20040625-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=NONE&r2=1.1.2.1
Patch applied to 3.4.