Skip to content

Conversation

@zzambers
Copy link
Contributor

@zzambers zzambers commented Sep 3, 2025

This backport explicitly sets c99 standard for jdk c sources on compilers, where appropriate. Motivation for this is, recent gcc switch to c23 as default, causing errors (jdk sources are not c23 compatible).

Main problem is, that c23 no longer supports non-prototype function declarations (see (3) here). Example errors:

/mnt/ramdisk/java-11-openjdk-11.0.28.0.6-1.portable.jdk.el.x86_64.tarxz/src/src/java.base/unix/native/libnet/DefaultProxySelector.c:389:16: error: too many arguments to function 'g_proxy_resolver_lookup'; expected 0, have 4
  389 |     proxies = (*g_proxy_resolver_lookup)(resolver, uri, NULL, &error);
      |               ~^~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~
/mnt/ramdisk/java-11-openjdk-11.0.28.0.6-1.portable.jdk.el.x86_64.tarxz/src/test/hotspot/jtreg/runtime/jsig/libTestJNI.c:45:22: error: assignment to 'void (*)(int,  siginfo_t *, void *)' from incompatible pointer type 'void (*)(void)' [-Wincompatible-pointer-types]
   45 |     act.sa_sigaction = (void (*)())sig_handler;
      |                      ^

There are more, if above are fixed. Non prototype function declarations are present even in newest jdk, but newer jdks have c standard set explicitly, so they do not cause errors there.

This should not increase minimum requirements for compiler toolchains needed to build jdk11. Jdk13, where this originates from, has same minimal compiler requirements as jdk11. ( gcc 4.8, clang 3.2, Xcode 8, Solaris Studio 12.4)


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • JDK-8224087 needs maintainer approval

Issue

  • JDK-8224087: Compile C code for at least C99 Standard compliance (Enhancement - P4 - Approved)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3087

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

Using diff file

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

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Sep 3, 2025

👋 Welcome back zzambers! 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
Copy link

openjdk bot commented Sep 3, 2025

@zzambers This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

8224087: Compile C code for at least C99 Standard compliance

Reviewed-by: andrew

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 4 new commits pushed to the master branch:

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@gnu-andrew) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot changed the title backport 03428d73fc3618202627bbe9d9880a63221c1811 8224087: Compile C code for at least C99 Standard compliance Sep 3, 2025
@openjdk
Copy link

openjdk bot commented Sep 3, 2025

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

@openjdk openjdk bot added backport Port of a pull request already in a different code base clean Identical backport; no merge resolution required approval Requires approval; will be removed when approval is received rfr Pull request is ready for review labels Sep 3, 2025
@mlbridge
Copy link

mlbridge bot commented Sep 3, 2025

Webrevs

Copy link
Member

@gnu-andrew gnu-andrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in two minds about this. Specifying the standard explicitly is the right thing to do (I added similar for C++ years back) but we could just make that C90 so as to not raise older compilers.

I think, given some compilers have been defaulting to C99 for some time anyway (and maybe even later), this should be safe and may avoid problems with future backports. Let's get it in early and ensure it is well tested before January.

What compilers have you tested this change with?

@zzambers
Copy link
Contributor Author

zzambers commented Sep 8, 2025

I was also thinking, whether c99 is ok here, but given that jdk13 has same minimum compiler versions, it should be safe.

Apart from testing in GHA. I tested these compiler versions (WORK):

  • gcc-15.2.1 (f42; original motivation for this backport)
  • gcc-4.8.5 (rhel-7)
  • clang-3.8.1 (debian stretch)

Minimum accepted versions are gcc-4.8 and clang 3.2, so I tried to get close to that, based on what is available in old rhel/debian, not building my own toolchain. (Debian Jessie has clang 3.5, but that one (clang) was segfaulting during the build even on master, and Wheezy has clang 3.0)
(builds needed to disable warnings as errors, but that was same for master (and warnings seem the same))

For msvc there should be no change in compiler flags by this backport.

When it comes to Solaris, I could not test build there, because I don't have that OS available to me. But given that some awt libraries were already compiled with c99 flags, compiler flags should work.

@zzambers
Copy link
Contributor Author

zzambers commented Sep 8, 2025

Build logs:
jdk-c99-logs.tar.gz

Copy link
Member

@gnu-andrew gnu-andrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. My main concern was less what is claimed to be supported and more what people are actively building on at present. In particular, I'm reassured that it still builds on RHEL 7 as that's the one of most importance to us and where I test each build promotion.

I think the Solaris change, while untested, should be safe as it essentially just widens the scope of the existing usage from awt to the whole library build. My concern was the new option.

I think this is good to go in and should then get plenty of testing before January's release.

@gnu-andrew
Copy link
Member

/approve yes

@openjdk
Copy link

openjdk bot commented Sep 18, 2025

@gnu-andrew
8224087: The approval request has been approved.

@openjdk openjdk bot added ready Pull request is ready to be integrated and removed approval Requires approval; will be removed when approval is received labels Sep 18, 2025
@vieiro
Copy link
Contributor

vieiro commented Sep 19, 2025

Confirmed this builds on rhel7 with gcc8.

@zzambers
Copy link
Contributor Author

/integrate

@openjdk openjdk bot added the sponsor Pull request is ready to be sponsored label Sep 22, 2025
@openjdk
Copy link

openjdk bot commented Sep 22, 2025

@zzambers
Your change (at version e7e9318) is now ready to be sponsored by a Committer.

@jerboaa
Copy link
Contributor

jerboaa commented Sep 22, 2025

/sponsor

@openjdk
Copy link

openjdk bot commented Sep 22, 2025

Going to push as commit 156d998.
Since your change was applied there have been 4 commits pushed to the master branch:

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Sep 22, 2025
@openjdk openjdk bot closed this Sep 22, 2025
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Sep 22, 2025
@openjdk
Copy link

openjdk bot commented Sep 22, 2025

@jerboaa @zzambers Pushed as commit 156d998.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Port of a pull request already in a different code base clean Identical backport; no merge resolution required integrated Pull request has been integrated

Development

Successfully merging this pull request may close these issues.

4 participants