Bug#772718: adns: test failure on Ubuntu ppc64el, with -O3
Ian Jackson wrote: > I don't intend (with my Debian hat on) to upload this > to Debian right now since > - Debian is frozen for jessie > - The actual library and binaries are fine > - AFAIAA the test suite failure does not affect Debian itself > > When Debian unfreezes, I will fix this with a new upstream version. Ping! Thanks, Jeremy Bicha
Bug#772718: adns: test failure on Ubuntu ppc64el, with -O3
On Wed, Dec 10, 2014 at 11:21:13PM +, Ian Jackson wrote: > The attached patch should fix this. It is in the upstream git and > will be in the next upstream adns release. Indeed I may bring that > release forward. > > I don't intend (with my Debian hat on) to upload this to Debian right > now since > - Debian is frozen for jessie > - The actual library and binaries are fine > - AFAIAA the test suite failure does not affect Debian itself > > When Debian unfreezes, I will fix this with a new upstream version. Thanks! I've cherry-picked this patch into Ubuntu for now. > BTW, building with -O3 by default seems a bit mad. But IMO it ought > to work, so this is a real bug. Well, yes, I agree and argued against it, but ... -- Colin Watson [cjwat...@ubuntu.com] -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#772718: adns: test failure on Ubuntu ppc64el, with -O3
Control: tags -1 + patch Colin Watson writes ("Bug#772718: adns: test failure on Ubuntu ppc64el, with -O3"): > adns 1.5.0~rc1-1 fails to build on Ubuntu ppc64el; 1.4-2ubuntu1 (which > was modified to use dh-autoreconf in order to build on newer > architectures; The attached patch should fix this. It is in the upstream git and will be in the next upstream adns release. Indeed I may bring that release forward. I don't intend (with my Debian hat on) to upload this to Debian right now since - Debian is frozen for jessie - The actual library and binaries are fine - AFAIAA the test suite failure does not affect Debian itself When Debian unfreezes, I will fix this with a new upstream version. BTW, building with -O3 by default seems a bit mad. But IMO it ought to work, so this is a real bug. Thanks, Ian. commit d8fa191ed7774818862febd6ade774cb7e149ab9 Author: Ian Jackson Date: Wed Dec 10 23:16:37 2014 + Fix for malicious optimisation of memcpy in test suite, which causes failure with gcc-4.1.9 -O3. See Debian bug #772718. diff --git a/changelog b/changelog index a3e7dbb..fedd31a 100644 --- a/changelog +++ b/changelog @@ -1,6 +1,8 @@ adns (1.5.1~~) unstable; urgency=low * Portability fix for systems where socklen_t is bigger than int. + * Fix for malicious optimisation of memcpy in test suite, which +causes failure with gcc-4.1.9 -O3. See Debian bug #772718. -- diff --git a/regress/hcommon.c b/regress/hcommon.c index ebbef94..e63a8cd 100644 --- a/regress/hcommon.c +++ b/regress/hcommon.c @@ -281,7 +281,7 @@ void *Hrealloc(void *op, size_t nsz) { size_t osz; if (op) { oldnode= (void*)((char*)op - MALLOCHSZ); osz= oldnode->sz; } else { osz= 0; } np= Hmalloc(nsz); - memcpy(np,op, osz>nsz ? nsz : osz); + if (osz) memcpy(np,op, osz>nsz ? nsz : osz); Hfree(op); return np; } diff --git a/regress/hcommon.c.m4 b/regress/hcommon.c.m4 index c5069ee..330da4d 100644 --- a/regress/hcommon.c.m4 +++ b/regress/hcommon.c.m4 @@ -301,7 +301,7 @@ void *Hrealloc(void *op, size_t nsz) { if (op) { oldnode= (void*)((char*)op - MALLOCHSZ); osz= oldnode->sz; } else { osz= 0; } np= Hmalloc(nsz); - memcpy(np,op, osz>nsz ? nsz : osz); + if (osz) memcpy(np,op, osz>nsz ? nsz : osz); Hfree(op); return np; } -- -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#772718: adns: test failure on Ubuntu ppc64el, with -O3
Colin Watson writes ("Bug#772718: adns: test failure on Ubuntu ppc64el, with -O3"): > To reproduce this it appears to be sufficient to build with -O3. Feel > free to use the "cjwatson-adns" session on pastel.debian.net to > reproduce this; "DEB_CFLAGS_APPEND=-O3 debian/rules build" in an > unpacked copy of the latest adns source package does the job. Thanks. I have repro'd this and traced the root cause. The problem is a malicious optimisation of the following function in the test suite: void *Hrealloc(void *op, size_t nsz) { struct malloced *oldnode; void *np; size_t osz; if (op) { oldnode= (void*)((char*)op - MALLOCHSZ); osz= oldnode->sz; } else { osz= 0; } np= Hmalloc(nsz); memcpy(np,op, osz>nsz ? nsz : osz); Hfree(op); return np; } The standards bodies have decided that (contrary to traditional practice) memcpy(0,0,0) is UB. Hence, GCC reasons as follows: If !op, we still call memcpy(np,op,0). Which is UB. Therefore !!op. Therefore I don't need to actually compile the !op branches. I suspect that this only shows up with -O3 because in that case a copy of Hfree is inlined into Hrealloc. (gcc evidently fails to remove the also wrongfully-provably-unnecessary osz=0 branch, since osz=oldnode->sz would crash.) With the debugging printf patch below, and building upstream adns 1.5.0 with gcc 4.9.1-19 on i386 and with XCFLAGS='-O3 -fno-builtin-cpp', I see this output: Hmalloc 8 -> 0x8299018 (node=0x8299008 next=(nil) back=(nil) count=1) Hmalloc 16 -> 0x8299038 (node=0x8299028 next=(nil) back=0x8299008 count=2) Hmalloc 1452 -> 0x8299060 (node=0x8299050 next=(nil) back=0x8299028 count=3) Hmalloc 8 -> 0x8299620 (node=0x8299610 next=(nil) back=0x8299050 count=4) Hmalloc 41 -> 0x8299640 (node=0x8299630 next=(nil) back=0x8299610 count=5) Hfree((nil)) Hmalloc 40 -> 0x8299b98 (node=0x8299b88 next=(nil) back=0x8299630 count=6) Hrealloc((nil), 40) -> 0x8299b98 Hfree((nil)) Hfree((nil)) (node=0xfff0) Segmentation fault (core dumped) Notice that in Hfree, which looks like this: void Hfree(void *ptr) { struct malloced *oldnode; fprintf(stderr,"Hfree(%p)\n",ptr); if (!ptr) return; oldnode= (void*)((char*)ptr - MALLOCHSZ); fprintf(stderr,"Hfree(%p) (node=%p)\n",ptr, oldnode); fprintf(stderr,"Hfree(%p) (node=%p next=%p back=%p count=%lu)\n",ptr, oldnode, oldnode->next, oldnode->back, The if (!ptr) check is removed. gdb shows this stack trace: #0 0x0804e129 in Hrealloc () at hcommon.c:278 #1 0x080595c9 in adns__vbuf_appendstr () #2 0x0804d1c6 in Qsocket () at hcommon.c:209 #3 0x0804b0c4 in Hsocket () #4 0x0805c21e in init_finish () at ../src/setup.c:670 #5 0x0805cbba in adns_init_strcfg () at ../src/setup.c:761 #6 0x0804909a in main () at ../client/adnstest.c:244 The fix is obvious and will follow in my next email. Ian. diff --git a/regress/hcommon.c b/regress/hcommon.c index ebbef94..d59104b 100644 --- a/regress/hcommon.c +++ b/regress/hcommon.c @@ -265,12 +265,20 @@ void *Hmalloc(size_t sz) { } assert(newnode->count != mallocfailat); memset(&newnode->data,0xc7,sz); +fprintf(stderr,"Hmalloc %lu -> %p (node=%p next=%p back=%p count=%lu)\n", + (unsigned long)sz,&newnode->data, + newnode, newnode->next, newnode->back, newnode->count); return &newnode->data; } void Hfree(void *ptr) { struct malloced *oldnode; +fprintf(stderr,"Hfree(%p)\n",ptr); if (!ptr) return; oldnode= (void*)((char*)ptr - MALLOCHSZ); +fprintf(stderr,"Hfree(%p) (node=%p)\n",ptr, oldnode); +fprintf(stderr,"Hfree(%p) (node=%p next=%p back=%p count=%lu)\n",ptr, + oldnode, + oldnode->next, oldnode->back, oldnode->count); LIST_UNLINK(mallocedlist,oldnode); memset(&oldnode->data,0x38,oldnode->sz); free(oldnode); @@ -281,6 +289,7 @@ void *Hrealloc(void *op, size_t nsz) { size_t osz; if (op) { oldnode= (void*)((char*)op - MALLOCHSZ); osz= oldnode->sz; } else { osz= 0; } np= Hmalloc(nsz); +fprintf(stderr,"Hrealloc(%p, %lu) -> %p\n", op, (unsigned long)nsz, np); memcpy(np,op, osz>nsz ? nsz : osz); Hfree(op); return np; diff --git a/regress/hcommon.c.m4 b/regress/hcommon.c.m4 index c5069ee..3290596 100644 --- a/regress/hcommon.c.m4 +++ b/regress/hcommon.c.m4 @@ -280,15 +280,28 @@ void *Hmalloc(size_t sz) { } assert(newnode->count != mallocfailat); memset(&newnode->data,0xc7,sz); + +fprintf(stderr,"Hmalloc %lu -> %p (node=%p next=%p back=%p count=%lu)\n", + (unsigned long)sz,&newnode->data, + newnode, newnode-&g
Bug#772718: adns: test failure on Ubuntu ppc64el, with -O3
Package: adns Version: 1.5.0~rc1-1 Severity: normal User: ubuntu-de...@lists.ubuntu.com Usertags: origin-ubuntu vivid adns 1.5.0~rc1-1 fails to build on Ubuntu ppc64el; 1.4-2ubuntu1 (which was modified to use dh-autoreconf in order to build on newer architectures; http://launchpadlibrarian.net/126377262/adns_1.4-2build2_1.4-2ubuntu1.diff.gz) built cleanly. Here's the build log: https://launchpadlibrarian.net/191985562/buildlog_ubuntu-vivid-ppc64el.adns_1.5.0~rc1-1_FAILEDTOBUILD.txt.gz To reproduce this it appears to be sufficient to build with -O3. Feel free to use the "cjwatson-adns" session on pastel.debian.net to reproduce this; "DEB_CFLAGS_APPEND=-O3 debian/rules build" in an unpacked copy of the latest adns source package does the job. Thanks, -- Colin Watson [cjwat...@ubuntu.com] -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org