RFR: 8188836: Upgrade to Harfbuzz 1.7.1 in JDK 10

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8188836: Upgrade to Harfbuzz 1.7.1 in JDK 10

Phil Race
Bug: https://bugs.openjdk.java.net/browse/JDK-8188836
Webrev : http://cr.openjdk.java.net/~prr/8188836/

harfbuzz is the OpenType font layout engine in JDK 9
JDK 9 uses harfbuzz 1.5.1 and JDK 10 should update to the most recent
version.

This has been built on all supported platforms (and. FWIW the 32 bit
unsupported
ones as well) and as a consequence I had to make a few tweaks.
One for window and 3 others for Solaris

On Windows+ Visual Studio 2013
On Windows I had to make a JDK build change for this library to
disable Visual C++ error 4101 - unreferenced local variable.

This is apparently down to debug code whereby if HB_DEBUG is off still
plants code like

#define TRACE_CLOSURE(this) hb_no_trace_t<hb_void_t> trace HB_UNUSED

The Solaris changes are as follows :-

--

harfbuzz/hb-face.cc", line 493: Error: The operation "extern "C"
void(*)(void*) != void(*)(void*)" is illegal.

The line it did not like is
   if (face->destroy != _hb_face_for_data_closure_destroy)

The fix is to declare hb_face_for_data_closure_destroy as extern "C" at
line 119
in the same file if compiler is __SUNPRO_CC

---
hb-dsalgs.hh", line 80: Error: Badly formed expression.
hb-dsalgs.hh", line 82: Error: Use ";" to terminate declarations.
hb-dsalgs.hh", line 82: Error: "," expected instead of ")".
hb-dsalgs.hh", line 82: Error: Use ";" to terminate declarations.
hb-dsalgs.hh", line 82: Error: A declaration was expected instead of ",".
hb-dsalgs.hh", line 83: Error: "," expected instead of ")".
hb-dsalgs.hh", line 85: Error: Use ";" to terminate declarations.
hb-dsalgs.hh", line 85: Error: A declaration was expected instead of ",".
hb-dsalgs.hh", line 85: Error: a is not defined.
hb-dsalgs.hh", line 85: Error: w is not defined.
hb-dsalgs.hh", line 86: Error: A declaration was expected instead of "if".
hb-dsalgs.hh", line 86: Error: a is not defined.

It turns out that the SunStudio compiler does not support "restrict" so
this line was not parsed correctly :
static int sort_r_cmpswap(char *__restrict a, char *__restrict b, size_t w,

I changed it to
#ifndef __SUNPRO_CC
/* __restrict is same as restrict but better support on old machines */
static int sort_r_cmpswap(char *__restrict a, char *__restrict b, size_t w,
#else
static int sort_r_cmpswap(char *a, char *b, size_t w,
#endif

---

3) hb-string-array.hh", line 51: Error: An array cannot have zero size
unless you use the option -features=zla.

That turns out to be the union member str in this code :
#undef _S
   } st;
   char str[0];

Since you need 'str' just to get the offset of the beginning of the
array that is the other
member of this union and that is way bigger than the one extra byte I
decided it was
easiest and harmless to just change it to

   char str[1];
---

These issues have been reported upstream .. some variation of these
fixes should make it into an upstream release before we need to upgrade
again.

-phil.


Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8188836: Upgrade to Harfbuzz 1.7.1 in JDK 10

Sergey Bylokhov
An update to the new Harfbuzz looks fine.

On 30/11/2017 11:01, Phil Race wrote:

> Bug: https://bugs.openjdk.java.net/browse/JDK-8188836
> Webrev : http://cr.openjdk.java.net/~prr/8188836/
>
> harfbuzz is the OpenType font layout engine in JDK 9
> JDK 9 uses harfbuzz 1.5.1 and JDK 10 should update to the most recent
> version.
>
> This has been built on all supported platforms (and. FWIW the 32 bit
> unsupported
> ones as well) and as a consequence I had to make a few tweaks.
> One for window and 3 others for Solaris
>
> On Windows+ Visual Studio 2013
> On Windows I had to make a JDK build change for this library to
> disable Visual C++ error 4101 - unreferenced local variable.
>
> This is apparently down to debug code whereby if HB_DEBUG is off still
> plants code like
>
> #define TRACE_CLOSURE(this) hb_no_trace_t<hb_void_t> trace HB_UNUSED
>
> The Solaris changes are as follows :-
>
> --
>
> harfbuzz/hb-face.cc", line 493: Error: The operation "extern "C"
> void(*)(void*) != void(*)(void*)" is illegal.
>
> The line it did not like is
>    if (face->destroy != _hb_face_for_data_closure_destroy)
>
> The fix is to declare hb_face_for_data_closure_destroy as extern "C" at
> line 119
> in the same file if compiler is __SUNPRO_CC
>
> ---
> hb-dsalgs.hh", line 80: Error: Badly formed expression.
> hb-dsalgs.hh", line 82: Error: Use ";" to terminate declarations.
> hb-dsalgs.hh", line 82: Error: "," expected instead of ")".
> hb-dsalgs.hh", line 82: Error: Use ";" to terminate declarations.
> hb-dsalgs.hh", line 82: Error: A declaration was expected instead of ",".
> hb-dsalgs.hh", line 83: Error: "," expected instead of ")".
> hb-dsalgs.hh", line 85: Error: Use ";" to terminate declarations.
> hb-dsalgs.hh", line 85: Error: A declaration was expected instead of ",".
> hb-dsalgs.hh", line 85: Error: a is not defined.
> hb-dsalgs.hh", line 85: Error: w is not defined.
> hb-dsalgs.hh", line 86: Error: A declaration was expected instead of "if".
> hb-dsalgs.hh", line 86: Error: a is not defined.
>
> It turns out that the SunStudio compiler does not support "restrict" so
> this line was not parsed correctly :
> static int sort_r_cmpswap(char *__restrict a, char *__restrict b, size_t w,
>
> I changed it to
> #ifndef __SUNPRO_CC
> /* __restrict is same as restrict but better support on old machines */
> static int sort_r_cmpswap(char *__restrict a, char *__restrict b, size_t w,
> #else
> static int sort_r_cmpswap(char *a, char *b, size_t w,
> #endif
>
> ---
>
> 3) hb-string-array.hh", line 51: Error: An array cannot have zero size
> unless you use the option -features=zla.
>
> That turns out to be the union member str in this code :
> #undef _S
>    } st;
>    char str[0];
>
> Since you need 'str' just to get the offset of the beginning of the
> array that is the other
> member of this union and that is way bigger than the one extra byte I
> decided it was
> easiest and harmless to just change it to
>
>    char str[1];
> ---
>
> These issues have been reported upstream .. some variation of these
> fixes should make it into an upstream release before we need to upgrade
> again.
>
> -phil.
>
>


--
Best regards, Sergey.