8348597: Update HarfBuzz to 10.4.0 #3026

vieiro · 2025-04-25T08:44:47Z

Backport of JDK-8348597 from JDK17 that updates HarfBuzz to 10.4.0 (which improves drawing performande by 10+% and includes different build fixes). Backport is not clean because it required adapting make/lib/Awt2dLibraries.gmk to fit JDK11.

Even though the backport built correctly, the resulting jdk/lib/libfontmanager.so contained undefined symbols that caused Swing tests to fail with UnsatisfiedLinkError. This is because JDK11 is missing JDK-8319197 that excludes hb-subset from compilation, so this is also included as the second commit in the pull request.

While at it, a third commit adds an additional -Wno-attributes flag to HARFBUZZ_DISABLED_WARNINGS_CXX_gcc, restoring the capability to build JDK11 on Linux with gcc-4.8.5 (this is an old version of gcc, dating from 2015, but it's still the system gcc version in RHEL-7).

Tested

on Windows 10 with jdk:jfc_demo.
on Linux with jdk:jfc_demo, with
- gcc 4.8.5/harfbuzz-1.7.5 (x86_64 & s390x),
- gcc 8.5.0/harfbuzz-1.7.5 (x86_64),
- gcc 11.5.0/harfbuzz-2.7.4 (x86_64)
- and gcc 14.2.1/harfbuzz-9.0.0 (x86_64)

with both --with-harfbuzz=system and --with-harfbuzz=bundled.

Progress

Change must not contain extraneous whitespace
JDK-8348597 needs maintainer approval
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 2 Reviewers)
JDK-8319197 needs maintainer approval

Issues

JDK-8348597: Update HarfBuzz to 10.4.0 (Bug - P3 - Requested)
JDK-8319197: Exclude hb-subset and hb-style from compilation (Enhancement - P4 - Requested)

Reviewers

Severin Gehwolf (@jerboaa - Reviewer) 🔄 Re-review required (review applies to 25b84f6a)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev.git pull/3026/head:pull/3026
$ git checkout pull/3026

Update a local copy of the PR:
$ git checkout pull/3026
$ git pull https://git.openjdk.org/jdk11u-dev.git pull/3026/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3026

View PR using the GUI difftool:
$ git pr show -t 3026

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/3026.diff

Using Webrev

Link to Webrev Comment

vieiro · 2025-04-25T08:45:13Z

/issue add JDK-8334964

bridgekeeper · 2025-04-25T08:45:16Z

👋 Welcome back vieiro! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-04-25T08:46:03Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-04-25T08:46:53Z

@vieiro
Adding additional issue to issue list: 8334964: Exclude hb-subset and hb-style from compilation.

openjdk · 2025-04-25T08:50:40Z

This backport pull request has now been updated with issue from the original commit.

openjdk · 2025-04-25T08:52:09Z

This backport pull request has now been updated with issue from the original commit.

mlbridge · 2025-04-25T08:55:45Z

Webrevs

vieiro · 2025-04-25T09:53:44Z

Fixing issues:
/issue add JDK-8319197
/issue remove JDK-8334964

openjdk · 2025-04-25T09:55:32Z

@vieiro
Adding additional issue to issue list: 8319197: Exclude hb-subset and hb-style from compilation.

openjdk · 2025-04-25T09:56:32Z

@vieiro
Removing additional issue from issue list: 8334964.

jerboaa

A few comments that need clarifications. Thanks!

jerboaa · 2025-05-09T15:15:10Z

make/lib/Awt2dLibraries.gmk

+   # Early re-canonizing has to be disabled to workaround an internal XlC compiler error
+   # when building libharfbuzz
+   ifeq ($(call isTargetOs, aix), true)
+     HARFBUZZ_CFLAGS += -qdebug=necan
+   endif


This is actually a change from JDK-8258484 which is not in JDK 11u. Intentional? If yes, we should probably use /issue add JDK-8258484 to have it tracked.

Yep. this is intentional. Will add the issue.

Why are you including this? Have you built this on AIX?

jerboaa · 2025-05-09T15:29:05Z

make/lib/Awt2dLibraries.gmk

