RVM
  1. RVM
  2. RVM-831

Incomplete implementation of GetFieldID and GetStaticFieldID

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.1.0
    • Fix Version/s: 3.1.1
    • Component/s: Runtime: JNI
    • Labels:
      None
    • Environment:
      IA32, and Ubuntu
    • Testcase included:
      yes
    • Patch Submitted:
      Yes
    • Number of attachments :
      3

      Description

      Both GetFieldID and GetStaticFieldID does not correctly run the test case which I attach.
      Jikes RVM produced wrong results, which are different from what Hotspot 1.6.0_13 and J9 1.6(SR4) showed:

      $rvm TestJNIGetFieldID
      instance_a: fail
      static_s: fail
      static_f: fail

      $java -showversion TestJNIGetFieldID
      java version "1.6.0_13"
      Java(TM) SE Runtime Environment (build 1.6.0_13-b03)
      Java HotSpot(TM) Server VM (build 11.3-b02, mixed mode)
      instance_a: pass
      static_s: pass
      static_f: pass

      $java -showversion TestJNIGetFieldID
      java version "1.6.0"
      Java(TM) SE Runtime Environment (build pxi3260sr4-20090219_01(SR4))
      IBM J9 VM (build 2.4, J2RE 1.6.0 IBM J9 2.4 Linux x86-32 jvmxi3260-20090215_29883 (JIT enabled, AOT enabled)
      J9VM - 20090215_029883_lHdSMr
      JIT - r9_20090213_2028
      GC - 20090213_AA)
      JCL - 20090218_01
      instance_a: pass
      static_s: pass
      static_f: pass

      GetFieldID could not handle a hidden instance field, and GetStaticFieldID could not find
      a static field in super classes or interfaces. I attach my patch.

      1. JNIGetFieldID.patch
        3 kB
        Byeong Lee
      2. TestJNIGetFieldID.c
        0.8 kB
        Byeong Lee
      3. TestJNIGetFieldID.java
        1 kB
        Byeong Lee

        Activity

        Hide
        David Grove added a comment -

        The patch is more complex than it needs to be to fix the problem. Working on simpler fix.

        Show
        David Grove added a comment - The patch is more complex than it needs to be to fix the problem. Working on simpler fix.
        Hide
        David Grove added a comment -

        fix and test cases committed in r15712.

        Show
        David Grove added a comment - fix and test cases committed in r15712.
        Hide
        Byeong Lee added a comment - - edited

        The r15712 does not yet complete the GetStaticFieldID, and here is an updated testcase.

        m$svn diff  testing/tests/jni/src/TestJNIGetFieldID.java
        Index: testing/tests/jni/src/TestJNIGetFieldID.java
        ===================================================================
        --- testing/tests/jni/src/TestJNIGetFieldID.java	(revision 15712)
        +++ testing/tests/jni/src/TestJNIGetFieldID.java	(working copy)
        @@ -74,6 +74,18 @@
               allTestPass = false;
             }
         
        +    try {
        +      if (getStaticFinalF(I.class) == 1) {
        +        if (verbose) System.out.println("static_I.f: pass");
        +      } else {
        +        if (verbose) System.out.println("static_I.f: fail");
        +        allTestPass = false;
        +      }
        +    } catch(Throwable e) {
        +      if (verbose) System.out.println("static_f: fail");
        +      allTestPass = false;
        +    }
        +
             if (allTestPass) {
               System.out.println("PASS: TestJNIGetFieldID");
             } else {
        

        Here is a failure in Jikes RVM, and successes in Hotspot and J9:

        $rvm -showversion -Djava.library.path=./target/tests/byeong/prototype/jni -classpath ./target/tests/byeong/prototype/jni/classes TestJNIGetFieldID
        production Jikes RVM 3.1.0+svn (r15711M)
        instance_a: pass
        static_s: pass
        static_f: pass
        static_f: fail
        FAIL: TestJNIGetFieldID
        
        $java -showversion -Djava.library.path=./target/tests/byeong/prototype/jni -classpath ./target/tests/byeong/prototype/jni/classes TestJNIGetFieldID
        java version "1.6.0_13"
        Java(TM) SE Runtime Environment (build 1.6.0_13-b03)
        Java HotSpot(TM) Server VM (build 11.3-b02, mixed mode)
        
        instance_a: pass
        static_s: pass
        static_f: pass
        static_I.f: pass
        PASS: TestJNIGetFieldID
        
        $java -showversion -Djava.library.path=./target/tests/byeong/prototype/jni -classpath ./target/tests/byeong/prototype/jni/classes TestJNIGetFieldID
        java version "1.6.0"
        Java(TM) SE Runtime Environment (build pxi3260sr4-20090219_01(SR4))
        IBM J9 VM (build 2.4, J2RE 1.6.0 IBM J9 2.4 Linux x86-32 jvmxi3260-20090215_29883 (JIT enabled, AOT enabled)
        J9VM - 20090215_029883_lHdSMr
        JIT  - r9_20090213_2028
        GC   - 20090213_AA)
        JCL  - 20090218_01
        
        instance_a: pass
        static_s: pass
        static_f: pass
        static_I.f: pass
        PASS: TestJNIGetFieldID
        
        

        Here is a proposed patch and the result:

        $svn diff rvm/src/org/jikesrvm/jni/JNIFunctions.java
        Index: rvm/src/org/jikesrvm/jni/JNIFunctions.java
        ===================================================================
        --- rvm/src/org/jikesrvm/jni/JNIFunctions.java	(revision 15712)
        +++ rvm/src/org/jikesrvm/jni/JNIFunctions.java	(working copy)
        @@ -3447,12 +3447,13 @@
                     }
                   }
                 }
        -        // Now search all implemented interfaces (includes inherited interfaces)
        -        for (RVMClass curClass : rvmType.asClass().getAllImplementedInterfaces()) {
        -          for (RVMField field : curClass.getStaticFields()) {
        -            if (field.getName() == fieldName && field.getDescriptor() == descriptor) {
        +      }
        +
        +      // Now search all implemented interfaces (includes inherited interfaces)
        +      for (RVMClass curClass : rvmType.asClass().getAllImplementedInterfaces()) {
        +        for (RVMField field : curClass.getStaticFields()) {
        +          if (field.getName() == fieldName && field.getDescriptor() == descriptor) {
                       return field.getId();
        -            }
                   }
                 }
               }
        
        $rvm -showversion -Djava.library.path=./target/tests/byeong/prototype/jni -classpath ./target/tests/byeong/prototype/jni/classes TestJNIGetFieldID
        prototype Jikes RVM 3.1.0+svn (r15712M)
        instance_a: pass
        static_s: pass
        static_f: pass
        static_I.f: pass
        PASS: TestJNIGetFieldID
        

        The RVM r15711rejects an interface class, which the JNI specification allows:
        ". The field may be defined in the class or interface referred to by clazz or in one of its superclass or superinterfaces. "
        http://java.sun.com/docs/books/jni/html/functions.html#65115

        Show
        Byeong Lee added a comment - - edited The r15712 does not yet complete the GetStaticFieldID, and here is an updated testcase. m$svn diff testing/tests/jni/src/TestJNIGetFieldID.java Index: testing/tests/jni/src/TestJNIGetFieldID.java =================================================================== --- testing/tests/jni/src/TestJNIGetFieldID.java (revision 15712) +++ testing/tests/jni/src/TestJNIGetFieldID.java (working copy) @@ -74,6 +74,18 @@ allTestPass = false; } + try { + if (getStaticFinalF(I.class) == 1) { + if (verbose) System.out.println("static_I.f: pass"); + } else { + if (verbose) System.out.println("static_I.f: fail"); + allTestPass = false; + } + } catch(Throwable e) { + if (verbose) System.out.println("static_f: fail"); + allTestPass = false; + } + if (allTestPass) { System.out.println("PASS: TestJNIGetFieldID"); } else { Here is a failure in Jikes RVM, and successes in Hotspot and J9: $rvm -showversion -Djava.library.path=./target/tests/byeong/prototype/jni -classpath ./target/tests/byeong/prototype/jni/classes TestJNIGetFieldID production Jikes RVM 3.1.0+svn (r15711M) instance_a: pass static_s: pass static_f: pass static_f: fail FAIL: TestJNIGetFieldID $java -showversion -Djava.library.path=./target/tests/byeong/prototype/jni -classpath ./target/tests/byeong/prototype/jni/classes TestJNIGetFieldID java version "1.6.0_13" Java(TM) SE Runtime Environment (build 1.6.0_13-b03) Java HotSpot(TM) Server VM (build 11.3-b02, mixed mode) instance_a: pass static_s: pass static_f: pass static_I.f: pass PASS: TestJNIGetFieldID $java -showversion -Djava.library.path=./target/tests/byeong/prototype/jni -classpath ./target/tests/byeong/prototype/jni/classes TestJNIGetFieldID java version "1.6.0" Java(TM) SE Runtime Environment (build pxi3260sr4-20090219_01(SR4)) IBM J9 VM (build 2.4, J2RE 1.6.0 IBM J9 2.4 Linux x86-32 jvmxi3260-20090215_29883 (JIT enabled, AOT enabled) J9VM - 20090215_029883_lHdSMr JIT - r9_20090213_2028 GC - 20090213_AA) JCL - 20090218_01 instance_a: pass static_s: pass static_f: pass static_I.f: pass PASS: TestJNIGetFieldID Here is a proposed patch and the result: $svn diff rvm/src/org/jikesrvm/jni/JNIFunctions.java Index: rvm/src/org/jikesrvm/jni/JNIFunctions.java =================================================================== --- rvm/src/org/jikesrvm/jni/JNIFunctions.java (revision 15712) +++ rvm/src/org/jikesrvm/jni/JNIFunctions.java (working copy) @@ -3447,12 +3447,13 @@ } } } - // Now search all implemented interfaces (includes inherited interfaces) - for (RVMClass curClass : rvmType.asClass().getAllImplementedInterfaces()) { - for (RVMField field : curClass.getStaticFields()) { - if (field.getName() == fieldName && field.getDescriptor() == descriptor) { + } + + // Now search all implemented interfaces (includes inherited interfaces) + for (RVMClass curClass : rvmType.asClass().getAllImplementedInterfaces()) { + for (RVMField field : curClass.getStaticFields()) { + if (field.getName() == fieldName && field.getDescriptor() == descriptor) { return field.getId(); - } } } } $rvm -showversion -Djava.library.path=./target/tests/byeong/prototype/jni -classpath ./target/tests/byeong/prototype/jni/classes TestJNIGetFieldID prototype Jikes RVM 3.1.0+svn (r15712M) instance_a: pass static_s: pass static_f: pass static_I.f: pass PASS: TestJNIGetFieldID The RVM r15711rejects an interface class, which the JNI specification allows: ". The field may be defined in the class or interface referred to by clazz or in one of its superclass or superinterfaces. " http://java.sun.com/docs/books/jni/html/functions.html#65115
        Hide
        David Grove added a comment -

        Hi BK,

        Are you actually working with a more complex example in which the order of traversing superclasses vs. superinterfaces matters? The current svn head passes the extended test case for me. So unless it is a question of ordering (the field is declared in both an interface of this class and as a field of a superclass and we are supposed to find the interface one in preference to the superclass one), then I don't see why it isn't working for you.

        Show
        David Grove added a comment - Hi BK, Are you actually working with a more complex example in which the order of traversing superclasses vs. superinterfaces matters? The current svn head passes the extended test case for me. So unless it is a question of ordering (the field is declared in both an interface of this class and as a field of a superclass and we are supposed to find the interface one in preference to the superclass one), then I don't see why it isn't working for you.
        Hide
        Byeong Lee added a comment -

        Hi Dave,

        I am not working with the complex example on Jikes RVM. I also observed that the SVN head passed the extended test.
        For some reason, my local version of Jikes RVM did not work with the extended test. The RVM-831 is OK to be closed.

        Show
        Byeong Lee added a comment - Hi Dave, I am not working with the complex example on Jikes RVM. I also observed that the SVN head passed the extended test. For some reason, my local version of Jikes RVM did not work with the extended test. The RVM-831 is OK to be closed.
        Hide
        David Grove added a comment -

        ok, Appreciate getting nice test cases along with the bug report...thanks!

        Show
        David Grove added a comment - ok, Appreciate getting nice test cases along with the bug report...thanks!

          People

          • Assignee:
            David Grove
            Reporter:
            Byeong Lee
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: