From owner-svn-src-all@freebsd.org Sat Jul 18 06:26:22 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 77E929A4CB1; Sat, 18 Jul 2015 06:26:22 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id 3D2831BE6; Sat, 18 Jul 2015 06:26:21 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id E52F610457BD; Sat, 18 Jul 2015 16:26:18 +1000 (AEST) Date: Sat, 18 Jul 2015 16:26:17 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Pedro Giffuni cc: Peter Jeremy , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r285644 - head/contrib/sqlite3 In-Reply-To: <55A98763.1030705@FreeBSD.org> Message-ID: <20150718155507.J876@besplex.bde.org> References: <201507162207.t6GM7ECT009955@repo.freebsd.org> <20150717222631.GD36150@server.rulingia.com> <55A98763.1030705@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=eZjABOwH c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=kj9zAlcOel0A:10 a=6I5d2MoRAAAA:8 a=5sSLQ3LKH8a_lCe5FSsA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 18 Jul 2015 06:26:22 -0000 On Fri, 17 Jul 2015, Pedro Giffuni wrote: > On 07/17/15 17:26, Peter Jeremy wrote: >> On 2015-Jul-16 22:07:14 +0000, "Pedro F. Giffuni" wrote: >>> Log: >> ... >>> sqlite: clean a couple of invocations of memcpy(3) >>> Found almost accidentally by our native gcc when enhanced with >>> FORTIFY_SOURCE. >> ... >>> - memcpy((void *)&aHdr[1], (void *)&pWal->hdr, sizeof(WalIndexHdr)); >>> + memcpy((void *)&aHdr[1], (const void *)&pWal->hdr, >>> sizeof(WalIndexHdr)); >>> walShmBarrier(pWal); >>> - memcpy((void *)&aHdr[0], (void *)&pWal->hdr, sizeof(WalIndexHdr)); >>> + memcpy((void *)&aHdr[0], (const void *)&pWal->hdr, >>> sizeof(WalIndexHdr)); >> If the compiler complained about that, the compiler is broken. > > The change was rather collateral (read cosmetical) to the real warning > which remains unfixed. > > The compiler warning is this: > ... > ===> lib/libsqlite3 (obj,depend,all,install) > cc1: warnings being treated as errors > /scratch/tmp/pfg/head/lib/libsqlite3/../../contrib/sqlite3/sqlite3.c: In > function 'walIndexWriteHdr': > /scratch/tmp/pfg/head/lib/libsqlite3/../../contrib/sqlite3/sqlite3.c:49490: > warning: passing argument 1 of 'memcpy' discards qualifiers from pointer > target type > /scratch/tmp/pfg/head/lib/libsqlite3/../../contrib/sqlite3/sqlite3.c:49492: > warning: passing argument 1 of 'memcpy' discards qualifiers from pointer > target type > --- sqlite3.So --- > *** [sqlite3.So] Error code 1 > ... > > make[6]: stopped in /scratch/tmp/pfg/head/lib/libsqlite3 > > It only happens when using the experimental FORTIFY_SOURCE support [1], > and it only happens with gcc (the base one is the only tested). > > Yes, I am suspecting a compiler bug but gcc has been really useful to find > bugs in fortify_source. gcc is correct. This has nothing to do with 'const'. aHdr has a volatile qualifier. >> 'const' >> is a promise to the caller that the given parameter will not be modified >> by the callee. There's no requirement that the passed argument be const. >> As bde commented, the casts are all spurious. There is a requirement that the parameter doesn't point to volatile storage even if it has a volatile qualifier. memcpy() doesn't support volatile storage. This allows it to be more efficient by possibly doing the stores out of order or more than once. Someone broke the warnings about the prototype discarding the volatile qualifier by adding casts to discard the qualifier explicitly. This is the correct way to turn volatile storage into nonvolatile storage iff the storage is actually nonvolatile for the lifetime of the cast. It is unclear how the storage becomes transiently non-volatile here. But casting away qualifiers is usually an error, so we ask the compiler to warn about it using -Wcast-qual. -Wcast-qual is broken in clang, so bugs found by it show up more after switching to gcc. clang has ways to enable this warning, but the standard way doesn't work. Casting away volatile is more likely to be just a bug than casting away const, so it is quite reasonable for the compiler to warn about it unconditionally or at least under -Wall. Almost all of the casts of memcpy()'s args in sqlite3.c are for or near the casts of the volatile foo *aHdr. The old ones are all to void *. This kills all qualifiers, but there are only volatile qualifiers in sight. Someone also sprinkled them on the nearby second arg of &hdr. This seems to be not even wrong (there are no qualifiers in sight for hdr). Sometimes aHdr is the second arg of memcpy(). This cannot be volatile either, so volatile is cast away using void *. The complete set of conversions for this is: aHdr is volatile foo * for some reason the cast changes it to void * the prototype changes it to const void *. The best way to express the magic removal of volatile probably to cast to foo *, not to void *. Let the prototype do all of the non-magic conversions. Casting to void * does 1 magic conversion but only half of the non-magic conversions that would be done by the prototype (the other one is adding a const qualifier). Bruce