RFR: JDK-8204211: windows : handle potential C++ exception in GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using SAFE_SIZE_ARRAY_ALLOC (original) (raw)

Baesken, Matthias matthias.baesken at sap.com
Mon Jun 4 14:19:48 UTC 2018

Hello, I prepared a second webrev.

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.stuefe at gmail.com> Cc: '2d-dev' <2d-dev at openjdk.java.net>; Langer, Christoph <christoph.langer at sap.com> Subject: RE: RFR: JDK-8204211: windows : handle potential C++ exception in GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using SAFESIZEARRAYALLOC / safeMalloc

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🔞03 2018 +0200 @@ -213,6 +213,7 @@ LIBAWTCFLAGS += -fgcse-after-reload endif _$(eval $(call SetupJdkLibrary, BUILDLIBAWT, _ _NAME := awt, _ _SRC := $(LIBAWTDIRS), _ @@ -224,7 +225,7 @@ _format-nonliteral parentheses, _ _DISABLEDWARNINGSclang := logical-op-parentheses extern-initializer, _ _DISABLEDWARNINGSsolstudio := EDECLARATIONINCODE, _ _- DISABLEDWARNINGSmicrosoft := 4297 4244 4267 4996, _ _+ DISABLEDWARNINGSmicrosoft := 4244 4267 4996, _ _ASFLAGS := $(LIBAWTASFLAGS), _ _LDFLAGS := (LDFLAGSJDKLIB)(LDFLAGSJDKLIB) (LDFLAGSJDKLIB)(call SETSHAREDLIBRARYORIGIN), _ _LDFLAGSmacosx := -L$(INSTALLLIBRARIESHERE), _ 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🔞03 2018 +0200 @@ -85,7 +85,13 @@ *pNpoints = outpoints; } if (outpoints > POLYTEMPSIZE) { - pPoints = (POINT *) SAFESIZEARRAYALLOC(safeMalloc, sizeof(POINT), outpoints); + try { + pPoints = (POINT *) SAFESIZEARRAYALLOC(safeMalloc, sizeof(POINT), outpoints); + } catch (const std::badalloc&) { + 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🔞03 2018 +0200_ _@@ -873,7 +873,12 @@_ _int numSizes = ::DeviceCapabilities(printerName, printerPort,_ _DCPAPERS, NULL, NULL);_ _if (numSizes > 0) { - LPTSTR papers = (LPTSTR)SAFESIZEARRAYALLOC(safeMalloc, numSizes, sizeof(WORD)); + LPTSTR papers; + try { + papers = (LPTSTR)SAFESIZEARRAYALLOC(safeMalloc, numSizes, sizeof(WORD)); + } catch (const std::badalloc&) { + papers = NULL; + } if (papers != NULL && ::DeviceCapabilities(printerName, printerPort, DCPAPERS, papers, NULL) != -1) {

> -----Original Message----- > From: Baesken, Matthias > Sent: Freitag, 1. Juni 2018 14:18 > To: 'Thomas Stüfe' <thomas.stuefe at gmail.com> > Cc: 2d-dev <2d-dev at openjdk.java.net>; Langer, Christoph > <christoph.langer at sap.com> > Subject: RE: RFR: JDK-8204211: windows : handle potential C++ exception in > GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using > SAFESIZEARRAYALLOC / safeMalloc > > 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::badalloc& (non-const) is caught . > > For example : > > _> alloc.h 170 } catch (std::badalloc&) { _ _> 177 } catch (std::badalloc&) { _ _> 200 } catch (std::badalloc&) { _ _> 206 } catch (std::badalloc&) { _ > > awtInputTextInfor.cpp 223 } catch (std::badalloc&) { > 247 } catch (std::badalloc&) { > 317 } catch (std::badalloc&) { > 372 } catch (std::badalloc&) { > 407 } catch (std::badalloc&) { > > awtDnDDT.cpp 203 } catch (std::badalloc&) { > 264 } catch (std::badalloc&) { > 305 } catch (std::badalloc&) { > 366 } catch (std::badalloc&) { > 582 } catch (std::badalloc&) { > 635 } catch (std::badalloc&) { > 653 } catch (std::badalloc&) { > 698 } catch (std::badalloc&) { > 739 } catch (std::badalloc&) { > > awtDesktop.cpp 148 } catch (std::badalloc&) { > > WPrinterJob.cpp 166 } catch (std::badalloc&) { > 345 } catch (std::badalloc&) { > 425 } catch (std::badalloc&) { > 488 } catch (std::badalloc&) { > 631 } catch (std::badalloc&) { > 709 } catch (std::badalloc&) { > _> awtole.h 158 } catch (std::badalloc&) {_ > > awtDesktopProperties.cpp 125 catch (std::badalloc&) { > 269 catch (std::badalloc&) { > 647 catch (std::badalloc&) { > 664 catch (std::badalloc&) { > 689 catch (std::badalloc&) { > > awtPrintDialog.cpp 225 } catch (std::badalloc&) { > > awtDataTransferer.cpp 310 } catch (std::badalloc&) { > 724 } catch (std::badalloc &) { > 792 } catch (std::badalloc &) { > > awtMenuItem.cpp 328 } catch (std::badalloc&) { > 348 } catch (std::badalloc&) { > 524 } catch (std::badalloc&) { > > ShellFolder2.cpp 1410 } catch (std::badalloc&) { > 1435 } catch (std::badalloc&) { > > ... > > Best regards, Matthias > > > > -----Original Message----- > > From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com] > > Sent: Freitag, 1. Juni 2018 12:02 > > To: Baesken, Matthias <matthias.baesken at sap.com> > > Cc: 2d-dev <2d-dev at openjdk.java.net>; Langer, Christoph > > <christoph.langer at sap.com> > > Subject: Re: RFR: JDK-8204211: windows : handle potential C++ exception in > > GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using > > SAFESIZEARRAYALLOC / safeMalloc > > > > 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::badalloc&) {} > > > > Fine otherwise. I do not need another webrev. > > > > Best Regards, Thomas > > > > > > > > > > > > On Fri, Jun 1, 2018 at 10:39 AM, Baesken, Matthias > > <matthias.baesken at 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.stuefe at gmail.com] > > >> Sent: Mittwoch, 30. Mai 2018 17:37 > > >> To: Baesken, Matthias <matthias.baesken at sap.com> > > >> Cc: 2d-dev <2d-dev at openjdk.java.net> > > >> Subject: Re: [OpenJDK 2D-Dev] java2d coding using > > >> SAFESIZEARRAYALLOC / safeMalloc > > >> > > >> 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 badalloc, just to be safe). > > >> > > >> Best Regards, Thomas > > >> > > >> On Wed, May 30, 2018 at 5:08 PM, Baesken, Matthias > > >> <matthias.baesken at sap.com> wrote: > > >> > Hello , there is still some java2d coding where > > SAFESIZEARRAYALLOC / > > >> > safeMalloc 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 *) SAFESIZEARRAYALLOC(safeMalloc, > > >> > 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 > > >> > > > >> > > > >> > > > >> >

More information about the build-dev mailing list