Bug 7006 - Bad left shift in _FP_FRAC_SRS_2 macro in soft-fp/op-2.h on SH-4
Summary: Bad left shift in _FP_FRAC_SRS_2 macro in soft-fp/op-2.h on SH-4
Alias: None
Product: glibc
Classification: Unclassified
Component: soft-fp (show other bugs)
Version: 2.8
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
Depends on:
Reported: 2008-11-05 16:10 UTC by Ilyes Gouta
Modified: 2014-07-01 21:20 UTC (History)
4 users (show)

See Also:
Last reconfirmed:
fweimer: security-


Note You need to log in before you can comment on or make changes to this bug.
Description Ilyes Gouta 2008-11-05 16:10:44 UTC
Hi all,

I hit this bug two years ago while porting a VM on SH-4. The bug is located in
soft-fp/op-2.h, in the macro _FP_FRAC_SRS_2. The bug is in the line 95:

X##_f0 = (X##_f1 << (_FP_W_TYPE_SIZE - (N)) | X##_f0 >> (N) | \

I had to change it to:

X##_f0 = ((X##_f1 << (_FP_W_TYPE_SIZE - (N) - 1)) << 1 | X##_f0 >> (N) | \

Because in the extreme case where you have N == 0 and _FP_W_TYPE_SIZE == 32,
X##_f1 << (_FP_W_TYPE_SIZE - (N)) would return 0, however on SH-4, it returns
X##_f1 since only 5 bits are used in the shift instruction, SHLD, to encode the
shift amount, and 32 (which spans 6 bits) will be masked to 0. This issue can
also be attributed to the SH-4 GCC compiler.

Ilyes Gouta.
Comment 1 Joseph Myers 2012-03-06 21:40:43 UTC
Confirmed still present in current sources.  The proposed fix seems reasonable.
Comment 2 Richard Henderson 2012-03-06 22:51:34 UTC
In addition, the "sticky bit term" is

  (__builtin_constant_p(N) && (N) == 1                \
   ? X##_f0 & 1                                       \
   : (X##_f0 << (_FP_W_TYPE_SIZE - (N))) != 0));

which also must be zero for a zero shift.

The same pattern occurs in the _FP_FRAC_SRST_* macros.

All that said, when do we *ever* call the SRS macros
with a zero shift count?  Perhaps my poor grepping 
skills, but all I see is _FP_WFRACBITS_##fs and 
2*wfracbits, where wfracbits is _FP_WFRACBITS...
Comment 3 Joseph Myers 2013-06-17 22:42:51 UTC
Indeed, I don't actually see how the _SRS macros can be called with a shift count of 0.  In the interest of efficiency, I'm inclined to say we should define the interfaces to all the shift macros to require a shift count that is strictly positive and strictly less than the total number of bits in the given number of words, then review all shift counts for cases that might violate this; I expect checking for 0 at any rare call sites where it might occur will be more efficient than making these macros handle the case of 0.

In one such review attempt, I can see the use of *left* shift for converting integers to floating point may shift by 0.  The following testcase illustrates that on x86_64 for conversion from unsigned __int128 to __float128 (soft-fp as used in libgcc):

#include <stdlib.h>

# define VOL /* Empty.  */
# define VOL volatile

main (void)
  VOL unsigned long long a = 0x1000000000000ULL;
  VOL unsigned long long b = 0xffffffffffffffffULL;
  unsigned __int128 c = (((unsigned __int128) a) << 64) | b;
  __float128 d = c;
  if (d != 0x1.000000000000ffffffffffffffffp112q)
    abort ();
  exit (0);

If you define COMPILE_TIME and build with optimization, the test passes as the conversion is done at compile time; otherwise, it is done at runtime and a right-shift by 64 in _FP_FRAC_SLL_2 called to shift left by 0 results in an incorrect result.
Comment 4 Joseph Myers 2013-06-21 19:02:44 UTC
What I believe was the only remaining case of a zero shift count being passed to one of the shift macros is fixed for 2.18 by:

commit 8fdda7afb883589b46e423f91f32988373670bfa
Author: Joseph Myers <joseph@codesourcery.com>
Date:   Fri Jun 21 19:00:43 2013 +0000

    Fix bad shift in soft-fp (bug 7006).

If you find other cases of zero shift counts being passed to these macros, please file new bugs for them.