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
