[meta-java,2/2] openjdk-8: better interface invocations

Submitted by Attie Grande on Nov. 30, 2018, 1:10 p.m. | Patch ID: 156765

Details

Message ID CAAsqSTcN2Bkk_k_MLEWSTesEFEJURrXvHdyY2TzuA_fEh4iJUw@mail.gmail.com
State Changes Requested
Delegated to: Richard Leitner
Headers show

Commit Message

Attie Grande Nov. 30, 2018, 1:10 p.m.
Fixes a SEGFAULT when OpenJDK 8 is build for an i.MX6 platform, possibly
others.

The original patch (below) has been modified to work with jdk8.

Further information:
  https://bugs.openjdk.java.net/browse/JDK-8194739
  http://hg.openjdk.java.net/jdk/jdk10/rev/69d1a1590485

Signed-off-by: Attie Grande <attie.grande@argentum-systems.co.uk>
---
 recipes-core/openjdk/openjdk-8-release-16xbyy.inc  |  1 +
 .../JDK-8194739-better-interface-invocations.patch | 62 ++++++++++++++++++++++
 2 files changed, 63 insertions(+)
 create mode 100644
recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch

+         }
+-        int mindex = cache->f2_as_index();
++        int mindex = interface_method->itable_index();
++
+         itableMethodEntry* im = ki->first_method_entry(rcvr->klass());
+         callee = im[mindex].method();
+         if (callee == NULL) {

Patch hide | download patch | download mbox

diff --git a/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
b/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
index 23c99fe..b7cdede 100644
--- a/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
+++ b/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
@@ -17,6 +17,7 @@  PATCHES_URI = "\
     file://0009-jdk-disable-backtrace-musl-build-fix.patch \
     file://0010-build-fix-build-on-as-needed-toolchains-generic.patch \
     file://JDK-8202600-undefined-behaviour-in-os_linux_zero.cpp.patch \
+    file://JDK-8194739-better-interface-invocations.patch \
 "
 # some patches extracted from
http://cr.openjdk.java.net/~rkennke/shark-build-hotspot/webrev.01/hotspot.patch
 # reported via
http://mail.openjdk.java.net/pipermail/build-dev/2015-January/013972.html
diff --git a/recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch
b/recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch
new file mode 100644
index 0000000..a185dac
--- /dev/null
+++ b/recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch
@@ -0,0 +1,62 @@ 
+--- a/hotspot/src/cpu/zero/vm/methodHandles_zero.cpp Tue Dec 19
17:31:53 2017 -0500
++++ b/hotspot/src/cpu/zero/vm/methodHandles_zero.cpp Mon Jan 22
15:19:02 2018 +0000
+@@ -180,3 +180,9 @@
+     return NULL;
+   }
+ }
++
++#ifndef PRODUCT
++void MethodHandles::trace_method_handle(MacroAssembler* _masm, const
char* adaptername) {
++  // This is just a stub.
++}
++#endif //PRODUCT
+--- a/hotspot/src/share/vm/interpreter/bytecodeInterpreter.cpp Tue
Dec 19 17:31:53 2017 -0500
++++ b/hotspot/src/share/vm/interpreter/bytecodeInterpreter.cpp Mon
Jan 22 15:19:02 2018 +0000
+@@ -2569,13 +2569,35 @@
+
+         // this could definitely be cleaned up QQQ
+         Method* callee;
+-        Klass* iclass = cache->f1_as_klass();
+-        // InstanceKlass* interface = (InstanceKlass*) iclass;
++        Method *interface_method = cache->f2_as_interface_method();
++        InstanceKlass* iclass = interface_method->method_holder();
++
+         // get receiver
+         int parms = cache->parameter_size();
+         oop rcvr = STACK_OBJECT(-parms);
+         CHECK_NULL(rcvr);
+         InstanceKlass* int2 = (InstanceKlass*) rcvr->klass();
++
++        // Receiver subtype check against resolved interface klass (REFC).
++        {
++          Klass* refc = cache->f1_as_klass();
++          itableOffsetEntry* scan;
++          for (scan = (itableOffsetEntry*) int2->start_of_itable();
++               scan->interface_klass() != NULL;
++               scan++) {
++            if (scan->interface_klass() == refc) {
++              break;
++            }
++          }
++          // Check that the entry is non-null.  A null entry means
++          // that the receiver class doesn't implement the
++          // interface, and wasn't the same as when the caller was
++          // compiled.
++          if (scan->interface_klass() == NULL) {
++            VM_JAVA_ERROR(vmSymbols::java_lang_IncompatibleClassChangeError(),
"", note_no_trap);
++          }
++        }
++
+         itableOffsetEntry* ki = (itableOffsetEntry*) int2->start_of_itable();
+         int i;
+         for ( i = 0 ; i < int2->itable_length() ; i++, ki++ ) {
+@@ -2587,7 +2608,8 @@
+         if (i == int2->itable_length()) {
+           VM_JAVA_ERROR(vmSymbols::java_lang_IncompatibleClassChangeError(),
"", note_no_trap);

Comments

Richard Leitner Dec. 17, 2018, 1:54 p.m.
Hi,
thanks for the patch. Nonetheless this patch doesn't apply neither to master nor to master-next.
Please take a look at it. Thanks!

regards;Richard.L

On 30.11.18 14:10, Attie Grande wrote:
> Fixes a SEGFAULT when OpenJDK 8 is build for an i.MX6 platform, possibly
> others.

Do you have any logs of these segfaults?
I have no issues during my i.MX6 builds. What libc are you using?

> 
> The original patch (below) has been modified to work with jdk8.
> 
> Further information:
>   https://bugs.openjdk.java.net/browse/JDK-8194739
>   http://hg.openjdk.java.net/jdk/jdk10/rev/69d1a1590485
> 
> Signed-off-by: Attie Grande <attie.grande@argentum-systems.co.uk>
> ---
>  recipes-core/openjdk/openjdk-8-release-16xbyy.inc  |  1 +
>  .../JDK-8194739-better-interface-invocations.patch | 62 ++++++++++++++++++++++
>  2 files changed, 63 insertions(+)
>  create mode 100644
> recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch
> 
> diff --git a/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
> b/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
> index 23c99fe..b7cdede 100644
> --- a/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
> +++ b/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
> @@ -17,6 +17,7 @@ PATCHES_URI = "\
>      file://0009-jdk-disable-backtrace-musl-build-fix.patch \
>      file://0010-build-fix-build-on-as-needed-toolchains-generic.patch \
>      file://JDK-8202600-undefined-behaviour-in-os_linux_zero.cpp.patch \
> +    file://JDK-8194739-better-interface-invocations.patch \
>  "
>  # some patches extracted from
> http://cr.openjdk.java.net/~rkennke/shark-build-hotspot/webrev.01/hotspot.patch
>  # reported via
> http://mail.openjdk.java.net/pipermail/build-dev/2015-January/013972.html
> diff --git a/recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch
> b/recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch
> new file mode 100644
> index 0000000..a185dac
> --- /dev/null
> +++ b/recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch
> @@ -0,0 +1,62 @@
> +--- a/hotspot/src/cpu/zero/vm/methodHandles_zero.cpp Tue Dec 19
> 17:31:53 2017 -0500
> ++++ b/hotspot/src/cpu/zero/vm/methodHandles_zero.cpp Mon Jan 22
> 15:19:02 2018 +0000
> +@@ -180,3 +180,9 @@
> +     return NULL;
> +   }
> + }
> ++
> ++#ifndef PRODUCT
> ++void MethodHandles::trace_method_handle(MacroAssembler* _masm, const
> char* adaptername) {
> ++  // This is just a stub.
> ++}
> ++#endif //PRODUCT
> +--- a/hotspot/src/share/vm/interpreter/bytecodeInterpreter.cpp Tue
> Dec 19 17:31:53 2017 -0500
> ++++ b/hotspot/src/share/vm/interpreter/bytecodeInterpreter.cpp Mon
> Jan 22 15:19:02 2018 +0000
> +@@ -2569,13 +2569,35 @@
> +
> +         // this could definitely be cleaned up QQQ
> +         Method* callee;
> +-        Klass* iclass = cache->f1_as_klass();
> +-        // InstanceKlass* interface = (InstanceKlass*) iclass;
> ++        Method *interface_method = cache->f2_as_interface_method();
> ++        InstanceKlass* iclass = interface_method->method_holder();
> ++
> +         // get receiver
> +         int parms = cache->parameter_size();
> +         oop rcvr = STACK_OBJECT(-parms);
> +         CHECK_NULL(rcvr);
> +         InstanceKlass* int2 = (InstanceKlass*) rcvr->klass();
> ++
> ++        // Receiver subtype check against resolved interface klass (REFC).
> ++        {
> ++          Klass* refc = cache->f1_as_klass();
> ++          itableOffsetEntry* scan;
> ++          for (scan = (itableOffsetEntry*) int2->start_of_itable();
> ++               scan->interface_klass() != NULL;
> ++               scan++) {
> ++            if (scan->interface_klass() == refc) {
> ++              break;
> ++            }
> ++          }
> ++          // Check that the entry is non-null.  A null entry means
> ++          // that the receiver class doesn't implement the
> ++          // interface, and wasn't the same as when the caller was
> ++          // compiled.
> ++          if (scan->interface_klass() == NULL) {
> ++            VM_JAVA_ERROR(vmSymbols::java_lang_IncompatibleClassChangeError(),
> "", note_no_trap);
> ++          }
> ++        }
> ++
> +         itableOffsetEntry* ki = (itableOffsetEntry*) int2->start_of_itable();
> +         int i;
> +         for ( i = 0 ; i < int2->itable_length() ; i++, ki++ ) {
> +@@ -2587,7 +2608,8 @@
> +         if (i == int2->itable_length()) {
> +           VM_JAVA_ERROR(vmSymbols::java_lang_IncompatibleClassChangeError(),
> "", note_no_trap);
> +         }
> +-        int mindex = cache->f2_as_index();
> ++        int mindex = interface_method->itable_index();
> ++
> +         itableMethodEntry* im = ki->first_method_entry(rcvr->klass());
> +         callee = im[mindex].method();
> +         if (callee == NULL) {
>
Attie Grande Dec. 17, 2018, 2:07 p.m.
Hi Richard,

It looks like gmail has helpfully wrapped some of the lines...
Also, my patch was intended to go on top of the sumo branch - apologies.

It looks like you've bumped to 172b11 on master-next already.
I'll try this and report back - possibly in the new year...

Some crashes are available here:
  - https://pastebin.com/raw/f1PB4uJm
  - https://pastebin.com/raw/iXTz8v9z

Attie


On Mon, 17 Dec 2018 at 13:55, Richard Leitner
<richard.leitner@skidata.com> wrote:
>
> Hi,
> thanks for the patch. Nonetheless this patch doesn't apply neither to master nor to master-next.
> Please take a look at it. Thanks!
>
> regards;Richard.L
>
> On 30.11.18 14:10, Attie Grande wrote:
> > Fixes a SEGFAULT when OpenJDK 8 is build for an i.MX6 platform, possibly
> > others.
>
> Do you have any logs of these segfaults?
> I have no issues during my i.MX6 builds. What libc are you using?
>
> >
> > The original patch (below) has been modified to work with jdk8.
> >
> > Further information:
> >   https://bugs.openjdk.java.net/browse/JDK-8194739
> >   http://hg.openjdk.java.net/jdk/jdk10/rev/69d1a1590485
> >
> > Signed-off-by: Attie Grande <attie.grande@argentum-systems.co.uk>
> > ---
> >  recipes-core/openjdk/openjdk-8-release-16xbyy.inc  |  1 +
> >  .../JDK-8194739-better-interface-invocations.patch | 62 ++++++++++++++++++++++
> >  2 files changed, 63 insertions(+)
> >  create mode 100644
> > recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch
> >
> > diff --git a/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
> > b/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
> > index 23c99fe..b7cdede 100644
> > --- a/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
> > +++ b/recipes-core/openjdk/openjdk-8-release-16xbyy.inc
> > @@ -17,6 +17,7 @@ PATCHES_URI = "\
> >      file://0009-jdk-disable-backtrace-musl-build-fix.patch \
> >      file://0010-build-fix-build-on-as-needed-toolchains-generic.patch \
> >      file://JDK-8202600-undefined-behaviour-in-os_linux_zero.cpp.patch \
> > +    file://JDK-8194739-better-interface-invocations.patch \
> >  "
> >  # some patches extracted from
> > http://cr.openjdk.java.net/~rkennke/shark-build-hotspot/webrev.01/hotspot.patch
> >  # reported via
> > http://mail.openjdk.java.net/pipermail/build-dev/2015-January/013972.html
> > diff --git a/recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch
> > b/recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch
> > new file mode 100644
> > index 0000000..a185dac
> > --- /dev/null
> > +++ b/recipes-core/openjdk/patches-openjdk-8/JDK-8194739-better-interface-invocations.patch
> > @@ -0,0 +1,62 @@
> > +--- a/hotspot/src/cpu/zero/vm/methodHandles_zero.cpp Tue Dec 19
> > 17:31:53 2017 -0500
> > ++++ b/hotspot/src/cpu/zero/vm/methodHandles_zero.cpp Mon Jan 22
> > 15:19:02 2018 +0000
> > +@@ -180,3 +180,9 @@
> > +     return NULL;
> > +   }
> > + }
> > ++
> > ++#ifndef PRODUCT
> > ++void MethodHandles::trace_method_handle(MacroAssembler* _masm, const
> > char* adaptername) {
> > ++  // This is just a stub.
> > ++}
> > ++#endif //PRODUCT
> > +--- a/hotspot/src/share/vm/interpreter/bytecodeInterpreter.cpp Tue
> > Dec 19 17:31:53 2017 -0500
> > ++++ b/hotspot/src/share/vm/interpreter/bytecodeInterpreter.cpp Mon
> > Jan 22 15:19:02 2018 +0000
> > +@@ -2569,13 +2569,35 @@
> > +
> > +         // this could definitely be cleaned up QQQ
> > +         Method* callee;
> > +-        Klass* iclass = cache->f1_as_klass();
> > +-        // InstanceKlass* interface = (InstanceKlass*) iclass;
> > ++        Method *interface_method = cache->f2_as_interface_method();
> > ++        InstanceKlass* iclass = interface_method->method_holder();
> > ++
> > +         // get receiver
> > +         int parms = cache->parameter_size();
> > +         oop rcvr = STACK_OBJECT(-parms);
> > +         CHECK_NULL(rcvr);
> > +         InstanceKlass* int2 = (InstanceKlass*) rcvr->klass();
> > ++
> > ++        // Receiver subtype check against resolved interface klass (REFC).
> > ++        {
> > ++          Klass* refc = cache->f1_as_klass();
> > ++          itableOffsetEntry* scan;
> > ++          for (scan = (itableOffsetEntry*) int2->start_of_itable();
> > ++               scan->interface_klass() != NULL;
> > ++               scan++) {
> > ++            if (scan->interface_klass() == refc) {
> > ++              break;
> > ++            }
> > ++          }
> > ++          // Check that the entry is non-null.  A null entry means
> > ++          // that the receiver class doesn't implement the
> > ++          // interface, and wasn't the same as when the caller was
> > ++          // compiled.
> > ++          if (scan->interface_klass() == NULL) {
> > ++            VM_JAVA_ERROR(vmSymbols::java_lang_IncompatibleClassChangeError(),
> > "", note_no_trap);
> > ++          }
> > ++        }
> > ++
> > +         itableOffsetEntry* ki = (itableOffsetEntry*) int2->start_of_itable();
> > +         int i;
> > +         for ( i = 0 ; i < int2->itable_length() ; i++, ki++ ) {
> > +@@ -2587,7 +2608,8 @@
> > +         if (i == int2->itable_length()) {
> > +           VM_JAVA_ERROR(vmSymbols::java_lang_IncompatibleClassChangeError(),
> > "", note_no_trap);
> > +         }
> > +-        int mindex = cache->f2_as_index();
> > ++        int mindex = interface_method->itable_index();
> > ++
> > +         itableMethodEntry* im = ki->first_method_entry(rcvr->klass());
> > +         callee = im[mindex].method();
> > +         if (callee == NULL) {
> >
Richard Leitner Dec. 17, 2018, 2:47 p.m.
Hi,

On 17.12.18 15:07, Attie Grande wrote:
> Hi Richard,
> 
> It looks like gmail has helpfully wrapped some of the lines...
> Also, my patch was intended to go on top of the sumo branch - apologies.

No problem, please just put [sumo] in the subject then.

> 
> It looks like you've bumped to 172b11 on master-next already.
> I'll try this and report back - possibly in the new year...

Ok. Just fyi: We will likely update to 8u181b13 soon
(see https://patchwork.openembedded.org/patch/156314/)

> 
> Some crashes are available here:
>   - https://pastebin.com/raw/f1PB4uJm
>   - https://pastebin.com/raw/iXTz8v9z

Thanks, i will take a look at them.

> 
> Attie

regards;Richard.L
Attie Grande Dec. 17, 2018, 3:12 p.m.
> No problem, please just put [sumo] in the subject then.

Sure, thanks for letting me know

> Ok. Just fyi: We will likely update to 8u181b13 soon
> (see https://patchwork.openembedded.org/patch/156314/)

Great - I'll keep a lookout!

There is further information about my issue on jdk-dev.
The thread starts here:

  http://mail.openjdk.java.net/pipermail/jdk-dev/2018-November/002246.html

Attie
André Draszik Jan. 2, 2019, 8:25 p.m.
Hi,

I don't seem to have this patch in my mailbox for whatever reason...

but from a quick glance https://patchwork.openembedded.org/patch/156314/

just bumps the version of the the aarch32 and aarch64 ports to 181b13
without actually updating them - if I'm not mistaken.

It should be updated to not do that before merging, or ideally even update
the two aarch ports as well. 

Not sure if there are other issues...

Cheers,
Andre'


On Mon, 2018-12-17 at 15:47 +0100, Richard Leitner wrote:
> Hi,
> 
> On 17.12.18 15:07, Attie Grande wrote:
> > Hi Richard,
> > 
> > It looks like gmail has helpfully wrapped some of the lines...
> > Also, my patch was intended to go on top of the sumo branch - apologies.
> 
> No problem, please just put [sumo] in the subject then.
> 
> > It looks like you've bumped to 172b11 on master-next already.
> > I'll try this and report back - possibly in the new year...
> 
> Ok. Just fyi: We will likely update to 8u181b13 soon
> (see https://patchwork.openembedded.org/patch/156314/)
> 
> > Some crashes are available here:
> >   - https://pastebin.com/raw/f1PB4uJm
> >   - https://pastebin.com/raw/iXTz8v9z
> 
> Thanks, i will take a look at them.
> 
> > Attie
> 
> regards;Richard.L
Kyle Russell Jan. 28, 2019, 6:50 p.m.
I encountered the same problem on ARM, and arrived at the same upstream
patch independently.  Would you like me to send my meta-java patch prefixed
with [sumo]?  I'd be interested in having this fix backported, if you're
still taking fixes for sumo.

On Mon, Dec 17, 2018 at 10:12 AM Attie Grande <
attie.grande@argentum-systems.co.uk> wrote:

> > No problem, please just put [sumo] in the subject then.
>
> Sure, thanks for letting me know
>
> > Ok. Just fyi: We will likely update to 8u181b13 soon
> > (see https://patchwork.openembedded.org/patch/156314/)
>
> Great - I'll keep a lookout!
>
> There is further information about my issue on jdk-dev.
> The thread starts here:
>
>   http://mail.openjdk.java.net/pipermail/jdk-dev/2018-November/002246.html
>
> Attie
> --
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
>