Bug 1492139

Summary: [systemtap] Hotspot object_alloc tapset uses HeapWordSize incorrectly
Product: [Fedora] Fedora Reporter: Severin Gehwolf <sgehwolf>
Component: java-1.8.0-openjdkAssignee: jiri vanek <jvanek>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 26CC: ahughes, dbhole, jerboaa, jvanek, msrb, mvala, omajid, sgehwolf
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: java-1.8.0-openjdk-1.8.0.151-1.b12.fc26.x86_64 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-05-03 04:37:55 EDT Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Attachments:
Description Flags
Expected output.
none
Proposed fix
none
Proposed fix (correct version) none

Description Severin Gehwolf 2017-09-15 11:03:21 EDT
Created attachment 1326495 [details]
Expected output.

Description of problem:
Using probe hotspot.object_alloc fails with an error that HeapWordSize variable cannot be read. The reason for this could be a multitude. However, since the hotspot code passes the correct size already[1] I believe this is not necessary and thus can get removed from the tapset file "hotspot-1.8.0.stp.in".

Version-Release number of selected component (if applicable):
$ rpm -q java-1.8.0-openjdk
java-1.8.0-openjdk-1.8.0.144-5.b01.fc26.x86_64

How reproducible:
100%

Steps to Reproduce:
$ cat > HelloWorld.java <<EOF
public class HelloWorld {
  public static void main(String[] args) {
    System.out.println("Hello World");
  }
}
EOF
$ javac HelloWorld.java
$ wget http://icedtea.classpath.org/~vanaltj/stapexamples/jobjectstats2.stp
$ cp /usr/lib/jvm/java/include/jni.h /usr/lib/jvm/java/include/linux/jni_md.h . && \
  sudo stap -c "/usr/lib/jvm/java/bin/java -XX:+DTraceAllocProbes -XX:+ExtendedDTraceProbes HelloWorld" jobjectstats2.stp && \
  sudo rm -rf /root/.systemtap/cache; rm jni.h jni_md.h

Actual results:
semantic error: while processing probe process("/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.144-5.b01.fc26.x86_64/jre/lib/amd64/server/libjvm.so").statement(0x9d05ce) from: process("/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.144-5.b01.fc26.x86_64/jre/lib/amd64/server/libjvm.so").statement(0x9d05ce) from: process("/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.144-5.b01.fc26.x86_64/jre/lib/amd64/server/libjvm.so").mark("object__alloc") from: hotspot.object_alloc from: hotspot.object_alloc

semantic error: unable to find local 'HeapWordSize', [man error::dwarf] dieoffset 0x62301d6 in /usr/lib/jvm/java-1.8.0-openjdk-1.8.0.144-5.b01.fc26.x86_64/jre/lib/amd64/server/libjvm.so, near pc 0x9d05ce in <unknown> /usr/src/debug/java-1.8.0-openjdk-1.8.0.144-5.b01.fc26.x86_64/openjdk/hotspot/src/share/vm/runtime/sharedRuntime.cpp (alternatives: $name, $klass)): identifier '$HeapWordSize' at /usr/share/systemtap/tapset/x86_64/hotspot-1.8.0.144-5.b01.fc26.x86_64.stp:117:18
        source:   size = $arg4 * $HeapWordSize; // Note - systemtap-alloc-size-workaround.patch
                                 ^

semantic error: target-symbol requires debuginfo: identifier '$HeapWordSize' at :117:18
        source:   size = $arg4 * $HeapWordSize; // Note - systemtap-alloc-size-workaround.patch
                                 ^

semantic error: target-symbol requires debuginfo: identifier '$HeapWordSize' at :117:18
        source:   size = $arg4 * $HeapWordSize; // Note - systemtap-alloc-size-workaround.patch
                                 ^

Pass 2: analysis failed.  [man error::pass2]

Expected results:
Works. I'm attaching a file with expected output.

