JDK-8211280 : JavaFX build fails on Linux with gcc8
  • Type: Bug
  • Component: javafx
  • Sub-Component: window-toolkit
  • Affected Version: 8,openjfx11,openjfx12
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: linux
  • Submitted: 2018-09-28
  • Updated: 2020-01-31
  • Resolved: 2018-12-19
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 Other
8u212Fixed openjfx11.0.2Fixed
The following compilation error occurs when using GCC 8 to compile JavaFX:

modules/javafx.graphics/src/main/native-glass/gtk/glass_window_ime.cpp:236:66: error: cast between incompatible function types from ���int (*)(XIM, XPointer, XPointer)��� {aka ���int (*)(_XIM*, char*, char*)���} to ���XIMProc��� {aka ���void (*)(_XIM*, char*, char*)���} [-Werror=cast-function-type]
         XIMCallback startCallback = {(XPointer) jview, (XIMProc) im_preedit_start};

+1 for the 11u-dev backport

I request a backport to 11-dev. The patch at http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/e4dc16090ddf applies clean to 11-dev.

Changeset: e4dc16090ddf Author: pbansal Date: 2018-12-19 12:08 +0530 URL: http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/e4dc16090ddf 8211280: JavaFX build fails on Linux with gcc8 Reviewed-by: kcr, mbilla, arajkumar


+1 (no need for a new webrev to remove the two spaces that Murali noticed)

+1 Nit: you can remove the space after the parameter gpointer in dnd_finish_callback and pixbufDestroyNotifyFunc.

1. static gboolean dnd_finish_callback(gpointer data) { (Note: This is valid only in C++) [~arajkumar] I guess you mean static gboolean dnd_finish_callback(gpointer) { 2. Regarding if (pixels != NULL) { , , can we check with nullptr instead of NULL ?

updated webrev: http://cr.openjdk.java.net/~pbansal/8211280/webrev.01/ I have built this on gcc8, gcc7.3 on Ubuntu 18.04 and gcc5.4 on Ubuntu 16.04.

>> 2. Regarding if (pixels != NULL) { , , can we check with nullptr instead of NULL ? I dont think C++11 specific features are allowed in non WebKit native code.

Other than what Murali and Arun noted, I have no other comments (for Arun's #3, I was just getting ready to post the same comment). Once you post an updated webrev I want to run a quick test on existing compilers.

#1 +static gboolean dnd_finish_callback(gpointer data) { + (void) data; Instead of making the parameter variable unused, better avoid giving a name to it. It is a common pattern we follow in WebKit. Something like below, static gboolean dnd_finish_callback(gpointer data) { (Note: This is valid only in C++) #2 +void pixbufDestroyNotifyFunc (guchar *pixels, gpointer data) { + (void) data; Declare it is static function. It is not used across the compilation unit. Also #1 applies for this case as well. #3 + g_free(pixels); + pixels = NULL; There is no point in assigning the pointer to NULL.

[~mbilla] 1. I will remove the space. I hope a new webrev is not required for this. 2. I have not casted a int to void pointer. I have just used type erasion, so compiler does not give cast warning. I have changed the type of im_preedit_start to (void*) and then to XIMProc type. In this way, we are able to pass the im_preedit_start as it is. Please let me know if more clarification is required. 3. We don't need to compile this on windows as it is gtk specific code and changes are not applicable for Windows.

[Review In Progress] 1. Need to remove extra space between pixbufDestroyNotifyFunc and "(". void pixbufDestroyNotifyFunc (guchar *pixels, gpointer data) { 2. Im not sure about casting an int to void pointer. What if i cast 64 bit integer to void pointer on 32 bit machine and we may have loss of information in this case.Did you compiled on windows 64 bit machine with this change?

[~kcr] [~mbilla] Please review the fix. webrev: http://cr.openjdk.java.net/~pbansal/8211280/webrev.00/ From gcc8, the "function-cast-type" error has been enabled by default. Due to which our builds are failing on gcc8. The fix fixes those function pointer cast errors. In glass_window_ime.cpp, I have not changed/corrected the function signature of im_preedit_start. It returns a "integer" though it does not comply with the XIMProc type which expects it to return void. But this is how its supposed to be. It is mentioned in code comments and I have gone through a lot of documents on x11 programming [links below] and I see the same thing in all. So I have not correct the signature of im_preedit_start and I have just suppressed the error here by casting it to (void *) first and then to XIMProc. For sake of experiment, I did change the signature of im_preedit_start to return void and comply with XIMProc, and ran full gradle test suit, all runs fine. but I think as this is mentioned everywhere, we should not change it and let it be as it is. https://learning.oreilly.com/library/view/xlib-programming-manual/9780596806187/ch11s08.html https://www.x.org/releases/X11R7.7/doc/libX11/libX11/libX11.pdf