|
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
|
