|
Hi Adam,
I'm Okay with this fix.
If nobody else have comments then I'll build it, test a little bit
and push to the jdk/jdk repo.
Thanks,
Serguei
On 9/9/19 09:53, Adam Farley8 wrote:
Hi
Serguei,
Apologies for
the delay.
The errors have
all been fixed, and the requested tests mostly passed, windows
and linux.
No test group
had more failures post-fix than pre-fix, so I'm calling that a
win.
The new webrev
can be found here:
http://cr.openjdk.java.net/~afarley/8229378.2/webrev
Best Regards
Adam Farley
IBM Runtimes
"[email protected]"
<[email protected]> wrote on 29/08/2019
19:38:02:
> From: "[email protected]"
<[email protected]>
> To: Adam Farley8 <[email protected]>
> Cc: Chris Plummer <[email protected]>,
> [email protected],
[email protected]
> Date: 29/08/2019 19:38
> Subject: Re: RFR: 8229378: jdwp
library loader in linker_md.c
> quietly truncates on buffer overflow
>
> Hi Adam,
>
> Okay, thanks!
> Serguei
>
>
> On 8/29/19 06:26, Adam Farley8 wrote:
> Hi Serguei,
>
> I haven't actually run a fastdebug build before. Will do
that now
> and address the issues.
>
> Once done, I'll re-run the tests I ran, and also the
tests you've
> listed below.
>
> Can you advise on how "good coverage" is determined, so
I know for
> future bug fixes?
>
> As for the up-to-date-ness, I'll update the build before
doing the
above.
>
> Expect a webrev once all of this is complete.
>
> Best Regards
>
> Adam Farley
> IBM Runtimes
>
>
> "[email protected]"
<[email protected]>
wrote on
> 29/08/2019 03:54:56:
>
> > From: "[email protected]"
<[email protected]>
> > To: Adam Farley8 <[email protected]>
> > Cc: Chris Plummer <[email protected]>,
> > [email protected],
[email protected]
> > Date: 29/08/2019 04:23
> > Subject: Re: RFR: 8229378: jdwp library loader in
linker_md.c
> > quietly truncates on buffer overflow
> >
> > Hi Adam,
> >
> > Sorry for the latency.
> > I was in process to build, test and push your fix
and got the
> > fastdebug build errors below.
> >
> > So, my question is if you've ever built the
fastdebug version.
> > This change is in the system-dependent code, so it
has to be
tested
> > on both Unix and Windows.
> >
> > > 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
> >
> > This set of tests does not provide a good coverage.
> > To make sure nothing is broken you need to run the
the test/jdk/com/sun/jdi
> > and also the following vmTestbase tests:
> >
> > test/hotspot/jtreg/vmTestbase/nsk/jdi
> > test/hotspot/jtreg/vmTestbase/nsk/jdb
> > test/hotspot/jtreg/vmTestbase/nsk/jdwp
> >
> > BTW, your current webrev is not up-to-date:
> > http://cr.openjdk.java.net/~afarley/8229378/webrev/
> >
> > I guess, the change in the
src/hotspot/share/runtime/os.cpp became
> > obsolete after your previous fix that was already
pushed.
> >
> > Thanks,
> > Serguei
> >
> > . . .
> > In file included from
/scratch/sspitsyn/jdk14.1/open/src/
> > jdk.jdwp.agent/unix/native/libjdwp/linker_md.c:37:0:
> >
/scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/unix/native/
> > libjdwp/linker_md.c: In function ‘dll_build_name’:
> >
/scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/share/native/
> > libjdwp/util.h:46:23: error: ‘Do’ undeclared (first
use in
this function)
> > #define strdup(p) Do not use this interface.
> >
^
> >
/scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/unix/native/
> > libjdwp/linker_md.c:51:18: note: in expansion of
macro ‘strdup’
> > paths_copy = strdup(paths);
> >
^
> >
/scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/share/native/
> > libjdwp/util.h:46:23: note: each undeclared
identifier is reported
> > only once for each function it appears in
> > #define strdup(p) Do not use this interface.
> >
^
> >
/scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/unix/native/
> > libjdwp/linker_md.c:51:18: note: in expansion of
macro ‘strdup’
> > paths_copy = strdup(paths);
> >
^
> >
/scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/share/native/
> > libjdwp/util.h:46:26: error: expected ‘;’ before
‘not’
> > #define strdup(p) Do not use this interface.
> >
^
> >
/scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/unix/native/
> > libjdwp/linker_md.c:51:18: note: in expansion of
macro ‘strdup’
> > paths_copy = strdup(paths);
> >
^
> >
/scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/share/native/
> > libjdwp/util.h:38:24: error: expected ‘;’ before
‘not’
> > #define free(p) Do not use this interface.
> >
^
> >
/scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/unix/native/
> > libjdwp/linker_md.c:71:5: note: in expansion of
macro ‘free’
> > free(paths_copy);
> > ^
> > gmake[3]: ***
[/scratch/sspitsyn/jdk14.1/build/linux-x86_64-server-
> >
fastdebug/support/native/jdk.jdwp.agent/libjdwp/linker_md.o]
Error 1
> > gmake[2]: *** [jdk.jdwp.agent-libs] Error 1
> > gmake[2]: *** Waiting for unfinished jobs....
> >
> > ERROR: Build failed for target 'images' in
configuration 'linux-
> > x86_64-server-fastdebug' (exit code 2)
> >
> >
> >
> > 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
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
|