From: Julian Brown <julian@codesourcery.com>
To: Ian Lance Taylor <iant@google.com>
Cc: binutils@sourceware.org, GCC Patches <gcc-patches@gcc.gnu.org>,
DJ Delorie <dj@redhat.com>
Subject: Re: [PATCH, libiberty] Fix segfault in floatformat.c:get_field on 64-bit hosts
Date: Tue, 13 Jun 2006 17:23:00 -0000 [thread overview]
Message-ID: <448EEBB4.3090503@codesourcery.com> (raw)
In-Reply-To: <m3slm9c9j4.fsf@dhcp-172-24-103-66.corp.google.com>
Ian Lance Taylor wrote:
> Julian Brown <julian@codesourcery.com> 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
>>which reads:
>>
>> 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
>>immediates).
>
>
> 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
anyway :-)
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.
Thanks,
Julian
next prev parent reply other threads:[~2006-06-13 16:45 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-06-09 16:58 Julian Brown
2006-06-13 16:45 ` Ian Lance Taylor
2006-06-13 17:23 ` Julian Brown [this message]
2006-06-14 2:44 ` Ian Lance Taylor
2006-06-15 22:46 ` Julian Brown
2006-07-21 5:36 ` Mark Mitchell
2006-07-21 16:33 ` Ian Lance Taylor
2006-11-07 15:21 ` Julian Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=448EEBB4.3090503@codesourcery.com \
--to=julian@codesourcery.com \
--cc=binutils@sourceware.org \
--cc=dj@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=iant@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).