This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/2] soft-fp: Fix overwritten issue for division in op-4.h
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Zong Li <zongbox at gmail dot com>
- Cc: <palmer at dabbelt dot com>, <darius at bluespec dot com>, <andrew at sifive dot com>, <dj at redhat dot com>, <libc-alpha at sourceware dot org>, <jimw at sifive dot com>, <kito at andestech dot com>, <greentime at andestech dot com>
- Date: Mon, 15 Oct 2018 13:00:57 +0000
- Subject: Re: [PATCH 1/2] soft-fp: Fix overwritten issue for division in op-4.h
- References: <cover.1539595555.git.zongbox@gmail.com> <5e51c4ea8cfe99f2faac5e206982658bd6c5d265.1539595555.git.zongbox@gmail.com>
On Mon, 15 Oct 2018, Zong Li wrote:
> diff --git a/soft-fp/op-4.h b/soft-fp/op-4.h
> index 01b87d0..a407168 100644
> --- a/soft-fp/op-4.h
> +++ b/soft-fp/op-4.h
> @@ -517,10 +517,15 @@
> R##_f[_FP_DIV_MEAT_4_udiv_i] = -1; \
> if (!_FP_DIV_MEAT_4_udiv_i) \
> break; \
> - __FP_FRAC_SUB_4 (X##_f[3], X##_f[2], X##_f[1], X##_f[0], \
> + UWtype __FP_FRAC_tmp; \
> + __FP_FRAC_SUB_4 (X##_f[2], X##_f[1], X##_f[0], __FP_FRAC_tmp, \
> Y##_f[2], Y##_f[1], Y##_f[0], 0, \
> X##_f[2], X##_f[1], X##_f[0], \
> _FP_DIV_MEAT_4_udiv_n_f[_FP_DIV_MEAT_4_udiv_i]); \
> + X##_f[3] = X##_f[2]; \
> + X##_f[2] = X##_f[1]; \
> + X##_f[1] = X##_f[0]; \
> + X##_f[0] = __FP_FRAC_tmp; \
This approach, using the X array in a shifted form to store the result
with a single temporary, is not a clean solution.
First, we should work out what the interface *should* be to all the
FRAC_ADD and FRAC_SUB macros, regarding permitted overlap between the
output and the inputs. This requires examining all those macros and their
uses to determine what consistent or nearly consistent patterns there are
in that interface.
Given an appropriate choice of interface, all the definitions that do not
fit that choice, and all the uses that do not fit that choice, then need
to be fixed. If that choice says the overlap above it not OK, a whole
four-word temporary should be defined using _FP_FRAC_DECL_4 for the
result, and then copied with _FP_FRAC_COPY_4. The commit message for any
proposed change needs to describe the conclusions regarding the interface
for permitted overlaps, and the analysis done to determine what needs to
change.
I suspect that the intended interface is that *exact* overlap is permitted
between the output and the *second* input, but not other overlap. In
which case at least the definitions of __FP_FRAC_SUB_3 and __FP_FRAC_SUB_4
look buggy and need changing (they reference y1 and y2 after r1 and r2
have been set), as well as the user you're changing above (which should
use a full temporary).
--
Joseph S. Myers
joseph@codesourcery.com