From owner-freebsd-arch@FreeBSD.ORG Thu Jan 16 17:43:36 2014 Return-Path: Delivered-To: arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 454DDB81; Thu, 16 Jan 2014 17:43:36 +0000 (UTC) Received: from h2.funkthat.com (gate2.funkthat.com [208.87.223.18]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 093F114A1; Thu, 16 Jan 2014 17:43:35 +0000 (UTC) Received: from h2.funkthat.com (localhost [127.0.0.1]) by h2.funkthat.com (8.14.3/8.14.3) with ESMTP id s0GHhY6h088351 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 16 Jan 2014 09:43:34 -0800 (PST) (envelope-from jmg@h2.funkthat.com) Received: (from jmg@localhost) by h2.funkthat.com (8.14.3/8.14.3/Submit) id s0GHhYSR088350; Thu, 16 Jan 2014 09:43:34 -0800 (PST) (envelope-from jmg) Date: Thu, 16 Jan 2014 09:43:34 -0800 From: John-Mark Gurney To: arch@FreeBSD.org Subject: CFR: removing off_t casts of td_retval Message-ID: <20140116174334.GC75135@funkthat.com> Mail-Followup-To: arch@FreeBSD.org, Bruce Evans Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4.2.3i X-Operating-System: FreeBSD 7.2-RELEASE i386 X-PGP-Fingerprint: 54BA 873B 6515 3F10 9E88 9322 9CB1 8F74 6D3F A396 X-Files: The truth is out there X-URL: http://resnet.uoregon.edu/~gurney_j/ X-Resume: http://resnet.uoregon.edu/~gurney_j/resume.html X-to-the-FBI-CIA-and-NSA: HI! HOW YA DOIN? can i haz chizburger? X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.2.2 (h2.funkthat.com [127.0.0.1]); Thu, 16 Jan 2014 09:43:35 -0800 (PST) Cc: Bruce Evans X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Jan 2014 17:43:36 -0000 In attempting to bring up an AVILA board, I found that td_retval has became unaligned to an 8 byte boundary on 32bit platforms. As we cast td_retval (of struct thread) to off_t, this results in an alignment fault. After consulting w/ bde, we decided on making a union of td_retval and a new off_t member. This will now align things properly on arches that require it (arm). Keep the 32bit alignment on arches that doesn't need it (verified on i386), and should keep things unchanged on 64bit arches (since there register_t, type of td_retval provided 64bit alignment). This also has the added benefit of making our code more correct as the code will be alias safe as we are not casting pointers between types. There never was a problem with the code as in all cases I saw, as we would only ever read/write from/to these members in a function once. This also eliminates a bad example. I have tested this patch on amd64 and armeb (as far as I can due to other bugs). tinderbox builds are running, and assuming they complete cleanly, I'll commit. I may have missed some locations as I only did a: grep td_retval | grep off_t Comments? Index: compat/freebsd32/freebsd32_misc.c =================================================================== --- compat/freebsd32/freebsd32_misc.c (revision 260499) +++ compat/freebsd32/freebsd32_misc.c (working copy) @@ -1504,7 +1504,7 @@ ap.whence = uap->whence; error = sys_lseek(td, &ap); /* Expand the quad return into two parts for eax and edx */ - pos = *(off_t *)(td->td_retval); + pos = td->td_uretoff.tdu_off; td->td_retval[RETVAL_LO] = pos & 0xffffffff; /* %eax */ td->td_retval[RETVAL_HI] = pos >> 32; /* %edx */ return error; Index: kern/uipc_shm.c =================================================================== --- kern/uipc_shm.c (revision 260499) +++ kern/uipc_shm.c (working copy) @@ -270,7 +270,7 @@ if (offset < 0 || offset > shmfd->shm_size) error = EINVAL; else - *(off_t *)(td->td_retval) = offset; + td->td_uretoff.tdu_off = offset; } foffset_unlock(fp, offset, error != 0 ? FOF_NOUPDATE : 0); return (error); Index: kern/vfs_vnops.c =================================================================== --- kern/vfs_vnops.c (revision 260499) +++ kern/vfs_vnops.c (working copy) @@ -2080,7 +2080,7 @@ if (error != 0) goto drop; VFS_KNOTE_UNLOCKED(vp, 0); - *(off_t *)(td->td_retval) = offset; + td->td_uretoff.tdu_off = offset; drop: foffset_unlock(fp, offset, error != 0 ? FOF_NOUPDATE : 0); return (error); Index: sys/proc.h =================================================================== --- sys/proc.h (revision 260499) +++ sys/proc.h (working copy) @@ -300,7 +300,11 @@ TDS_RUNQ, TDS_RUNNING } td_state; /* (t) thread state */ - register_t td_retval[2]; /* (k) Syscall aux returns. */ + union { + register_t tdu_retval[2]; + off_t tdu_off; + } td_uretoff; /* (k) Syscall aux returns. */ +#define td_retval td_uretoff.tdu_retval struct callout td_slpcallout; /* (h) Callout for sleep. */ struct trapframe *td_frame; /* (k) */ struct vm_object *td_kstack_obj;/* (a) Kstack object. */ -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."