[OpenJDK 2D-Dev] RFR: JDK-8204211: windows : handle potential C++ exception in GDIRenderer -was : RE: java2d coding using SAFE_SIZE_ARRAY_ALLOC / safe_Malloc
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Tue Jun 5 19:56:11 UTC 2018
Looks fine.
On 05/06/2018 09:05, Erik Joelsson wrote:
> Build change looks ok, but the validity of disabling the warning needs
> review from someone else.
>
> /Erik
>
>
> On 2018-06-05 00:47, 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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>
--
Best regards, Sergey.
More information about the 2d-dev
mailing list