| Message ID | AANLkTinAEXYRQ_zk3u1dkb7U7DB20XJVxi0hmxKzBfn2@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Mar 10, 2011 at 1:19 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > Hello! > > Using binutils-2.21, a couple of > gcc.c-torture/execute/builtins/__-chk.c testcases fail on > alphaev68-pc-linux-gnu (-lto) with: > > /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: > Warning: alignment 8 of symbol `buf5' in > /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in > /tmp/ccc3QsSw.o.ironly > /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: > Warning: alignment 8 of symbol `buf7' in > /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in > /tmp/ccc3QsSw.o.ironly > /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: > Warning: alignment 8 of symbol `buf1' in > /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in > /tmp/ccc3QsSw.o.ironly > > Attached patch fixes these failures. I think this needs more investigation as there are no conflicting definitions of those vars that would warrant this kind of diagnostic from ld. We probably bring the vars local by making them hidden and distribute them to multiple ltrans units (with only one definiton obviously and multiple externs). Richard. > 2011-03-10 Uros Bizjak <ubizjak@gmail.com> > > PR testsuite/48055 > * gcc.c-torture/execute/builtins/memcpy-chk.c (buf1, buf5, buf7): > Declare as static. > * gcc.c-torture/execute/builtins/mempcpy-chk.c (buf1, buf5, buf7): > Ditto. > * gcc.c-torture/execute/builtins/memmove-chk.c (buf1, buf5, buf7): > Ditto. > > Tested on alphaev68-pc-linux-gnu with binutils-2.21. OK for mainline > and release branches? > > Uros >
On Thu, Mar 10, 2011 at 1:30 PM, Richard Guenther <richard.guenther@gmail.com> wrote: >> Using binutils-2.21, a couple of >> gcc.c-torture/execute/builtins/__-chk.c testcases fail on >> alphaev68-pc-linux-gnu (-lto) with: >> >> /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: >> Warning: alignment 8 of symbol `buf5' in >> /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in >> /tmp/ccc3QsSw.o.ironly >> /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: >> Warning: alignment 8 of symbol `buf7' in >> /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in >> /tmp/ccc3QsSw.o.ironly >> /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: >> Warning: alignment 8 of symbol `buf1' in >> /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in >> /tmp/ccc3QsSw.o.ironly >> >> Attached patch fixes these failures. > > I think this needs more investigation as there are no conflicting > definitions of those vars that would warrant this kind of diagnostic from ld. > > We probably bring the vars local by making them hidden and distribute > them to multiple ltrans units (with only one definiton obviously and multiple > externs). Jan claims that this problem is a linker bug, so perhaps the patch is still suitable for the gcc testsuite. The patch doesn't alter tests in any way, while it avoids false positives due to linker bugs. The linker problem was filled as binutils PR 12564 [1] against ld 2.21. [1] http://sourceware.org/bugzilla/show_bug.cgi?id=12564 Uros.
On Thu, Mar 10, 2011 at 3:44 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Thu, Mar 10, 2011 at 1:30 PM, Richard Guenther > <richard.guenther@gmail.com> wrote: > >>> Using binutils-2.21, a couple of >>> gcc.c-torture/execute/builtins/__-chk.c testcases fail on >>> alphaev68-pc-linux-gnu (-lto) with: >>> >>> /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: >>> Warning: alignment 8 of symbol `buf5' in >>> /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in >>> /tmp/ccc3QsSw.o.ironly >>> /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: >>> Warning: alignment 8 of symbol `buf7' in >>> /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in >>> /tmp/ccc3QsSw.o.ironly >>> /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: >>> Warning: alignment 8 of symbol `buf1' in >>> /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in >>> /tmp/ccc3QsSw.o.ironly >>> >>> Attached patch fixes these failures. >> >> I think this needs more investigation as there are no conflicting >> definitions of those vars that would warrant this kind of diagnostic from ld. >> >> We probably bring the vars local by making them hidden and distribute >> them to multiple ltrans units (with only one definiton obviously and multiple >> externs). > > Jan claims that this problem is a linker bug, so perhaps the patch is > still suitable for the gcc testsuite. The patch doesn't alter tests in > any way, while it avoids false positives due to linker bugs. Are you sure? Making the vars static enables folding the zero initialization. I don't think we should make testsuite changes to paper over bugs elsewhere. Richard. > The linker problem was filled as binutils PR 12564 [1] against ld 2.21. > > [1] http://sourceware.org/bugzilla/show_bug.cgi?id=12564 > > Uros. >
On Thu, Mar 10, 2011 at 3:59 PM, Richard Guenther <richard.guenther@gmail.com> wrote: >> <richard.guenther@gmail.com> wrote: >> >>>> Using binutils-2.21, a couple of >>>> gcc.c-torture/execute/builtins/__-chk.c testcases fail on >>>> alphaev68-pc-linux-gnu (-lto) with: >>>> >>>> /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: >>>> Warning: alignment 8 of symbol `buf5' in >>>> /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in >>>> /tmp/ccc3QsSw.o.ironly >>>> /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: >>>> Warning: alignment 8 of symbol `buf7' in >>>> /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in >>>> /tmp/ccc3QsSw.o.ironly >>>> /usr/lib/gcc/alpha-unknown-linux-gnu/4.4.5/../../../../alpha-unknown-linux-gnu/bin/ld: >>>> Warning: alignment 8 of symbol `buf1' in >>>> /tmp/ccgnDykf.ltrans1.ltrans.o is smaller than 16 in >>>> /tmp/ccc3QsSw.o.ironly >>>> >>>> Attached patch fixes these failures. >>> >>> I think this needs more investigation as there are no conflicting >>> definitions of those vars that would warrant this kind of diagnostic from ld. >>> >>> We probably bring the vars local by making them hidden and distribute >>> them to multiple ltrans units (with only one definiton obviously and multiple >>> externs). >> >> Jan claims that this problem is a linker bug, so perhaps the patch is >> still suitable for the gcc testsuite. The patch doesn't alter tests in >> any way, while it avoids false positives due to linker bugs. > > Are you sure? Making the vars static enables folding the zero initialization. I was looking at other similar testcases (snprintf-chk.c, vsprintf-chk.c), where uninitialized buffer is declared as static (and it didn't fail lto tests). Anyway, let's ask the author of the test (CC'd). > I don't think we should make testsuite changes to paper over bugs > elsewhere. How far do we want to reach in case the test uncovers the problem in (sort of...) unrelated product? Uros.
On Thu, Mar 10, 2011 at 04:48:48PM +0100, Uros Bizjak wrote: > > Are you sure? Making the vars static enables folding the zero initialization. > > I was looking at other similar testcases (snprintf-chk.c, > vsprintf-chk.c), where uninitialized buffer is declared as static (and > it didn't fail lto tests). Anyway, let's ask the author of the test > (CC'd). The tests weren't written with LTO in mind, after all LTO wasn't supported by GCC at that point. But I agree with Richard, we shouldn't working around buggy ld in the gcc testsuite, it is good to know that you have a buggy linker... Jakub
On Thu, Mar 10, 2011 at 4:57 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Mar 10, 2011 at 04:48:48PM +0100, Uros Bizjak wrote: >> > Are you sure? Making the vars static enables folding the zero initialization. >> >> I was looking at other similar testcases (snprintf-chk.c, >> vsprintf-chk.c), where uninitialized buffer is declared as static (and >> it didn't fail lto tests). Anyway, let's ask the author of the test >> (CC'd). > > The tests weren't written with LTO in mind, after all LTO wasn't supported > by GCC at that point. But I agree with Richard, we shouldn't working around > buggy ld in the gcc testsuite, it is good to know that you have a buggy > linker... AFAICS, the linker is not buggy, resulting executables still work OK. But I agree, and won't push this minor issue any further. Thanks, Uros.
Index: gcc.c-torture/execute/builtins/memcpy-chk.c =================================================================== --- gcc.c-torture/execute/builtins/memcpy-chk.c (revision 170823) +++ gcc.c-torture/execute/builtins/memcpy-chk.c (working copy) @@ -78,10 +78,10 @@ abort (); } -long buf1[64]; +static long buf1[64]; char *buf2 = (char *) (buf1 + 32); -long buf5[20]; -char buf7[20]; +static long buf5[20]; +static char buf7[20]; void __attribute__((noinline)) Index: gcc.c-torture/execute/builtins/memmove-chk.c =================================================================== --- gcc.c-torture/execute/builtins/memmove-chk.c (revision 170823) +++ gcc.c-torture/execute/builtins/memmove-chk.c (working copy) @@ -81,10 +81,10 @@ abort (); } -long buf1[64]; +static long buf1[64]; char *buf2 = (char *) (buf1 + 32); -long buf5[20]; -char buf7[20]; +static long buf5[20]; +static char buf7[20]; void __attribute__((noinline)) Index: gcc.c-torture/execute/builtins/mempcpy-chk.c =================================================================== --- gcc.c-torture/execute/builtins/mempcpy-chk.c (revision 170823) +++ gcc.c-torture/execute/builtins/mempcpy-chk.c (working copy) @@ -84,10 +84,10 @@ mempcpy_disallowed = 0; } -long buf1[64]; +static long buf1[64]; char *buf2 = (char *) (buf1 + 32); -long buf5[20]; -char buf7[20]; +static long buf5[20]; +static char buf7[20]; void __attribute__((noinline))