JRuby (please use github issues at http://bugs.jruby.org)
  1. JRuby (please use github issues at http://bugs.jruby.org)
  2. JRUBY-6777

RubyFixnum.java - two methods fail to detect some long integer overflows

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: JRuby 1.7.0.pre1
    • Fix Version/s: JRuby 1.7.1
    • Component/s: Core Classes/Modules
    • Labels:
      None
    • Environment:
      Microsoft Windows Vista, but these bugs seem to be also in JRuby 1.6.6, and I suspect they may also be in earlier versions of JRuby.
    • Number of attachments :
      3

      Description

      This deals with two possible "critical" issues in RubyFixnum.java,
      and also suggests a possible improvement to related code which is working.
      (As with my submission in JRUBY-6612, my apologies if I'm raising something
      which you are already aware of, or if I'm looking at the wrong code,
      or making another error which invalidates this bug report.
      And I'd be happy to (try to!) answer any questions.)

      I think there are some (critical?) bugs in

      • private IRubyObject idivLong: RubyFixnum.java line 566;
      • private IRubyObject divmodFixnum: RubyFixnum.java line 635;
        The problems are very similar to those I raised in JRUBY-6612 for:
      • public IRubyObject op_mul: RubyFixnum.java line 491.

      Using jirb from JRUBY 1.6.6
      v = 256
      v *= v #=> 65536
      v *= v * v * (v / -2) #=> -9223372036854775808 == Long.MIN_VALUE
      v / -1 #=> -9223372036854775808
      v.div(-1) #=> -9223372036854775808
      v.divmod(-1) #=> [-9223372036854775808, 0]

      1. This next line is also a problem in JRuby 1.6.6, but following
      2. JRUBY-6612 has, I think, been fixed in JRuby 1.7.0.preview1.
        -1 * v #=> -9223372036854775808 # wrong
        v * -1 #=> 9223372036854775808 # correct

      Using jirb from JRUBY 1.7.0.preview1 gives the same results
      except that "-1 * v" now gives the correct 9223372036854775808.

      The problem seems to be that some long overflows aren't being detected.

      I suggest patches in the attached file which:
      (a) make what I think are appropriate checks for overflows;
      (b) change the code to avoid using both "/" and "%", to speed things up.

      I've put this as critical because when I raised a similar problem
      for "public IRubyObject op_mul" (see JRUBY-6612) I called it "major"
      and it was upgraded to "critical".

        Activity

        Hide
        Colin Bartlett added a comment -

        I've put a fuller reply on SkyDrive because I think it might be a bit long for a comment here.

        >> Colin: Checking on your progress, since it has been more than two or three days
        My apologies: I was too optimistic about the two or three days, and then kept on being optimistic it would only be another day or two: sorry.

        I finished the single text file summary this weekend, and checked it: that doesn't mean I haven't made any errors in it, but it does mean that I have made a big effort to avoid errors. (Admittedly not quite the same thing.)

        I've posted it here, along with a "please-read-first-jruby-iterator-issues.txt" file:
        https://skydrive.live.com/#cid=01F6D7100ADB0423
        (Let me know via Twitter @colinb8 if the link doesn't work or you can't get at that file.)

        I suggest downloading that summary, but it's longer than I intended - I was trying to be comprehensive about the problems. So you might want to just look at the "contents" section for now, and (for now) rely on what I say in the
        "please-read-first-jruby-iterator-issues.txt" file.

        I'll put my patches on that skydrive public folder: I'll clean them up and send you a Twitter message when I've done that.

        Feel free to email me directly at colinwb@yahoo.co.uk: it might be a good idea to tweet me at Twitter@colinb8 the first time (if you do that) just in case Yahoo is insensitive enough to put an email in the Spam folder.

        Show
        Colin Bartlett added a comment - I've put a fuller reply on SkyDrive because I think it might be a bit long for a comment here. >> Colin: Checking on your progress, since it has been more than two or three days My apologies: I was too optimistic about the two or three days, and then kept on being optimistic it would only be another day or two: sorry. I finished the single text file summary this weekend, and checked it: that doesn't mean I haven't made any errors in it, but it does mean that I have made a big effort to avoid errors. (Admittedly not quite the same thing.) I've posted it here, along with a "please-read-first-jruby-iterator-issues.txt" file: https://skydrive.live.com/#cid=01F6D7100ADB0423 (Let me know via Twitter @colinb8 if the link doesn't work or you can't get at that file.) I suggest downloading that summary, but it's longer than I intended - I was trying to be comprehensive about the problems. So you might want to just look at the "contents" section for now, and (for now) rely on what I say in the "please-read-first-jruby-iterator-issues.txt" file. I'll put my patches on that skydrive public folder: I'll clean them up and send you a Twitter message when I've done that. Feel free to email me directly at colinwb@yahoo.co.uk: it might be a good idea to tweet me at Twitter@colinb8 the first time (if you do that) just in case Yahoo is insensitive enough to put an email in the Spam folder.
        Hide
        Colin Bartlett added a comment -

        example of error message I'm sometimes - and sometimes not - getting

        Show
        Colin Bartlett added a comment - example of error message I'm sometimes - and sometimes not - getting
        Hide
        Colin Bartlett added a comment -

        1. My patches for JRuby Fixnum iteration Java long integer overflow problems seem to be working: they pass the tests I've devised to show these problems.

        2. BUT: I've tried two types of patching, both on preview1 source (I haven't been able to successfully compile preview2):
        2.a. patch all the existing methods in RubyInteger.java, RubyNumeric.java, RubyRange.java;
        2.b. set up new generic public static methods in RubyFixnum.java, and call those from the relevant methods inRubyInteger.java, RubyNumeric.java, RubyRange.java.

        2.1. Doing 2.b I've added large numbers of "ordinary" Fixnum iterations, which shouldn't fail.

        2.2. Sometimes they don't fail, but sometimes they do, giving this type of error:
        ...java.lang.NoClassDefFoundError: org/jruby/runtime/ThreadContext...

        2.3. When the failure has occurred, I've immediately rerun the same Ruby program with the same JRuby executable jar that I compiled with my patches, and sometimes it fails at the same point, sometimes at a rather different point.

        2.4. I'll try to narrow this problem down: I'm concerned that I can't seem to properly compile JRuby java code.

        2.5. But in the meantime I think the safest way to patch is using type 2.a patches. BUT I haven't yet tried to get the types of errors I'm getting with type 2.b patches: I'll have a go at that.

        3. Trying to find out what's happening here, I found something else which might affect the patches: it looks as though in my compiled jar files "block.getBody().getArgumentType()" might be returning integer 2 even if there is no block or if the block just has one variable argument. I've out the following type of "debugging" information code in RubyArray.java and RubyInteger.java
        in places where the test "if (block.getBody().getArgumentType() == BlockBody.ZERO_ARGS)" is made,
        and limited testing seems to show that my comment is true for my system,
        compiling both preview2 and also jruby.1.6.7.2
        System.out.println("at RubyXXX.java:999:argType=" + //+
        block.getBody().getArgumentType() + " ZERO_ARGS=" + BlockBody.ZERO_ARGS); //+
        if (block.getBody().getArgumentType() == BlockBody.ZERO_ARGS) {
        then calling, for example:
        Array.new(2)

        { "noBlockArg" }

        Array.new(2)

        { |v| v }

        Charles - when you have a moment, could you try the same sort of thing to see if it's just a problem with my setup?

        Thanks.

        Show
        Colin Bartlett added a comment - 1. My patches for JRuby Fixnum iteration Java long integer overflow problems seem to be working: they pass the tests I've devised to show these problems. 2. BUT: I've tried two types of patching, both on preview1 source (I haven't been able to successfully compile preview2): 2.a. patch all the existing methods in RubyInteger.java, RubyNumeric.java, RubyRange.java; 2.b. set up new generic public static methods in RubyFixnum.java, and call those from the relevant methods inRubyInteger.java, RubyNumeric.java, RubyRange.java. 2.1. Doing 2.b I've added large numbers of "ordinary" Fixnum iterations, which shouldn't fail. 2.2. Sometimes they don't fail, but sometimes they do, giving this type of error: ...java.lang.NoClassDefFoundError: org/jruby/runtime/ThreadContext... 2.3. When the failure has occurred, I've immediately rerun the same Ruby program with the same JRuby executable jar that I compiled with my patches, and sometimes it fails at the same point, sometimes at a rather different point. 2.4. I'll try to narrow this problem down: I'm concerned that I can't seem to properly compile JRuby java code. 2.5. But in the meantime I think the safest way to patch is using type 2.a patches. BUT I haven't yet tried to get the types of errors I'm getting with type 2.b patches: I'll have a go at that. 3. Trying to find out what's happening here, I found something else which might affect the patches: it looks as though in my compiled jar files "block.getBody().getArgumentType()" might be returning integer 2 even if there is no block or if the block just has one variable argument. I've out the following type of "debugging" information code in RubyArray.java and RubyInteger.java in places where the test "if (block.getBody().getArgumentType() == BlockBody.ZERO_ARGS)" is made, and limited testing seems to show that my comment is true for my system, compiling both preview2 and also jruby.1.6.7.2 System.out.println("at RubyXXX.java:999:argType=" + //+ block.getBody().getArgumentType() + " ZERO_ARGS=" + BlockBody.ZERO_ARGS); //+ if (block.getBody().getArgumentType() == BlockBody.ZERO_ARGS) { then calling, for example: Array.new(2) { "noBlockArg" } Array.new(2) { |v| v } Charles - when you have a moment, could you try the same sort of thing to see if it's just a problem with my setup? Thanks.
        Hide
        Colin Bartlett added a comment -

        example of error message

        Show
        Colin Bartlett added a comment - example of error message
        Hide
        Charles Oliver Nutter added a comment -

        Additional fixes:

        commit ee963e52200617634eb11d9ffbf984b956f7fb21
        Author: Charles Oliver Nutter <headius@headius.com>
        Date:   Thu Nov 8 13:44:42 2012 -0600
        
            Fix JRUBY-6612, JRUBY-6777, JRUBY-6778, JRUBY-6779, JRUBY-6790
            
            [JRUBY-6612] some problems with JRuby seeming to not detect Java Long arithmetic overflows
            [JRUBY-6777] RubyFixnum.java - two methods fail to detect some long integer overflows
            [JRUBY-6778] Possible long integer overflow bug in Integer#succ in RubyInteger.java
            [JRUBY-6779] Strange behaviour of some Integer Ranges with Range#each - maybe an integer overflow problem?
            [JRUBY-6790] Possible long integer overflow in fixnumStep in RubyNumeric.java
            
            Patches by Colin Bartlett. Thank you!
        
        :100644 100644 93f83c8... 109b856... M	src/org/jruby/RubyInteger.java
        :100644 100644 b631a40... 7ecf098... M	src/org/jruby/RubyNumeric.java
        :100644 100644 3ed0b55... d15c2ed... M	src/org/jruby/RubyRange.java
        
        Show
        Charles Oliver Nutter added a comment - Additional fixes: commit ee963e52200617634eb11d9ffbf984b956f7fb21 Author: Charles Oliver Nutter <headius@headius.com> Date: Thu Nov 8 13:44:42 2012 -0600 Fix JRUBY-6612, JRUBY-6777, JRUBY-6778, JRUBY-6779, JRUBY-6790 [JRUBY-6612] some problems with JRuby seeming to not detect Java Long arithmetic overflows [JRUBY-6777] RubyFixnum.java - two methods fail to detect some long integer overflows [JRUBY-6778] Possible long integer overflow bug in Integer#succ in RubyInteger.java [JRUBY-6779] Strange behaviour of some Integer Ranges with Range#each - maybe an integer overflow problem? [JRUBY-6790] Possible long integer overflow in fixnumStep in RubyNumeric.java Patches by Colin Bartlett. Thank you! :100644 100644 93f83c8... 109b856... M src/org/jruby/RubyInteger.java :100644 100644 b631a40... 7ecf098... M src/org/jruby/RubyNumeric.java :100644 100644 3ed0b55... d15c2ed... M src/org/jruby/RubyRange.java

          People

          • Assignee:
            Charles Oliver Nutter
            Reporter:
            Colin Bartlett
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: