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

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Tue Jun 5 20:37:43 UTC 2018

I have checked the fix using mach5.

On 05/06/2018 12:45, Phil Race wrote:

Oh .. can I please ask that you make sure that VS2017 is OK with the re-enabled warning ? I seriously doubt that it has anything new to add over VS2013, but a jdk-submit will tell you if it has ..

VS2017 is now the default so a jdk-submit will use that. -phil. On 06/05/2018 12:43 PM, Phil Race wrote: This looks good to me except for what looks like in my browser like missing indentation in http://cr.openjdk.java.net/~mbaesken/webrevs/8204211.1/src/java.desktop/windows/native/libawt/java2d/windows

/GDIRenderer.cpp.udiff.html You can fix that with or without an updated webrev. Good to clear a class of warnings. -phil. On 06/05/2018 12:47 AM, Baesken, Matthias wrote: 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.baesken at sap.com>; Thomas Stüfe <thomas.stuefe at gmail.com>; 'build-dev at openjdk.java.net' <build-_ _dev at openjdk.java.net>; awt-dev at openjdk.java.net Cc: 2d-dev <2d-dev at openjdk.java.net> Subject: RE: RFR: JDK-8204211: windows : handle potential C++ exception in GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using SAFESIZEARRAYALLOC / safeMalloc 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.stuefe at gmail.com>; 'build- dev at openjdk.java.net' <build-dev at openjdk.java.net>; awt- dev at openjdk.java.net 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 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.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

-- Best regards, Sergey.

More information about the build-dev mailing list