Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8299635: Hotspot update for deprecated sprintf in Xcode 14 #2844

Closed
wants to merge 2 commits into from

Conversation

vieiro
Copy link
Contributor

@vieiro vieiro commented Jul 9, 2024

Backport of JDK-8299635 as applied to JDK-17, in order to get building on macos-13/Xcode 14/clang 14.

Mostly clean but for these files:

src/hotspot/os/aix/os_aix.cpp
src/hotspot/os/linux/os_linux.cpp
src/hotspot/share/opto/cfgnode.cpp
src/hotspot/share/opto/chaitin.cpp
src/hotspot/share/opto/chaitin.hpp
src/hotspot/share/opto/type.cpp

In src/hotspot/share/opto/type.cpp we restored JDK-17's nullptr back to JDK-11's NULL.

After this PR the macos-builds are expected to continue failing with error messages similar to:

A) The -Wdeprecated-non-prototype flag. This seems JDK-11 specific as these files were later reorganized in next JDK versions. We may need a JDK-11 specific issue for this compilation failure.

Compiling main.c (for pack200)
/Users/runner/work/jdk11u-dev/jdk11u-dev/src/java.desktop/share/native/libawt/awt/medialib/awt_ImagingLib.c:922:48: error: passing arguments to a function without a prototype is deprecated in all versions of C and is not supported in C2x [-Werror,-Wdeprecated-non-prototype]
Creating support/modules_cmds/jdk.pack/unpack200 from 7 file(s)
    if ((status = (*sMlibFns[MLIB_AFFINE].fptr)(dst, src, mtx, filter,
Compiling bands.cpp (for unpack200)

B) Some missing sprintf in files in the src/jdk.pack directory, which is JDK-11 specific as Pack200 was removed in later versions.

/Applications/Xcode_14.3.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk/usr/include/stdio.h:188:1: note: 'sprintf' has been explicitly marked deprecated here
__deprecated_msg("This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.")
^
/Applications/Xcode_14.3.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk/usr/include/sys/cdefs.h:215:48: note: expanded from macro '__deprecated_msg'
        #define __deprecated_msg(_msg) __attribute__((__deprecated__(_msg)))
                                                      ^
/Users/runner/work/jdk11u-dev/jdk11u-dev/src/jdk.pack/share/native/common-unpack/bands.cpp:165:25: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
    if (le_back != 0) { sprintf(bp, "/back%d", le_back); bp += strlen(bp); }

Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • JDK-8299635 needs maintainer approval

Integration blocker

 ⚠️ Dependency #2842 must be integrated first

Issue

  • JDK-8299635: Hotspot update for deprecated sprintf in Xcode 14 (Enhancement - P4)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2844

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

Using diff file

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

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Jul 9, 2024

👋 Welcome back vieiro! A progress list of the required criteria for merging this PR into pr/2842 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 Jul 9, 2024

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

@openjdk openjdk bot changed the title Backport faf55ddad00559be71ba7f4b34f9df3d4f251d11 8299635: Hotspot update for deprecated sprintf in Xcode 14 Jul 9, 2024
@openjdk
Copy link

openjdk bot commented Jul 9, 2024

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

@openjdk openjdk bot added backport rfr Pull request is ready for review labels Jul 9, 2024
@mlbridge
Copy link

mlbridge bot commented Jul 9, 2024

Webrevs

@gdams
Copy link
Member

gdams commented Jul 16, 2024

@vieiro it looks like you should be able to apply the remaining changes to src/hotspot/os/linux/os_linux.cpp. Looking on Line 507 I still see the sprintf command which was replaced in https://github.com/openjdk/jdk17u-dev/pull/2663/files#diff-aeec57d804d56002f26a85359fc4ac8b48cfc249d57c656a30a63fc6bf3457adL488-R490

@gdams
Copy link
Member

gdams commented Jul 16, 2024

@vieiro it looks like you should be able to apply the remaining changes to src/hotspot/os/linux/os_linux.cpp. Looking on Line 507 I still see the sprintf command which was replaced in https://github.com/openjdk/jdk17u-dev/pull/2663/files#diff-aeec57d804d56002f26a85359fc4ac8b48cfc249d57c656a30a63fc6bf3457adL488-R490

Other than this, the rest of the changes LGTM

@vieiro
Copy link
Contributor Author

vieiro commented Jul 16, 2024

@gdams Thanks for the review!

Copy link
Member

@gdams gdams left a comment

Choose a reason for hiding this comment

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

LGTM - although I'm not a reviewer so this won't count in the backport process

@bridgekeeper
Copy link

bridgekeeper bot commented Aug 14, 2024

@vieiro This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@vieiro
Copy link
Contributor Author

vieiro commented Sep 5, 2024

Parent pull requests have been closed without review, so there's no point in continuing just with this PR.

@vieiro vieiro closed this Sep 5, 2024
@vieiro vieiro deleted the backports/JDK-8299635 branch September 5, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

2 participants