Additional info:
I don't know where this patch needs to go. Please route me into the right direction and I'll get it pushed to IcedTea.
Comment 2 Severin Gehwolf 2017-09-15 11:07 EDT
Created attachment 1326496 [details]
Proposed fix
Comment 3 Severin Gehwolf 2017-09-15 12:25 EDT
Created attachment 1326559 [details]
Proposed fix (correct version)
Comment 5 Andrew John Hughes 2017-09-20 12:31:42 EDT
The best way to handle this is to file upstream bug(s) in IcedTea and then we can sync that to RHEL & Fedora during updates.

Does this affect java-1.7.0-openjdk too?
Comment 6 Severin Gehwolf 2017-09-20 12:52:23 EDT
(In reply to Andrew John Hughes from comment #5)
> The best way to handle this is to file upstream bug(s) in IcedTea and then
> we can sync that to RHEL & Fedora during updates.

OK.

> Does this affect java-1.7.0-openjdk too?

I don't know as I haven't tried. The reproducer should work with minimal changes if you'd like to test.
Comment 7 Andrew John Hughes 2017-10-13 11:58:05 EDT
Same on 7:

semantic error: while processing probe process("/usr/lib/jvm/java-1.7.0-openjdk-1.7.0.151-2.6.11.1.el7_4.x86_64/jre/lib/amd64/server/libjvm.so").statement(0x89b37e) from: process("/usr/lib/jvm/java-1.7.0-openjdk-1.7.0.151-2.6.11.1.el7_4.x86_64/jre/lib/amd64/server/libjvm.so").statement(0x89b37e) from: process("/usr/lib/jvm/java-1.7.0-openjdk-1.7.0.151-2.6.11.1.el7_4.x86_64/jre/lib/amd64/server/libjvm.so").mark("object__alloc") from: hotspot.object_alloc from: hotspot.object_alloc

semantic error: unable to find local 'HeapWordSize', [man error::dwarf] dieoffset 0x5d5c1c6 in /usr/lib/jvm/java-1.7.0-openjdk-1.7.0.151-2.6.11.1.el7_4.x86_64/jre/lib/amd64/server/libjvm.so, near pc 0x89b37e in <unknown> /usr/src/debug/java-1.7.0-openjdk-1.7.0.151-2.6.11.1.el7_4.x86_64/openjdk/hotspot/src/share/vm/runtime/sharedRuntime.cpp (alternatives: $size, $name, $klass)): identifier '$HeapWordSize' at /usr/share/systemtap/tapset/x86_64/hotspot-1.7.0.151-2.6.11.1.el7_4.stp:117:18
        source:   size = $arg4 * $HeapWordSize; // Note - systemtap-alloc-size-workaround.patch
                                 ^

semantic error: target-symbol requires debuginfo: identifier '$HeapWordSize' at :117:18
        source:   size = $arg4 * $HeapWordSize; // Note - systemtap-alloc-size-workaround.patch
                                 ^

semantic error: target-symbol requires debuginfo: identifier '$HeapWordSize' at :117:18
        source:   size = $arg4 * $HeapWordSize; // Note - systemtap-alloc-size-workaround.patch
Comment 8 Andrew John Hughes 2017-10-16 23:35:15 EDT
This is the original introduction of it:

2010-07-28  Mark Wielaard  <mjw@redhat.com>

	Generating a dwarf location expression for variable * "constant" as used
        in this probe argument triggers unforseen complications. See
        https://bugzilla.redhat.com/show_bug.cgi?id=613824
        Workaround for now by passing the size without HeapWordSize adjustment.
	See also the hotspot.object_alloc in tapset/hotspot.stp[.in].
        * Makefile.am: Add patch.
	* NEWS: Updated.
        * patches/systemtap-alloc-size-workaround.patch:
	New patch to remove HeapWordSize adjustment.
	* tapset/hotspot.stp.in: Add adjustment here.

It looks like the OpenJDK side of the patch was later removed from IcedTea 2.x, but the tapset change wasn't reverted.

