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.baes...@sap.com>; Thomas Stüfe
<thomas.stu...@gmail.com>; 'build-dev@openjdk.java.net' <build-
d...@openjdk.java.net>; awt-...@openjdk.java.net
Cc: 2d-dev <2d-...@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.stu...@gmail.com>; 'build-
d...@openjdk.java.net' <build-dev@openjdk.java.net>; awt-
d...@openjdk.java.net
Cc: 2d-dev <2d-...@openjdk.java.net>; Langer, Christoph
<christoph.lan...@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.stu...@gmail.com>
Cc: '2d-dev' <2d-...@openjdk.java.net>; Langer, Christoph
<christoph.lan...@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.stu...@gmail.com>
Cc: 2d-dev <2d-...@openjdk.java.net>; Langer, Christoph
<christoph.lan...@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.stu...@gmail.com]
Sent: Freitag, 1. Juni 2018 12:02
To: Baesken, Matthias <matthias.baes...@sap.com>
Cc: 2d-dev <2d-...@openjdk.java.net>; Langer, Christoph
<christoph.lan...@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.baes...@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.stu...@gmail.com]
Sent: Mittwoch, 30. Mai 2018 17:37
To: Baesken, Matthias <matthias.baes...@sap.com>
Cc: 2d-dev <2d-...@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.baes...@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