+   # maybe-uninitialized required for GCC 8 builds. Not required for GCC 9+.
+   # calloc-transposed-args required for GCC 14 builds. (fixed upstream in Harfbuzz 032c931e1c0cfb20f18e5acb8ba005775242bd92)
+   HARFBUZZ_DISABLED_WARNINGS_CXX_gcc := reorder delete-non-virtual-dtor strict-overflow \
+        maybe-uninitialized class-memaccess unused-result extra use-after-free noexcept-type \


This also adds use-after-free as a disabled warning for gcc. That is part of JDK-8286562 which we probably don't want to partially introduce here.

Not strictly required (gcc 14, gcc 4.8.5). Will remove, thanks!

jerboaa · 2025-05-09T15:33:14Z

make/lib/Awt2dLibraries.gmk

+   HARFBUZZ_DISABLED_WARNINGS_gcc := type-limits missing-field-initializers strict-aliasing \
+        array-bounds parentheses dangling-pointer


parentheses dangling-pointer are new here. danling-pointer seems to be part of the JDK 17 harfbuzz update to 10.4.0. parenthesis not. Is it needed?

Not strictly required (gcc 14, gcc 4.8.5). Will remove, thanks!

vieiro · 2025-05-12T08:26:41Z

/issue add JDK-8258484

openjdk · 2025-05-12T08:27:13Z

@vieiro
Adding additional issue to issue list: 8258484: AIX build fails in Harfbuzz with XLC 16.01.0000.0006.

openjdk · 2025-05-12T09:51:31Z

⚠️ @vieiro This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

vieiro · 2025-05-12T10:03:24Z

/approval request Please consider approving this backport from JDK17 that levels up HarfBuzz version with upper JDKs and improves drawing performance. This also allows building again on older gcc v4.8.5.

openjdk · 2025-05-12T10:04:31Z

@vieiro
8348597: The approval request has been created successfully.
8319197: The approval request has been created successfully.
8258484: The approval request has been created successfully.

gnu-andrew · 2025-05-13T16:55:48Z

Will take a look at this.
/reviewers 2 reviewer

openjdk · 2025-05-13T16:56:50Z

@gnu-andrew
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 2 Reviewers).

vieiro · 2025-05-13T19:25:12Z

While at it, let's add MacOS 12.6.1/arm64 (virtualized)/clang 13.1.6 to the list of tests.

gnu-andrew

The code changes look fine and match 17u.

I'm confused by what is going on in make/lib/Awt2dLibraries.gmk. There should be just two changes here according to the description:

The addition of dangling-pointer to the disabled warnings as in 8348597
The expansion of LIBFONTMANAGER_EXCLUDE_FILES from 8319197

While I would prefer 8319197 was handled separately - as it is independent of this change - it is simple enough that it can be included here.

What I don't get is why the block is being indented by one space, which is obscuring the changes made to the set of disabled warnings. I think the clang ones are also being reordered but it's hard to tell for sure. I imagine this is why the other additions Severin already caught slipped in as well.

I also don't think an AIX change should be included if you are not testing on AIX or there is at least some relation to this change.

Please fix Awt2dLibraries.gmk to just update the lines we actually want to change as documented in the PR summary without altering the indentation and adding the AIX fix. Thanks.

vieiro · 2025-05-20T09:38:16Z

/issue remove JDK-8258484

openjdk · 2025-05-20T09:38:58Z

@vieiro
Removing additional issue from issue list: 8258484.

vieiro · 2025-05-20T09:43:48Z

Thanks for the review!

I'm confused by what is going on in make/lib/Awt2dLibraries.gmk. There should be just two changes here according to the description:
* The addition of `dangling-pointer` to the disabled warnings as in 8348597

* The expansion of `LIBFONTMANAGER_EXCLUDE_FILES` from 8319197

The intention was to make Awt2dLibraries.gmk more similar to the one in JDK-17 (which has had significant changes during the years), with the intention to make future backports easier (at the cost of a more difficult review), hence the reordering and the indentation.

So I've re-arranged the changes to make them more similar to the original Awt2dLibraries.gmk in 11. Hope this makes it easier to review. Note the attributes is required for gcc.4.8.5.

Retested with:

