[jdk8u-dev] RFR: 8335552: [8u] JDK-8303466 backport to 8u requires 3 ::Identity signature fixes

[Andrew John Hughes]

On Wed, 3 Jul 2024 14:13:27 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> Hi, here is a followup fix for #529, where @wkia has noted a problem with the `MaxLNode::Identity`, `MinLNode::Identity` and `ConvI2LNode::Identity` signatures: https://github.com/openjdk/jdk8u-dev/pull/529#discussion_r1661486052.
>> 
>> cbd483afa413f006c991f1b4d92c83463cc11baa (clean backport of openjdk/jdk9u at 8efb3ab7c1cf0164237c50d60c4cd564b3d6a897) incorporates `-Woverloaded-virtual` to the Hotspot build, and 85db200c448580e26361cee2e91b3e580a40c70c finally solves the issue.
>> 
>> [The GitHub Actions' Linux builds have failed for `cbd483a`](https://github.com/franferrax/jdk8u-dev/actions/runs/9766502841/job/26959655660#step:8:1044). I hope this ensures an issue like this one is caught next time.
>> 
>> <details><summary><strong>Expand to see the build log excerpt with the failure</strong></summary>
>> <pre>
>> /usr/bin/g++ -DLINUX -D_GNU_SOURCE -DAMD64 -DPRODUCT -I. -I/home/runner/work/jdk8u-dev/jdk8u-dev/jdk/hotspot/src/share/vm/prims -I/home/runner/work/jdk8u-dev/jdk8u-dev/jdk/hotspot/src/share/vm -I/home/runner/work/jdk8u-dev/jdk8u-dev/jdk/hotspot/src/share/vm/precompiled -I/home/runner/work/jdk8u-dev/jdk8u-dev/jdk/hotspot/src/cpu/x86/vm -I/home/runner/work/jdk8u-dev/jdk8u-dev/jdk/hotspot/src/os_cpu/linux_x86/vm -I/home/runner/work/jdk8u-dev/jdk8u-dev/jdk/hotspot/src/os/linux/vm -I/home/runner/work/jdk8u-dev/jdk8u-dev/jdk/hotspot/src/os/posix/vm -I../generated -DHOTSPOT_RELEASE_VERSION=""25.432-b00"" -DHOTSPOT_BUILD_TARGET=""product"" -DHOTSPOT_BUILD_USER=""runner"" -DHOTSPOT_LIB_ARCH="amd64" -DHOTSPOT_VM_DISTRO=""OpenJDK""  -DTARGET_OS_FAMILY_linux -DTARGET_ARCH_x86 -DTARGET_ARCH_MODEL_x86_64 -DTARGET_OS_ARCH_linux_x86 -DTARGET_OS_ARCH_MODEL_linux_x86_64 -DTARGET_COMPILER_gcc -DINCLUDE_JFR=1 -DCOMPILER2 -DCOMPILER1 -fPIC -fno-rtti -fno-exceptions -D_REENTRANT -fcheck-new -fvisibilit
 y=hidden -m64  -pipe -fno-strict-aliasing  -fno-omit-frame-pointer -O3  -g -DVM_LITTLE_ENDIAN -D_LP64=1 -Werror -Wpointer-arith -Wsign-compare -Wundef -Wunused-function -Wunused-value -Wformat=2 -Wreturn-type -Woverloaded-virtual   -fstack-protector  -fno-delete-null-pointer-checks -fno-lifetime-dse -std=gnu++98 -c -MMD -MP -MF ../generated/dependencies/precompiled.hpp.gch.d -fpch-deps -x c++-header /home/runner/work/jdk8u-dev/jdk8u-dev/jdk/hotspot/src/share/vm/precompiled/precompiled.hpp -o precompiled.hpp.gch 
>> In file included from /home/runner/work/jdk8u-dev/jdk8u-dev/jdk/hotspot/src/share/vm/precompiled/precompiled.hpp:262:
>> /home/runner/work/jdk8u-dev/jdk8u-dev/jdk/hot...
>
>> I just noticed that, given [cbd483a](https://github.com/openjdk/jdk8u-dev/commit/cbd483afa413f006c991f1b4d92c83463cc11baa), this is a clean backport of JDK 9's [JDK-8075511](https://bugs.openjdk.org/browse/JDK-8075511) ([openjdk/jdk9u at 8efb3ab](https://github.com/openjdk/jdk9u/commit/8efb3ab7c1cf0164237c50d60c4cd564b3d6a897)). For the sake of clarity, I will add the issue and apply for approval there too. Sorry @jerboaa for bothering you with one more approval.
> 
> @franferrax Hmm, thinking some more about this and seeing the warnings on the added issue. Please create a separate backport for it and then merge that into this one.

> @jerboaa:
> 
> > @franferrax Hmm, thinking some more about this and seeing the warnings on the added issue. Please create a separate backport for it and then merge that into this one.
> 
> How about it now? The PR is recognized as a backport, links both issues and has the title of the most relevant one.
> 
> @gnu-andrew:
> 
> > Yes, please separate the enhancement into a separate PR. Adding a new warning has a different potential impact to just fixing the bug introduced by JDK-8335472
> 
> @jerboaa was referring to the Skara tooling warning saying that the PR wasn't being recognized as a backport. With respecto to the GCC warning, it would have prevented [JDK-8335472](https://bugs.openjdk.org/browse/JDK-8335472). The addition of the warning has been there since the beginning of the PR, which has been reviewed and approved with that change.
> 
> What happened, is that later I realized that the change is in fact a clean backport of [JDK-8075511](https://bugs.openjdk.org/browse/JDK-8075511) and wanted to document that.
> 
> @jerboaa, @gnu-andrew: are you OK to continue in this way?

No. Please make [JDK-8075511](https://bugs.openjdk.org/browse/JDK-8075511) a separate PR and commit. I agree it is good to catch these kind of failures in future, but it is not a pre-requisite for fixing this bug. 8u is built on a wide range of compilers and isolating the warning change to an individual fix makes it easier to deal with any impact.

-------------

PR Comment: https://git.openjdk.org/jdk8u-dev/pull/532#issuecomment-2206591354