As far as I can tell from bug 613824, the original underlying issue is a GCC bug which has long since been fixed.
Comment 9 Severin Gehwolf 2017-10-26 04:44:28 EDT
It's my understanding that all there is left to do is to sync systemtap tapsets from icedtea-3.6.0pre02 tag in the the java-1.8.0-openjdk package. Re-assigning to Jiri.
Comment 10 jiri vanek 2017-10-26 05:10:29 EDT
Thanx. 3.6.0pre2 is part of CPU for feodra.
Comment 11 Severin Gehwolf 2017-10-26 05:38:51 EDT
(In reply to jiri vanek from comment #10)
> Thanx. 3.6.0pre2 is part of CPU for feodra.

Cool. Works for me now. To be precise java-1.8.0-openjdk-1.8.0.151-1.b12 and better should have the fix.
Comment 12 Andrew John Hughes 2017-10-26 12:11:43 EDT
Yes,

https://src.fedoraproject.org/rpms/java-1.8.0-openjdk/c/5d847512a3b55f72b8ce5a39dd71d8a322d272fa

is largely a copy of what I added to RHEL, including the SystemTap bump, despite Jiri claiming credit for it and losing the reasoning for many of these changes.
Comment 13 jiri vanek 2017-10-27 02:57:48 EDT
(In reply to Andrew John Hughes from comment #12)
> Yes,
> 
> https://src.fedoraproject.org/rpms/java-1.8.0-openjdk/c/
> 5d847512a3b55f72b8ce5a39dd71d8a322d272fa

here on line 202 of spec change.
> 
> is largely a copy of what I added to RHEL, including the SystemTap bump,

Its not largely a copy. It is exact clone.

> despite Jiri claiming credit for it

I never claim the credit for your work.  I always mention you in chnagelog.
If you feel bad about the way I do it, pleas, ping me privately and lets agree on way how yo would like to be credited. You understand depths of JDK so much more that I don't have slightest ambition  to claim them.

> and losing the reasoning for many of  these changes.

Not losing. Never heaving it. Thank you very keeping an code related stuff in shape like nobody other!
Comment 14 Andrew John Hughes 2017-10-30 11:25:48 EDT
(In reply to jiri vanek from comment #13)
> (In reply to Andrew John Hughes from comment #12)
> > Yes,
> > 
> > https://src.fedoraproject.org/rpms/java-1.8.0-openjdk/c/
> > 5d847512a3b55f72b8ce5a39dd71d8a322d272fa
> 
> here on line 202 of spec change.
> > 
> > is largely a copy of what I added to RHEL, including the SystemTap bump,
> 
> Its not largely a copy. It is exact clone.
> 
> > despite Jiri claiming credit for it
> 
> I never claim the credit for your work.  I always mention you in chnagelog.
> If you feel bad about the way I do it, pleas, ping me privately and lets
> agree on way how yo would like to be credited. You understand depths of JDK
> so much more that I don't have slightest ambition  to claim them.
> 
> > and losing the reasoning for many of  these changes.
> 
> Not losing. Never heaving it. Thank you very keeping an code related stuff
> in shape like nobody other!

Well, now you're doing yourself a dis-service because there are new changes of yours in there too, like the copy-jdk-configs work and two new patches. It actually reminded me that I need to backport that ARM one in IcedTea.

I was just referring to the changelog explicitly. It's one entry credited to you and, as you haven't used the original entries, you lose the description of why some patches are being added. I would have thought it was less work for you to just copy the originals over :)

It's up to you how you maintain the Fedora RPMs. I just find it hard to follow, and a little disingenuous, to have both imported and new changes all merged together under one changelog entry credited to yourself. Personally, when I'm moving things from branch to branch, I prefer to keep a 1 to 1 relationship where possible so the changes remain small, I know what's been applied where and you can use git blame easily to track down where something changed. 

As is, if you need to bring your Fedora changes over to RHEL, you now have to dig them out of that big changeset. If that works for you, then so be it, but it would drive me mad :-)
Comment 15 Fedora End Of Life 2018-05-03 04:21:16 EDT
This message is a reminder that Fedora 26 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 26. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '26'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version'
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not
able to fix it before Fedora 26 is end of life. If you would still like
to see this bug fixed and are able to reproduce it against a later version
of Fedora, you are encouraged  change the 'version' to a later Fedora
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's
lifetime, sometimes those efforts are overtaken by events. Often a
more recent Fedora release includes newer upstream software that fixes
bugs or makes them obsolete.