Added Dan and Chris to the CC-list.
Thanks,
Serguei
On 8/26/19 00:27, [email protected] wrote:
Hi Adam,
Thank you for the explanation below!
Then I'm Okay with the fix as it is.
Dan,
Do you have any suggestions or objections?
If not, then do I need to add your name to the list of
reviewers?
Thanks,
Serguei
On 8/15/19 04:38, Adam Farley8 wrote:
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
|
