From owner-svn-src-all@FreeBSD.ORG Sat Jul 11 22:43:20 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8F275106566B; Sat, 11 Jul 2009 22:43:20 +0000 (UTC) (envelope-from marcel@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 62A1D8FC13; Sat, 11 Jul 2009 22:43:20 +0000 (UTC) (envelope-from marcel@FreeBSD.org) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id n6BMhKAJ065426; Sat, 11 Jul 2009 22:43:20 GMT (envelope-from marcel@svn.freebsd.org) Received: (from marcel@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id n6BMhK0H065424; Sat, 11 Jul 2009 22:43:20 GMT (envelope-from marcel@svn.freebsd.org) Message-Id: <200907112243.n6BMhK0H065424@svn.freebsd.org> From: Marcel Moolenaar Date: Sat, 11 Jul 2009 22:43:20 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r195627 - head/sys/cddl/contrib/opensolaris/common/nvpair X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 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, 11 Jul 2009 22:43:21 -0000 Author: marcel Date: Sat Jul 11 22:43:20 2009 New Revision: 195627 URL: http://svn.freebsd.org/changeset/base/195627 Log: In nvpair_native_embedded_array(), meaningless pointers are zeroed. The programmer was aware that alignment was not guaranteed in the packed structure and used bzero() to NULL out the pointers. However, on ia64, the compiler is quite agressive in finding ILP and calls to bzero() are often replaced by simple assignments (i.e. stores). Especially when the width or size in question corresponds with a store instruction (i.e. st1, st2, st4 or st8). The problem here is not a compiler bug. The address of the memory to zero-out was given by '&packed->nvl_priv' and given the type of the 'packed' pointer the compiler could assume proper alignment for the replacement of bzero() with an 8-byte wide store to be valid. The problem is with the programmer. The programmer knew that the address did not have the alignment guarantees needed for a regular assignment, but failed to inform the compiler of that fact. In fact, the programmer told the compiler the opposite: alignment is guaranteed. The fix is to avoid using a pointer of type "nvlist_t *" and instead use a "char *" pointer as the basis for calculating the address. This tells the compiler that only 1-byte alignment can be assumed and the compiler will either keep the bzero() call or instead replace it with a sequence of byte-wise stores. Both are valid. Approved by: re (kib) Modified: head/sys/cddl/contrib/opensolaris/common/nvpair/nvpair.c Modified: head/sys/cddl/contrib/opensolaris/common/nvpair/nvpair.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/common/nvpair/nvpair.c Sat Jul 11 22:30:37 2009 (r195626) +++ head/sys/cddl/contrib/opensolaris/common/nvpair/nvpair.c Sat Jul 11 22:43:20 2009 (r195627) @@ -2543,7 +2543,6 @@ nvpair_native_embedded_array(nvstream_t nvs_native_t *native = (nvs_native_t *)nvs->nvs_private; char *value = native->n_curr - nvp->nvp_size + NVP_VALOFF(nvp); size_t len = NVP_NELEM(nvp) * sizeof (uint64_t); - nvlist_t *packed = (nvlist_t *)((uintptr_t)value + len); int i; /* * Null out pointers that are meaningless in the packed @@ -2552,13 +2551,17 @@ nvpair_native_embedded_array(nvstream_t */ bzero(value, len); - for (i = 0; i < NVP_NELEM(nvp); i++, packed++) + value += len; + for (i = 0; i < NVP_NELEM(nvp); i++) { /* * Null out the pointer that is meaningless in the * packed structure. The address may not be aligned, * so we have to use bzero. */ - bzero(&packed->nvl_priv, sizeof (packed->nvl_priv)); + bzero(value + offsetof(nvlist_t, nvl_priv), + sizeof(((nvlist_t *)NULL)->nvl_priv)); + value += sizeof(nvlist_t); + } } return (nvs_embedded_nvl_array(nvs, nvp, NULL));