Date: Thu, 16 Jan 2014 09:43:34 -0800 From: John-Mark Gurney <jmg@funkthat.com> To: arch@FreeBSD.org Cc: Bruce Evans <bde@FreeBSD.org> Subject: CFR: removing off_t casts of td_retval Message-ID: <20140116174334.GC75135@funkthat.com>
next in thread | raw e-mail | index | archive | help
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."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140116174334.GC75135>