This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH, libiberty] Fix segfault in floatformat.c:get_field on 64-bit hosts
- From: Julian Brown <julian at codesourcery dot com>
- To: Ian Lance Taylor <iant at google dot com>
- Cc: binutils at sourceware dot org, GCC Patches <gcc-patches at gcc dot gnu dot org>, DJ Delorie <dj at redhat dot com>
- Date: Tue, 13 Jun 2006 17:45:40 +0100
- Subject: Re: [PATCH, libiberty] Fix segfault in floatformat.c:get_field on 64-bit hosts
- References: <4489A779.firstname.lastname@example.org> <email@example.com>
Ian Lance Taylor wrote:
Julian Brown <firstname.lastname@example.org> writes:
This patch fixes a problem with floatformat.c:get_field on 64-bit (on
at least x86_64), when cross-assembling to arm-none-eabi. The line
result = *(data + cur_byte) >> (-cur_bitshift);
was executed with cur_byte = -1 (start + len == 0 and order ==
floatformat_little), which happily segfaulted (during printing of FP
I don't understand how start + len == 0 could ever be true. What was
calling the function? I note that put_field has the exact same
problem if start + len == 0.
Sorry, I botched my explanation a bit. I should have said:
(start + len) / FLOATFORMAT_CHAR_BIT == 0
which is true when e.g. extracting the sign bit of a single-precision
IEEE float -- in that case, start will be 0 and len will be 1 (with
big-endian bit numbering used elsewhere in floatformat.c). The function
is called by opcodes/arm-dis.c to print out the ARM Neon
"quarter-precision" floating-point immediates.
! return result & ((2 << (total_len - 1)) - 1);
Why do you need to do this? And if you do need to do it, why use 2?
Why not ((1 << total_len) - 1)?
It was to attempt to maintain the original semantics of the function, as
I understood them: the result is truncated to total_len after being
built up <=8 bits at a time (though looking again, that might not have
been the original intention at all, or at least not at that level of
granularity... I suspect that bit should be removed).
The reason for using 2 << ... rather than 1 << .. was so that, e.g., the
total_len == 32 case works properly. AIUI, shift amounts must be
strictly less than the width of the operand, and the total_len==0 case
isn't interesting. But that's irrelevant if I'm getting rid of that bit
So, I'll remove that bit and do the following...
Please compile the file as a standalone program with -DIEEE_DEBUG to
make sure those tests still work. Ideally on both a big- and
little-endian system, if possible.