Hi Christoph, thank's for the review . Could I have a second one for example from the awt or build-dev reviewers ?
Best Regards, Matthias > -----Original Message----- > From: Langer, Christoph > Sent: Montag, 4. Juni 2018 16:49 > To: Baesken, Matthias <matthias.baes...@sap.com>; Thomas Stüfe > <thomas.stu...@gmail.com>; 'build-dev@openjdk.java.net' <build- > d...@openjdk.java.net>; awt-...@openjdk.java.net > Cc: 2d-dev <2d-...@openjdk.java.net> > Subject: RE: RFR: JDK-8204211: windows : handle potential C++ exception in > GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using > SAFE_SIZE_ARRAY_ALLOC / safe_Malloc > > Hi Matthias, > > looks good to me. > > Don't forget the Copyright years. > > Best regards > Christoph > > > -----Original Message----- > > From: Baesken, Matthias > > Sent: Montag, 4. Juni 2018 16:20 > > To: Thomas Stüfe <thomas.stu...@gmail.com>; 'build- > > d...@openjdk.java.net' <build-dev@openjdk.java.net>; awt- > > d...@openjdk.java.net > > Cc: 2d-dev <2d-...@openjdk.java.net>; Langer, Christoph > > <christoph.lan...@sap.com> > > Subject: RE: RFR: JDK-8204211: windows : handle potential C++ exception in > > GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using > > SAFE_SIZE_ARRAY_ALLOC / safe_Malloc > > > > Hello, I prepared a second webrev. > > > > - use now const-reference in the catch-statements as suggested by > Thomas > > - reenabled the cl warning showing the exception issues in > > make/lib/Awt2dLibraries.gmk > > - found a second place in WPrinterJob.cpp with similar issues after > > reenabling the warnings > > > > Please review : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8204211.1/ > > > > (cc-ing build-dev because of the makefile change, and > > src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp > > because of the awt change ) > > > > > > Thanks, Matthias > > > > > > > > > -----Original Message----- > > > From: Baesken, Matthias > > > Sent: Montag, 4. Juni 2018 09:24 > > > To: 'Thomas Stüfe' <thomas.stu...@gmail.com> > > > Cc: '2d-dev' <2d-...@openjdk.java.net>; Langer, Christoph > > > <christoph.lan...@sap.com> > > > Subject: RE: RFR: JDK-8204211: windows : handle potential C++ exception > in > > > GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using > > > SAFE_SIZE_ARRAY_ALLOC / safe_Malloc > > > > > > A small update - I found a second place in WPrinterJob.cpp where > > > the > > > exception handling is missing. After fixing both places I can reenable > > > warning 4297 in > > > Awt2dLibraries.gmk which has been disabled before , probably > because > > of > > > the warnings generated when the exceptions where not handled . > > > Should I update the change with the other file + makefile change ? > > > > > > Best regards, Matthias > > > > > > > > > >hg diff > > > diff -r 12fe57c319e1 make/lib/Awt2dLibraries.gmk > > > --- a/make/lib/Awt2dLibraries.gmk Tue Apr 10 11:02:09 2018 +0800 > > > +++ b/make/lib/Awt2dLibraries.gmk Mon Jun 04 09:18:03 2018 +0200 > > > @@ -213,6 +213,7 @@ > > > LIBAWT_CFLAGS += -fgcse-after-reload > > > endif > > > > > > > > > $(eval $(call SetupJdkLibrary, BUILD_LIBAWT, \ > > > NAME := awt, \ > > > SRC := $(LIBAWT_DIRS), \ > > > @@ -224,7 +225,7 @@ > > > format-nonliteral parentheses, \ > > > DISABLED_WARNINGS_clang := logical-op-parentheses extern- > initializer, > > \ > > > DISABLED_WARNINGS_solstudio := E_DECLARATION_IN_CODE, \ > > > - DISABLED_WARNINGS_microsoft := 4297 4244 4267 4996, \ > > > + DISABLED_WARNINGS_microsoft := 4244 4267 4996, \ > > > ASFLAGS := $(LIBAWT_ASFLAGS), \ > > > LDFLAGS := $(LDFLAGS_JDKLIB) $(call SET_SHARED_LIBRARY_ORIGIN), > \ > > > LDFLAGS_macosx := -L$(INSTALL_LIBRARIES_HERE), \ > > > diff -r 12fe57c319e1 > > > > > > src/java.desktop/windows/native/libawt/java2d/windows/GDIRenderer.cpp > > > --- > > > > > > a/src/java.desktop/windows/native/libawt/java2d/windows/GDIRenderer.c > > > pp Tue Apr 10 11:02:09 2018 +0800 > > > +++ > > > > > > b/src/java.desktop/windows/native/libawt/java2d/windows/GDIRenderer.c > > > pp Mon Jun 04 09:18:03 2018 +0200 > > > @@ -85,7 +85,13 @@ > > > *pNpoints = outpoints; > > > } > > > if (outpoints > POLYTEMPSIZE) { > > > - pPoints = (POINT *) SAFE_SIZE_ARRAY_ALLOC(safe_Malloc, > > > sizeof(POINT), outpoints); > > > + try { > > > + pPoints = (POINT *) SAFE_SIZE_ARRAY_ALLOC(safe_Malloc, > > > sizeof(POINT), outpoints); > > > + } catch (const std::bad_alloc&) { > > > + return NULL; > > > + } > > > } > > > BOOL isempty = fixend; > > > for (int i = 0; i < npoints; i++) { > > > diff -r 12fe57c319e1 > > > src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp > > > --- > a/src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp > > > Tue Apr 10 11:02:09 2018 +0800 > > > +++ > > b/src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp > > > Mon Jun 04 09:18:03 2018 +0200 > > > @@ -873,7 +873,12 @@ > > > int numSizes = ::DeviceCapabilities(printerName, printerPort, > > > DC_PAPERS, NULL, NULL); > > > if (numSizes > 0) { > > > - LPTSTR papers = (LPTSTR)SAFE_SIZE_ARRAY_ALLOC(safe_Malloc, > > > numSizes, sizeof(WORD)); > > > + LPTSTR papers; > > > + try { > > > + papers = (LPTSTR)SAFE_SIZE_ARRAY_ALLOC(safe_Malloc, > > numSizes, > > > sizeof(WORD)); > > > + } catch (const std::bad_alloc&) { > > > + papers = NULL; > > > + } > > > if (papers != NULL && > > > ::DeviceCapabilities(printerName, printerPort, > > > DC_PAPERS, papers, NULL) != -1) { > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > From: Baesken, Matthias > > > > Sent: Freitag, 1. Juni 2018 14:18 > > > > To: 'Thomas Stüfe' <thomas.stu...@gmail.com> > > > > Cc: 2d-dev <2d-...@openjdk.java.net>; Langer, Christoph > > > > <christoph.lan...@sap.com> > > > > Subject: RE: RFR: JDK-8204211: windows : handle potential C++ > exception > > in > > > > GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using > > > > SAFE_SIZE_ARRAY_ALLOC / safe_Malloc > > > > > > > > Hi Thomas , using the const-reference sounds like a good idea ( I > > > > just > > > > copied from other locations in the source code where (almost?) > always > > > > std::bad_alloc& (non-const) is caught . > > > > > > > > For example : > > > > > > > > > > > > alloc.h 170 } catch (std::bad_alloc&) { \ > > > > 177 } catch (std::bad_alloc&) { \ > > > > 200 } catch (std::bad_alloc&) { \ > > > > 206 } catch (std::bad_alloc&) { \ > > > > > > > > awt_InputTextInfor.cpp 223 } catch (std::bad_alloc&) { > > > > 247 } catch (std::bad_alloc&) { > > > > 317 } catch (std::bad_alloc&) { > > > > 372 } catch (std::bad_alloc&) { > > > > 407 } catch (std::bad_alloc&) { > > > > > > > > awt_DnDDT.cpp 203 } catch (std::bad_alloc&) { > > > > 264 } catch (std::bad_alloc&) { > > > > 305 } catch (std::bad_alloc&) { > > > > 366 } catch (std::bad_alloc&) { > > > > 582 } catch (std::bad_alloc&) { > > > > 635 } catch (std::bad_alloc&) { > > > > 653 } catch (std::bad_alloc&) { > > > > 698 } catch (std::bad_alloc&) { > > > > 739 } catch (std::bad_alloc&) { > > > > > > > > awt_Desktop.cpp 148 } catch (std::bad_alloc&) { > > > > > > > > WPrinterJob.cpp 166 } catch (std::bad_alloc&) { > > > > 345 } catch (std::bad_alloc&) { > > > > 425 } catch (std::bad_alloc&) { > > > > 488 } catch (std::bad_alloc&) { > > > > 631 } catch (std::bad_alloc&) { > > > > 709 } catch (std::bad_alloc&) { > > > > > > > > awt_ole.h 158 } catch (std::bad_alloc&) {\ > > > > > > > > awt_DesktopProperties.cpp 125 catch (std::bad_alloc&) { > > > > 269 catch (std::bad_alloc&) { > > > > 647 catch (std::bad_alloc&) { > > > > 664 catch (std::bad_alloc&) { > > > > 689 catch (std::bad_alloc&) { > > > > > > > > awt_PrintDialog.cpp 225 } catch (std::bad_alloc&) { > > > > > > > > awt_DataTransferer.cpp 310 } catch (std::bad_alloc&) { > > > > 724 } catch (std::bad_alloc &) { > > > > 792 } catch (std::bad_alloc &) { > > > > > > > > awt_MenuItem.cpp 328 } catch (std::bad_alloc&) { > > > > 348 } catch (std::bad_alloc&) { > > > > 524 } catch (std::bad_alloc&) { > > > > > > > > ShellFolder2.cpp 1410 } catch (std::bad_alloc&) { > > > > 1435 } catch (std::bad_alloc&) { > > > > > > > > ... > > > > > > > > Best regards, Matthias > > > > > > > > > > > > > -----Original Message----- > > > > > From: Thomas Stüfe [mailto:thomas.stu...@gmail.com] > > > > > Sent: Freitag, 1. Juni 2018 12:02 > > > > > To: Baesken, Matthias <matthias.baes...@sap.com> > > > > > Cc: 2d-dev <2d-...@openjdk.java.net>; Langer, Christoph > > > > > <christoph.lan...@sap.com> > > > > > Subject: Re: RFR: JDK-8204211: windows : handle potential C++ > > exception > > > in > > > > > GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using > > > > > SAFE_SIZE_ARRAY_ALLOC / safe_Malloc > > > > > > > > > > Hi Matthias, > > > > > > > > > > Please consider catching all exceptions, not just std::alloc: > > > > > > > > > > } catch (...) { return NULL; } > > > > > > > > > > and doing it at the exit extern "C" function, not somewhere > > > > > internally. Regardless of which exceptions get thrown around below > > you > > > > > and by whom, you are safe that way. > > > > > > > > > > However, if you want to keep your patch as it is, please catch at > > > > > least as const reference: > > > > > > > > > > } catch (const std::bad_alloc&) {} > > > > > > > > > > Fine otherwise. I do not need another webrev. > > > > > > > > > > Best Regards, Thomas > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Jun 1, 2018 at 10:39 AM, Baesken, Matthias > > > > > <matthias.baes...@sap.com> wrote: > > > > > > Hi Thomas , thanks for the feedback. > > > > > > I created a bug and change for the excpetion handling in > > > > GDIRenderer.cpp > > > > > . > > > > > > Please review . > > > > > > > > > > > > Thanks, Matthias > > > > > > > > > > > > Bug: > > > > > > > > > > > > https://bugs.openjdk.java.net/browse/JDK-8204211 > > > > > > > > > > > > JDK-8204211: windows : handle potential C++ exception in > > GDIRenderer > > > > > > > > > > > > > > > > > > Change : > > > > > > > > > > > > > > > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8204211/ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >> -----Original Message----- > > > > > >> From: Thomas Stüfe [mailto:thomas.stu...@gmail.com] > > > > > >> Sent: Mittwoch, 30. Mai 2018 17:37 > > > > > >> To: Baesken, Matthias <matthias.baes...@sap.com> > > > > > >> Cc: 2d-dev <2d-...@openjdk.java.net> > > > > > >> Subject: Re: [OpenJDK 2D-Dev] java2d coding using > > > > > >> SAFE_SIZE_ARRAY_ALLOC / safe_Malloc > > > > > >> > > > > > >> Letting c++ exceptions escape from extern "C" functions is UB and > > > may > > > > > >> (and probably will) crash the process. This should be fixed. > Approach > > > > > >> taken by JDK-8039394 is fine (I would probably catch every C++ > > > > > >> exception with catch(...), not just bad_alloc, just to be safe). > > > > > >> > > > > > >> Best Regards, Thomas > > > > > >> > > > > > >> On Wed, May 30, 2018 at 5:08 PM, Baesken, Matthias > > > > > >> <matthias.baes...@sap.com> wrote: > > > > > >> > Hello , there is still some java2d coding where > > > > > SAFE_SIZE_ARRAY_ALLOC / > > > > > >> > safe_Malloc is used and the (potentially occurring) > > > > > >> > exception > is > > > not > > > > > >> > handled . > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > This leads to CL warnings (when enabled ) like > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > " function assumed not to throw an exception but does ; The > > > function > > > > is > > > > > >> > extern "C" and /EHc was specified" > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Example : > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > > > java.desktop/windows/native/libawt/java2d/windows/GDIRenderer.cpp > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > static POINT *TransformPoly() > > > > > >> > > > > > > >> > ….. > > > > > >> > > > > > > >> > if (outpoints > POLYTEMPSIZE) { > > > > > >> > > > > > > >> > pPoints = (POINT *) SAFE_SIZE_ARRAY_ALLOC(safe_Malloc, > > > > > >> > sizeof(POINT), outpoints); > > > > > >> > > > > > > >> > } > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Should we add exception handling here ? > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Similar fixes were done in the change 8039394: Compiler > warnings > > > > > about > > > > > >> C++ > > > > > >> > exceptions in windows printing code > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > https://bugs.openjdk.java.net/browse/JDK-8039394 > > > > > >> > > > > > > >> > http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/823387e2bf42 > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Best regards, Matthias > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> >
