Harmony
  1. Harmony
  2. HARMONY-2528

[drlvm] VM crashes instead of throwing AbstractMethodError

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: DRLVM
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      The following test perfectly compiles with javac, and throws
      java.lang.AbstractMethodError: javax/management/monitor/Monitor.createMX4JMonitor()Lmx4j/monitor/MX4JMonitor;
      at javax.management.monitor.Monitor.getMX4JMonitor(Monitor.java:54)
      at javax.management.monitor.Monitor.getObservedObjects(Monitor.java:149)
      at Test1.main(Test1.java:7)
      when used with mx4j implementation. DRLVM + JIT crashes trying to compile abstract method.

      import javax.management.monitor.*;
      import javax.management.*;

      public class Test1 {

      public static void main(String[] args) throws Exception

      { (new MonitorImpl()).getObservedObjects(); }

      }
      class MonitorImpl extends javax.management.monitor.Monitor {
      @Override
      public void start() {}
      @Override
      public void stop() {}
      }

      1. Test1.java
        0.3 kB
        Alexei Fedotov
      2. H2528-AME.patch
        7 kB
        Pavel Pervov
      3. AbstractMethodError.patch
        12 kB
        Alexei Fedotov
      4. AbstractMethodError_2.patch
        12 kB
        Alexei Fedotov
      5. AbstractMethodError_1.patch
        12 kB
        Alexei Fedotov

        Issue Links

        There are no Sub-Tasks for this issue.

          Activity

          Alexei Fedotov made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Alexei Fedotov added a comment -

          Gregory, thanks - the patch is applied as expected.

          Show
          Alexei Fedotov added a comment - Gregory, thanks - the patch is applied as expected.
          Hide
          Gregory Shimansky added a comment -

          Forgot to mention that the revision for this patch is 490881.

          Show
          Gregory Shimansky added a comment - Forgot to mention that the revision for this patch is 490881.
          Gregory Shimansky made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Gregory Shimansky added a comment -

          Attached test passes now.

          Patch from HARMONY-2906 has been applied to fix this issue. Please check that it was applied as expected.

          Show
          Gregory Shimansky added a comment - Attached test passes now. Patch from HARMONY-2906 has been applied to fix this issue. Please check that it was applied as expected.
          Hide
          Gregory Shimansky added a comment -

          I assume that the patch for this issue is actually the patch attached to HARMONY-2906, called abstract_method_error_3.patch.

          Show
          Gregory Shimansky added a comment - I assume that the patch for this issue is actually the patch attached to HARMONY-2906 , called abstract_method_error_3.patch.
          Hide
          Alexei Fedotov added a comment -

          HARMONY-2906 testing completed.

          Show
          Alexei Fedotov added a comment - HARMONY-2906 testing completed.
          Alexei Fedotov made changes -
          Link This issue is blocked by HARMONY-2906 [ HARMONY-2906 ]
          Hide
          Alexei Fedotov added a comment -

          The code inspection patch simplifies the fix of HARMONY-2528.

          Show
          Alexei Fedotov added a comment - The code inspection patch simplifies the fix of HARMONY-2528 .
          Hide
          Alexei Fedotov added a comment -

          Gregory,

          I will split my patch into two patches. One will target code inspection notes, and it will be orthogonal to Pavel's patch. I will file a new JIRA issue for that patch. The rest of my patch (the actual fix) will be the following code in compile_jit_a_method stub - only it will compete with Pavel's patch.

          + tmn_suspend_enable();
          + if (method->is_abstract())

          { + compile_raise_exception("java/lang/AbstractMethodError", "Cannot compile", method); + tmn_suspend_disable(); + return NULL; + }
          Show
          Alexei Fedotov added a comment - Gregory, I will split my patch into two patches. One will target code inspection notes, and it will be orthogonal to Pavel's patch. I will file a new JIRA issue for that patch. The rest of my patch (the actual fix) will be the following code in compile_jit_a_method stub - only it will compete with Pavel's patch. + tmn_suspend_enable(); + if (method->is_abstract()) { + compile_raise_exception("java/lang/AbstractMethodError", "Cannot compile", method); + tmn_suspend_disable(); + return NULL; + }
          Hide
          Gregory Shimansky added a comment -

          Both patches fail to apply to the current source tree. Also could we have some decision made about alternative approaches? I think you could at least give reasons to both ways and explain which one is better. Patch(es) have to be updated too.

          Show
          Gregory Shimansky added a comment - Both patches fail to apply to the current source tree. Also could we have some decision made about alternative approaches? I think you could at least give reasons to both ways and explain which one is better. Patch(es) have to be updated too.
          Gregory Shimansky made changes -
          Assignee Gregory Shimansky [ gshimansky ]
          Pavel Pervov made changes -
          Attachment H2528-AME.patch [ 12346806 ]
          Hide
          Pavel Pervov added a comment -

          Here is alternative patch which fixes reported issue.

          Show
          Pavel Pervov added a comment - Here is alternative patch which fixes reported issue.
          Hide
          Pavel Pervov added a comment -

          Alexei, as per your request, I reviewed your patch. First of all it contains lots of unrelated changes. I would suggest filing separate JIRA against changes requested in http://wiki.apache.org/harmony/Code_Inspection%3a_compile%2ecpp.

          Commenting your fix, I would suggest making these changes in vm/vmcore/src/class_support/Prepare.cpp. There should be 'if' inserted for abstract methods where compile_me_stubs are assigned into methods' code addresses. Respective stub for throwing AME is already available there. Actually, I'll attach alternative patch by myself.

          Show
          Pavel Pervov added a comment - Alexei, as per your request, I reviewed your patch. First of all it contains lots of unrelated changes. I would suggest filing separate JIRA against changes requested in http://wiki.apache.org/harmony/Code_Inspection%3a_compile%2ecpp . Commenting your fix, I would suggest making these changes in vm/vmcore/src/class_support/Prepare.cpp. There should be 'if' inserted for abstract methods where compile_me_stubs are assigned into methods' code addresses. Respective stub for throwing AME is already available there. Actually, I'll attach alternative patch by myself.
          Alexei Fedotov made changes -
          Patch Info [Patch Available]
          Hide
          Alexei Fedotov added a comment -

          All tests passed for ia32 platforms (including class library tests). The patch also contains several improvements according to the last section of http://wiki.apache.org/harmony/Code_Inspection%3a_compile%2ecpp

          Please could you commit the patch?

          Show
          Alexei Fedotov added a comment - All tests passed for ia32 platforms (including class library tests). The patch also contains several improvements according to the last section of http://wiki.apache.org/harmony/Code_Inspection%3a_compile%2ecpp Please could you commit the patch?
          Alexei Fedotov made changes -
          Attachment AbstractMethodError_2.patch [ 12346730 ]
          Hide
          Alexei Fedotov added a comment -

          A questionable assertion is removed from the patch and filed as a separate issue, see HARMONY-2532.

          Show
          Alexei Fedotov added a comment - A questionable assertion is removed from the patch and filed as a separate issue, see HARMONY-2532 .
          Alexei Fedotov made changes -
          Attachment AbstractMethodError_1.patch [ 12346728 ]
          Hide
          Alexei Fedotov added a comment -

          The patch: take two. An assertion was incorrect. Still testing.

          Show
          Alexei Fedotov added a comment - The patch: take two. An assertion was incorrect. Still testing.
          Alexei Fedotov made changes -
          Field Original Value New Value
          Attachment Test1.java [ 12346722 ]
          Attachment AbstractMethodError.patch [ 12346723 ]
          Hide
          Alexei Fedotov added a comment -

          The test should be compiled with standard javac to get a class file with abstract method. I'm testing the patch now.

          Show
          Alexei Fedotov added a comment - The test should be compiled with standard javac to get a class file with abstract method. I'm testing the patch now.
          Alexei Fedotov created issue -

            People

            • Assignee:
              Gregory Shimansky
              Reporter:
              Alexei Fedotov
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development