8374917: [8u] C++ flags get passed to C compiles in the HotSpot build#741
8374917: [8u] C++ flags get passed to C compiles in the HotSpot build#741gnu-andrew wants to merge 1 commit intoopenjdk:masterfrom
Conversation
|
👋 Welcome back andrew! A progress list of the required criteria for merging this PR into |
|
@gnu-andrew This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be: 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 1 new commit pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
|
|
|
Thanks Severin. |
|
@gnu-andrew |
|
/approve yes |
|
@jerboaa |
|
/integrate |
|
@gnu-andrew Pushed as commit 002def2. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
In gcc, this has been throwing a warning for a long time:
With clang builds, this is instead an error.
This escalates the problem for newer MacOS builds which need the
-stdflag for the C++ code to compile. It also causes Linux+clang builds to fail.This fix does the minimum to get C++ flags out of the C code builds (the serviceability agent & jsig):
NO_DELETE_NULL_POINTER_CHECKS_CFLAG,NO_LIFETIME_DSE_CFLAGandCXXSTD_CXXFLAGare moved from
EXTRA_CFLAGStoEXTRA_CXXFLAGS. The last of these is essential to fix the problem described above. The others should be safe for the little C code in the HotSpot build;fno-lifetime-dseis a C++ flag anyway and the null pointer checks in question are comparisons ofthiswithNULLfrom what I can see (again C++ only). Both have been removed in 21u and later, with the required fixes being in the C1 & ADLC code (see JDK-8316893 and JDK-8245051)HOST_CFLAGSandHOST_CXXFLAGSused by the ADLC build.CFLAGSandLFLAGSinhotspot/make/<platform>/makefiles/adlc.makeare switched to useHOST_CXXFLAGSas this compiles C++ codeCFLAGSandLFLAGSinhotspot/make/<platform>/makefiles/vm.makeare switched to useEXTRA_CXXFLAGSas this compiles C++ codeCXXFLAGSare removed fromCC_COMPILEinhotspot/make/<platform>/makefiles/rules.make. We leaveCXX_COMPILEuntouched to reduce risk.CXXFLAGSis rarely used (though most of the code is C++) whileCFLAGSis used for most flags.EXTRA_CXXFLAGSandHOST_CXXFLAGSwere unused prior to this change. So the risk of changing these is low, while we leaveCFLAGSuntouched.We also move
NO_DELETE_NULL_POINTER_CHECKS_CFLAG,NO_LIFETIME_DSE_CFLAGandCXXSTD_CXXFLAGtohotspot-spec.gmk.infromspec.gmk.in. They are now no longer used by the JDK build (which instead adds them toCCXXFLAGS_JDKas can be seen in #740) and this makes it easier to check the values and their usage in a single generated file.I've built this locally on GNU/Linux with both gcc (8.5.0) and clang (20.1.8). This fix, along with #740 and a few additional warning removal flags, allowed the clang build to complete. I compared the flags in the logs with a previous build from the 8u repository and all desired flags are preserved as before.
Builders may need to adjust their compilation flags if they are relying on
--with-extra-cxxflagspassing flags to the C code part of the HotSpot build instead of--with-extra-cflags. This is likely worth a release note.Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/741/head:pull/741$ git checkout pull/741Update a local copy of the PR:
$ git checkout pull/741$ git pull https://git.openjdk.org/jdk8u-dev.git pull/741/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 741View PR using the GUI difftool:
$ git pr show -t 741Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/741.diff
Using Webrev
Link to Webrev Comment