RFR(xs): 8216559: [JFR] Native libraries not correctly parsed from /proc/self/maps

[Severin Gehwolf]

On Fri, 2019-01-11 at 18:29 +0100, jesper.wilhelmsson at oracle.com wrote:
> Looks good and trivial enough :-)
> /Jesper

Thanks for the review, Jesper!

Cheers,
Severin

> 
> > On 11 Jan 2019, at 18:15, Severin Gehwolf <sgehwolf at redhat.com> wrote:
> > 
> > Hi,
> > 
> > On Fri, 2019-01-11 at 08:43 -0800, Markus Gronlund wrote:
> > > Hi Severin,
> > > 
> > > Looks good, thanks for fixing.
> > 
> > Thanks for the review, Markus!
> > 
> > Any other reviewers or is this trivial enough for me to push with 1
> > review?
> > 
> > Thanks,
> > Severin
> > 
> > > Markus
> > > 
> > > -----Original Message-----
> > > From: Severin Gehwolf <sgehwolf at redhat.com>
> > > Sent: den 11 januari 2019 14:35
> > > To: hotspot-dev <hotspot-dev at openjdk.java.net>;
> > > hotspot-jfr-dev at openjdk.java.net
> > > Subject: RFR(xs): 8216559: [JFR] Native libraries not correctly
> > > parsed from /proc/self/maps
> > > 
> > > Hi,
> > > 
> > > Could I please get a review of this tiny fix for native libraries
> > > parsing from /proc/<pid>/maps? It's a Linux-only change and seems to
> > > surface only on certain systems (with 3-digit device ids in /proc).
> > > The issue has been noticed since TestNativeLibrariesEvent.java was
> > > failing for me.
> > > 
> > > What's the problem? The Linux impl of os::get_loaded_modules_info
> > > reads /proc/self/maps and scans each line into variables via
> > > sscanf(). The format for the device pointer allowed up to 5
> > > characters. In my case it was 6 characters long. As a result, the
> > > last digit of the device id got interpreted as inode value and the
> > > inode value as name. Thus, the failing test was showing strange
> > > library names like "25847051".
> > > 
> > > According to the spec, major:minor device numbers may be up to 3-
> > > digits in length. Hence, I propose to change the format from '%5s' to
> > > '%7s': 3 digits for major/minor each, plus one for ':'.
> > > 
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8216559
> > > webrev:
> > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8216559/webrev.01/
> > > 
> > > Testing: jdk_jfr tests on Linux x86_64 including
> > > TestNativeLibrariesEvent.java
> > >         which now passes.
> > > 
> > > Thoughts?
> > > 
> > > Thanks,
> > > Severin
> > > 
> 
>