Re: [V2 PATCH 3/3] powerpc: Fix Unaligned LE Floating Point Loads and Stores
From: Paul Mackerras <hidden>
Date: 2013-12-11 03:54:40
On Thu, Oct 31, 2013 at 01:38:58PM -0500, Tom wrote:
quoted hunk ↗ jump to hunk
From: Tom Musta <redacted> This patch addresses unaligned single precision floating point loads and stores in the single-step code. The old implementation improperly treated an 8 byte structure as an array of two 4 byte words, which is a classic little endian bug. Signed-off-by: Tom Musta <redacted> --- arch/powerpc/lib/sstep.c | 52 +++++++++++++++++++++++++++++++++++---------- 1 files changed, 40 insertions(+), 12 deletions(-)diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c index c8743e1..1cfd150 100644 --- a/arch/powerpc/lib/sstep.c +++ b/arch/powerpc/lib/sstep.c@@ -355,22 +355,36 @@ static int __kprobes do_fp_load(int rn, int (*func)(int, unsigned long), struct pt_regs *regs) { int err; - unsigned long val[sizeof(double) / sizeof(long)]; + union { + double dbl; + unsigned long ul[2]; + struct { +#ifdef __BIG_ENDIAN__ + unsigned _pad_; + unsigned word; +#endif +#ifdef __LITTLE_ENDIAN__ + unsigned word; + unsigned _pad_; +#endif + } single; + } data; unsigned long ptr; if (!address_ok(regs, ea, nb)) return -EFAULT; if ((ea & 3) == 0) return (*func)(rn, ea); - ptr = (unsigned long) &val[0]; + ptr = (unsigned long) &data.ul; if (sizeof(unsigned long) == 8 || nb == 4) { - err = read_mem_unaligned(&val[0], ea, nb, regs); - ptr += sizeof(unsigned long) - nb; + err = read_mem_unaligned(&data.ul[0], ea, nb, regs); + if (nb == 4) + ptr = (unsigned long)&(data.single.word); } else { /* reading a double on 32-bit */ - err = read_mem_unaligned(&val[0], ea, 4, regs); + err = read_mem_unaligned(&data.ul[0], ea, 4, regs); if (!err) - err = read_mem_unaligned(&val[1], ea + 4, 4, regs); + err = read_mem_unaligned(&data.ul[1], ea + 4, 4, regs);
This breaks 32-bit big-endian (as well as making the code longer and more complex). In fact, to make this work for 64-bit little-endian, all you really needed to do was ifdef out the statement: ptr += sizeof(unsigned long) - nb; Paul.