[OpenJDK 2D-Dev] RFR: JDK-8204211: windows : handle potential C++ exception in GDIRenderer -was : RE: java2d coding using SAFE_SIZE_ARRAY_ALLOC / safe_Malloc
Baesken, Matthias
matthias.baesken at sap.com
Wed Jun 6 11:45:51 UTC 2018
Hi Sergey, thanks for checking .
@Christoph : copyright years updated in the cpp files .
@Phil , I checked the indentation it looked indeed strange in the udiff however in the cpp file itself it looks ok to me .
Thanks for the reviews and best regards, Matthias
> -----Original Message-----
> From: Phil Race [mailto:philip.race at oracle.com]
> Sent: Dienstag, 5. Juni 2018 23:41
> To: Sergey Bylokhov <Sergey.Bylokhov at oracle.com>; Baesken, Matthias
> <matthias.baesken at sap.com>; Langer, Christoph
> <christoph.langer 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
> SAFE_SIZE_ARRAY_ALLOC / safe_Malloc
>
>
> In that case Matthias is good to go after fixing the indentation.
>
> -phil.
>
> On 06/05/2018 01:37 PM, Sergey Bylokhov wrote:
> > 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.deskto
> p/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
> >>>>> 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.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
> >>>>>> 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.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
> >>>>>>> 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.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
> >>>>>>>> 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.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
> >>>>>>>>> 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.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
> >>>>>>>>>>> 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.baesken at 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
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>
> >>
> >
> >
More information about the 2d-dev
mailing list