Hi Serguei, Daniel,
Good to hear you like the fix.
My intention with the testing was to make sure my change didn't break
anything else. I didn't do a code paths check before I ran it though;
saturation run.
As for writing a new test, I'm finding it tricky.
Here's the current flow:
Step 1) VM initialises.
Step 2) VM loads a couple of libraries and shuts down if one or more
paths is too long in sun.boot.library.path.
Step 3) JDWP initializes
Step 4) JDWP loads a library and shuts down if one or more paths is
too long in sun.boot.library.path.
As you can see, Step 2 prevents us from reaching Step 4 with a
too-long-path (required to cause failure).
I worked around that with my webrev by disabling the bit in os.cpp
that enacts Step 2.
Since my hack will be removed in the final webrev, we need another
way to reach step 4.
So what we need to test this change, I believe, is a way to insert
Step 2.5) Change the property to include a too-long path.
This allows the VM to start up properly, but gives us the excessive
path we need to test the jdwp fix.
Right now, I'm not seeing a way to do this outside of using the JNI.
1) shell script launches cpp file.
2) cpp starts vm without jdwp.
3) change the property.
4) call jdwp library-loading method directly.
5) check the return code.
This seems messy, but I'm not seeing a way to initialise jdwp from
inside java code (which sounds better to me).
I welcome anyone who can think of a better way to do this.
Best Regards
Adam Farley
IBM Runtimes
"[email protected]" <[email protected]> wrote on
15/08/2019 09:25:36:
> From: "[email protected]" <[email protected]>
> To: Adam Farley8 <[email protected]>
> Cc: Chris Plummer <[email protected]>,
> [email protected], [email protected]
> Date: 15/08/2019 09:25
> Subject: Re: RFR: 8229378: jdwp library loader in linker_md.c
> quietly truncates on buffer overflow
>
> Hi Adam,
>
> The fix itself looks Okay to me.
> I'm not sure there is any test case in these test suites which
> provide a coverage for it.
> It looks like you need to develop a unit jtreg unit test for this.
>
> Thanks,
> Serguei
>
>
> On 8/13/19 09:28, Adam Farley8 wrote:
> Hi Serguei, Daniel,
>
> My testing was limited to the bug specific test case I mentioned,
> and the following jdwp tests:
>
> test/jdk/com/sun/jdi/Jdwp*
> test/hotspot/jtreg/serviceability/jdwp
>
> Best Regards
>
> Adam Farley
> IBM Runtimes
>
>
> "[email protected]" <[email protected]> wrote on
> 13/08/2019 17:04:43:
>
> > From: "[email protected]" <[email protected]>
> > To: [email protected], Adam Farley8
> > <[email protected]>, Chris Plummer <[email protected]>
> > Cc: [email protected]
> > Date: 13/08/2019 17:08
> > Subject: Re: RFR: 8229378: jdwp library loader in linker_md.c
> > quietly truncates on buffer overflow
> >
> > Hi Adam,
> >
> > I'm looking at your fix.
> > Also interested about your testing.
> >
> > Thanks,
> > Serguei
> >
> > On 8/13/19 08:48, Daniel D. Daugherty wrote:
> > I don't see any information about how this change was tested...
> > Is there something on another email thread?
> >
> > Dan
> >
>
> > On 8/13/19 11:41 AM, Adam Farley8 wrote:
> > Hi Chris,
> >
> > Thanks!
> >
> > I understand we need a second reviewer/sponsor to get this change
> > in. Any volunteers?
> >
> > Best Regards
> >
> > Adam Farley
> > IBM Runtimes
> >
> >
> > Chris Plummer <[email protected]> wrote on 12/08/2019
21:35:06:
> >
> > > From: Chris Plummer <[email protected]>
> > > To: Adam Farley8 <[email protected]>, serviceability-
> > [email protected]
> > > Date: 12/08/2019 21:35
> > > Subject: Re: RFR: 8229378: jdwp library loader in linker_md.c
> > > quietly truncates on buffer overflow
> > >
> > > Hi Adam,
> > >
> > > It looks good to me.
> > >
> > > thanks,
> > >
> > > Chris
> > >
> > > On 8/12/19 7:34 AM, Adam Farley8 wrote:
> > > Hi All,
> > >
> > > This is a known bug, mentioned in a code comment.
> > >
> > > Here is the fix for that bug.
> > >
> > > Reviewers and sponsors requested.
> > >
> > > Short version: if you set sun.boot.library.path to
> > > something beyond a system's max path length, the
> > > current code will return an empty string (rather than
> > > printing a useful error message and shutting down).
> > >
> > > This is also a problem if you've specified multiple
> > > paths with a separator, as this code seems to wrongly
> > > assess whether the *total* length exceeds max path
> > > length. So two 200 char paths on windows will cause
> > > failure, as the total length is 400 (which is beyond
> > > max length for windows).
> > >
> > > Note that the os.cpp bit of the webrev will not be included
> > > in the final webrev, it just makes this change trivially
> > > testable.
> > >
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8229378
> > > Webrev: http://cr.openjdk.java.net/~afarley/8229378/webrev/
> > >
> > >
> > > Best Regards
> > >
> > > Adam Farley
> > > IBM Runtimes
> > >
> > > Unless stated otherwise above:
> > > IBM United Kingdom Limited - Registered in England and Wales with
> > > number 741598.
> > > Registered office: PO Box 41, North Harbour, Portsmouth,
Hampshire PO6 3AU
> > Unless stated otherwise above:
> > IBM United Kingdom Limited - Registered in England and Wales with
> > number 741598.
> > Registered office: PO Box 41, North Harbour, Portsmouth,
Hampshire PO6 3AU
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with
> number 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
PO6 3AU
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with
number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
PO6 3AU
