[OpenJDK 2D-Dev] RFR: JDK-8204211: windows : handle potential C++ exception in GDIRenderer -was : RE: java2d coding using SAFE_SIZE_ARRAY_ALLOC / safe_Malloc
Phil Race
philip.race at oracle.com
Tue Jun 5 19:43:36 UTC 2018
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
>> 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