Summary
arithmetic-shift fails in extreme cases
Metadata
- Id: 29e3bcd893e62c46101cbd7de369585ac4dc5eb4
- Trac id: 823
- Type: defect
- Reporter: sjamaan
- Owner:
- Cc:
- Status: closed
- Component: core libraries
- Estimated difficulty:
- Resolution: fixed
- Priority: minor
- Milestone: 4.10.0
- Version: 4.7.x
- Changetime: 2015-08-04 10:38:45 UTC
- Created: 2012-05-04 14:58:01 UTC
- Keywords: edge cases, numbers, shifting, overflow
Description
I don't know whether it's worth the trouble to fix this, but I'll put it on here so at least it's documented.
This should be {{{-1}}}. It fails because in chicken.h the {{{C_i_fixnum_arithmetic_shift}}} function calls {{{C_fixnum_shift_right}}} with the negated value of the shift amount. This overflows and wraps around to zero.
Fixing it would be simplest by just inlining the shift and negating _after_ unboxing. This is easy, but there are some other places that call {{{C_fixnum_shift_right}}} in this way.
This code is obviously wrong but I'm not sure it's worth fixing, since it's such an extreme edge case (if you subtract 1 from the shift amount you get an error that the argument is not a fixnum). On the other hand, not fixing it just seems, I dunno... lazy and possibly dangerous. :)
Changes and comments
[2013-10-15 20:52:36 UTC] sjamaan changed summary
[2013-10-15 20:52:36 UTC] sjamaan wrote:
Turns out there's also a bug when shifting more than C_WORD_SIZE positions.
According to the C spec, shifting more than the number of bits in the type is undefined (#3 in section 6.5.7, "bitwise shift operators"). Shifting negative values is also pretty ill-defined (#4 and #5, final sentence in both).
C: Don't you just love it?
[2014-02-17 08:47:13 UTC] sjamaan changed milestone from 4.9.0 to 4.10.0
[2014-02-17 08:47:13 UTC] sjamaan wrote:
Not important enough to delay 4.9.0 any longer
[2015-08-04 10:38:45 UTC] sjamaan changed status from new to closed
[2015-08-04 10:38:45 UTC] sjamaan set resolution to fixed
[2015-08-04 10:38:45 UTC] sjamaan wrote:
This has been fixed by the numbers integration in CHICKEN 5. It's not important enough to fix in CHICKEN 4.