gcc 4.8.5/harfbuzz-1.7.5 (x86_64),
and gcc 14.2.1/harfbuzz-9.0.0 (x86_64)

vieiro added 3 commits April 21, 2025 13:07

Backport ba3db307dceb41e48014451873fdf4db3816a537

7bf1bc4

Backport f4e99de33e35826ec1fb368d6ad96005d7bd5cc4

947a4d5

Additional "attributes" flag for Linux/GCC 4.8.5

8064a5d

openjdk bot added the rfr Pull request is ready for review label Apr 25, 2025

vieiro changed the title ~~8348597: Update HarfBuzz to 10.4.0~~ Backport ba3db307dceb41e48014451873fdf4db3816a537 Apr 25, 2025

openjdk bot changed the title ~~Backport ba3db307dceb41e48014451873fdf4db3816a537~~ 8319197: Exclude hb-subset and hb-style from compilation Apr 25, 2025

openjdk bot added the backport label Apr 25, 2025

vieiro changed the title ~~8319197: Exclude hb-subset and hb-style from compilation~~ Backport f4e99de33e35826ec1fb368d6ad96005d7bd5cc4 Apr 25, 2025

openjdk bot changed the title ~~Backport f4e99de33e35826ec1fb368d6ad96005d7bd5cc4~~ 8348597: Update HarfBuzz to 10.4.0 Apr 25, 2025

jerboaa reviewed May 9, 2025

View reviewed changes

Removed parentheses and use-after-free (tested with gcc 14 & gcc 4.8.5)

25b84f6

jerboaa approved these changes May 12, 2025

View reviewed changes

openjdk bot added the approval label May 12, 2025

gnu-andrew suggested changes May 13, 2025

View reviewed changes

Rearrange Awt2dLibraries.gmk as per review

c9f53f7

		HARFBUZZ_DISABLED_WARNINGS_gcc := type-limits missing-field-initializers strict-aliasing \
		array-bounds parentheses dangling-pointer

8348597: Update HarfBuzz to 10.4.0 #3026

Are you sure you want to change the base?

8348597: Update HarfBuzz to 10.4.0 #3026

Uh oh!

Conversation

vieiro commented Apr 25, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewing

Uh oh!

vieiro commented Apr 25, 2025

Uh oh!

bridgekeeper bot commented Apr 25, 2025

Uh oh!

openjdk bot commented Apr 25, 2025

Uh oh!

openjdk bot commented Apr 25, 2025

Uh oh!

openjdk bot commented Apr 25, 2025

Uh oh!

openjdk bot commented Apr 25, 2025

Uh oh!

mlbridge bot commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

vieiro commented Apr 25, 2025

Uh oh!

openjdk bot commented Apr 25, 2025

Uh oh!

openjdk bot commented Apr 25, 2025

Uh oh!

jerboaa left a comment

Choose a reason for hiding this comment

Uh oh!

jerboaa May 9, 2025

Choose a reason for hiding this comment

Uh oh!

vieiro May 12, 2025

Choose a reason for hiding this comment

Uh oh!

gnu-andrew May 13, 2025

Choose a reason for hiding this comment

Uh oh!

jerboaa May 9, 2025

Choose a reason for hiding this comment

Uh oh!

vieiro May 12, 2025

Choose a reason for hiding this comment

Uh oh!

jerboaa May 9, 2025

Choose a reason for hiding this comment

Uh oh!

vieiro May 12, 2025

Choose a reason for hiding this comment

Uh oh!

vieiro commented May 12, 2025

Uh oh!

openjdk bot commented May 12, 2025

Uh oh!

openjdk bot commented May 12, 2025

Uh oh!

vieiro commented May 12, 2025

Uh oh!

openjdk bot commented May 12, 2025

Uh oh!

gnu-andrew commented May 13, 2025

Uh oh!

openjdk bot commented May 13, 2025

Uh oh!

vieiro commented May 13, 2025

Uh oh!

gnu-andrew left a comment

Choose a reason for hiding this comment

Uh oh!

vieiro commented May 20, 2025

Uh oh!

openjdk bot commented May 20, 2025

vieiro commented Apr 25, 2025 •

edited by openjdk bot

Loading

mlbridge bot commented Apr 25, 2025 •

edited

Loading