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

Phil Race philip.race at oracle.com
Tue Jun 5 21:41:22 UTC 2018 

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.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 build-dev mailing list