RFR: JDK-8204211: windows : handle potential C++ exception in GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using SAFE_SIZE_ARRAY_ALLOC / safe_Malloc
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
>>>> 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 build-dev
mailing list