8329826: GCC 12 reports some compiler error when building jdk8 #479

kuenking · 2024-04-09T09:30:05Z

Env:
ldd (GNU libc) 2.38
gcc (GCC) 12.3.1

Test the PR patch;
Build release/fastdebug/slowdebug version pass

Reported issue : https://bugs.openjdk.org/browse/JDK-8329826

Progress

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

Issue

JDK-8329826: GCC 12 reports some compiler error when building jdk8 (Bug - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 479

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2024-04-09T09:30:27Z

Hi @kuenking, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user kuenking" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

openjdk · 2024-04-09T09:31:38Z

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

openjdk · 2024-04-11T07:40:35Z

@kuenking Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

mlbridge · 2024-04-11T07:43:54Z

Webrevs

gnu-andrew · 2024-04-15T23:56:26Z

The register issue is already being handled by the work ongoing in #357

I have not seen other failures. Are they specific to certain architectures?

We should not be introducing changes uniquely to 8u unless it is really necessary, so we need to look at this code in later JDKs.

kuenking · 2024-04-16T01:34:03Z

The register issue is already being handled by the work ongoing in #357

I have not seen other failures. Are they specific to certain architectures?

We should not be introducing changes uniquely to 8u unless it is really necessary, so we need to look at this code in later JDKs.

Thank you very much for your code review and also suggest.
I found this problem when using gcc12 under the aarch64 architecture. After referring to #357, I still encounter other problems. I may need to remove the changes in adlc.make, the others are still having build issues in gcc12.

In file included from /home//jdk8u-dev-master/hotspot/src/share/vm/runtime/orderAccess.inline.hpp:45,
                 from /home//jdk8u-dev-master/hotspot/src/os/linux/vm/os_linux.inline.hpp:29,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/utilities/histogram.hpp:32,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/runtime/mutex.hpp:30,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/classfile/classLoaderData.hpp:32,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/oops/typeArrayKlass.hpp:28,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/oops/typeArrayOop.hpp:29,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/oops/constantPool.hpp:32,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/oops/method.hpp:33,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/runtime/frame.hpp:28,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/code/codeBlob.hpp:30,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/code/codeCache.hpp:28,
                 from /home//jdk8u-dev-master/hotspot/src/cpu/aarch64/vm/assembler_aarch64.inline.hpp:32,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/asm/assembler.inline.hpp:46,
                 from /home//jdk8u-dev-master/hotspot/src/share/vm/precompiled/precompiled.hpp:30:
In static member function ‘static void* OrderAccess::load_ptr_acquire(const volatile void*)’,
    inlined from ‘nmethod* volatile Method::code() const’ at /home//jdk8u-dev-master/hotspot/src/share/vm/oops/method.hpp:454:127,
    inlined from ‘static bool CompileBroker::compilation_is_complete(methodHandle, int, int)’ at /home//jdk8u-dev-master/hotspot/src/share/vm/compiler/compileBroker.cpp:1463:37,
    inlined from ‘static bool CompileBroker::compilation_is_complete(methodHandle, int, int)’ at /home//jdk8u-dev-master/hotspot/src/share/vm/compiler/compileBroker.cpp:1448:6,
    inlined from ‘static nmethod* CompileBroker::compile_method(methodHandle, int, int, methodHandle, int, const char*, Thread*)’ at /home//jdk8u-dev-master/hotspot/src/share/vm/compiler/compileBroker.cpp:1357:34:
/home//jdk8u-dev-master/hotspot/src/os_cpu/linux_aarch64/vm/orderAccess_linux_aarch64.inline.hpp:78:28: error: ‘long unsigned int __atomic_load_8(const volatile void*, int)’ writing 8 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=]
   78 | { void* data; __atomic_load((void* const volatile *)p, &data, __ATOMIC_ACQUIRE); return data; }
      |               ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Done with /home//jdk8u-dev-master/hotspot/src/share/vm/compiler/compilerOracle.cpp

Even though I modified gcc.make to fix the '-Werror=stringop-overflow=' problem. I also get the following error :

/home/jdk8u-dev-master/hotspot/src/share/vm/opto/type.cpp:2556:71: error: ‘this’ pointer is null [-Werror=nonnull]
 2556 |           ciInstanceKlass* k = o->as_instance()->java_lang_Class_klass()->as_instance_klass();

About opto/type.cpp build error , which is not closely related to the architecture. so I refer to https://bugs.openjdk.org/browse/JDK-8301074. After analysis and evaluation, issue JDK-8301074 is not suitable for backporting to jdk8u. Therefore, the modification solution is for reference only.

I'll test it on x86 architecture, and I'll report back with the results.

mirabilos · 2024-04-16T14:32:39Z

hotspot/make/linux/makefiles/gcc.make

@@ -212,7 +212,7 @@ ifeq ($(USE_CLANG), true)
  WARNINGS_ARE_ERRORS += -Wno-return-type -Wno-empty-body
 endif

-WARNING_FLAGS = -Wpointer-arith -Wsign-compare -Wundef -Wunused-function -Wunused-value -Wformat=2 -Wreturn-type
+WARNING_FLAGS = -Wpointer-arith -Wsign-compare -Wundef -Wunused-function -Wunused-value -Wformat=2 -Wreturn-type -Wno-stringop-overflow


disabling the warning instead of fixing the code is not the proper way to deal with this

Thank you for your code review.
The same solution is used in later JDK versions. https://bugs.openjdk.org/browse/JDK-8320212 ;
So JDK8 can also use this solution.

No, rather the other way round: please fix that in later JDK versions as well.

I’m only the Debian packager for JDK 8, so I have no relationship to the other versions. This doesn’t make my critique invalid.

thanks. Ok, I'll try to fix it forward through the code;
Since I'm not sure how many errors there are yet, this may take a while. I'll try;

Hi mirabilos，
I looked up some information; reference: https://gcc.gnu.org/bugzilla/show_bug.cgi? ID=106297. I also found that the gcc12 in my test environment was not a pure version but a version after secondary development, which affected the misjudgment of the jdk8 compilation error.
I built and tested using https://ftp.gnu.org/gnu/gcc/. The -Werror=stringop-overflow= error does not exist, and hotspot/src/share/vm/gc_implementation/g1/concurrentMark.cpp does not need to be modified.
The verification is as follows:

Therefore, I think the changes in gcc.make and concurrentMark.cpp should be removed.
thanks;

mirabilos · 2024-04-16T14:33:04Z

hotspot/make/linux/makefiles/adlc.make

@@ -66,7 +66,7 @@ CXXFLAGS += -DASSERT

 # CFLAGS_WARN holds compiler options to suppress/enable warnings.
 # Compiler warnings are treated as errors
-CFLAGS_WARN = $(WARNINGS_ARE_ERRORS)
+CFLAGS_WARN = $(WARNINGS_ARE_ERRORS) -Wno-register


that’s probably okay to ignore

mirabilos · 2024-04-16T14:34:49Z

hotspot/src/share/vm/opto/type.cpp

+          if (o != NULL) {
+            ciInstanceKlass* k = o->as_instance()->java_lang_Class_klass()->as_instance_klass();
+            field = k->get_field_by_offset(_offset, true);
+          }


This is useless papering over, not a fix, and not even good because it makes the code less legible at no gain.

Better to convert the two asserts to actual checks which abort the control flow of the function if triggered.

(disclaimer: I am not a reviewer)
This proposed change doesn't really handle the case where o == NULL, although it makes the compiler happy. I propose something like:

assert(o != NULL, "must be constant"); if (o != NULL) { ciInstanceKlass* k = o->as_instance()->java_lang_Class_klass()->as_instance_klass(); ciField* field = k->get_field_by_offset(_offset, true); assert(field != NULL, "missing field"); BasicType basic_elem_type = field->layout_type(); _is_ptr_to_narrowoop = UseCompressedOops && (basic_elem_type == T_OBJECT || basic_elem_type == T_ARRAY); } else { _is_ptr_to_narrowoop = (?? I am not familiar enough with this code to put a suggestion here) }

No, don’t assert and if something. (The compiler may delete one of these two under Undefined Behaviour rules.)

I’ve not looked at the callers, but returning an error or throwing an exception if o == NULL would be best. Do not rely on assert.

This is a good suggestion, I will try to fix it, refer to https://github.com/openjdk/jdk/blob/ad78b7fa67ba30cab2e8f496e4c765be15deeca6/src/hotspot/share/opto/type.cpp#L3560C3-L3572

thanks.

mlbridge · 2024-05-08T09:41:52Z

Mailing list message from Simon Tooke on jdk8u-dev:

mirabilos, we cannot see your comments until you either sign the OCA or
post them on this mailing list.

On 4/16/24 10:38 AM, mirabilos wrote:
--
Simon Tooke
Principal Software Engineer - Java Platform
Red Hat Canada, Inc.
stooke at redhat.com

mlbridge · 2024-05-08T09:41:56Z

Mailing list message from Thorsten Glaser on jdk8u-dev:

On Tue, 7 May 2024, Simon Tooke wrote:

mirabilos, we cannot see your comments until you either sign the OCA

Huh? Okay, I?ll have to find out what that is then?

or post them on this mailing list.

Funny: all posts on this mailing list regarding code issues have
been ignored so far?

bye,
//mirabilos
--
Infrastrukturexperte ? Qvest Digital AG
Am Dickobskreuz 10, D-53121 Bonn ? https://www.qvest-digital.com/
Telephon +49 228 54881-393 ? Fax: +49 228 54881-235
HRB AG Bonn 18196 ? USt-ID (VAT): DE274355441
Vorstand: Dr. Stefan Barth, Kai Ebenrett, Boris Esser, Alexander Steeg
Vorsitzender Aufsichtsrat: Peter N?then

mlbridge · 2024-05-08T09:41:57Z

Mailing list message from Simon Tooke on jdk8u-dev:

On 4/15/24 9:38 PM, Kun Wang wrote:

On Tue, 9 Apr 2024 09:30:05 GMT, Kun Wang <kwang at openjdk.org> wrote:

Env:
ldd (GNU libc) 2.38
gcc (GCC) 12.3.1

Test the PR patch;
Build release/fastdebug/slowdebug version pass

Reported issue : https://bugs.openjdk.org/browse/JDK-8329826
The `register` issue is already being handled by the work ongoing in #357

I have not seen other failures. Are they specific to certain architectures?

The issue in? hotspot/src/share/vm/opto/type.cpp:2556 (potential NULL
pointer) is common to all architectures.

I have see some other issues for GCC 14 (and the latest clang/Xcode),
which I will bring up in a separate PR.

--
Simon Tooke
Principal Software Engineer - Java Platform
Red Hat Canada, Inc.
stooke at redhat.com

…Klass

openjdk · 2024-05-10T01:11:26Z

@kuenking Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

bridgekeeper · 2024-06-07T04:37:50Z

@kuenking 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!

bridgekeeper · 2024-07-05T05:07:19Z

@kuenking This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

zzambers · 2024-08-19T10:27:27Z

Issue with register keyword has been fixed by:
#357

Issue in type.cpp seems like duplicate of these backports:
#536
#552
(which also include tests)

bridgekeeper bot added the oca Needs verification of OCA signatory status label Apr 9, 2024

bridgekeeper bot removed the oca Needs verification of OCA signatory status label Apr 9, 2024

8329826: GCC 12 reports some compiler error when building jdk8

a3b10aa

kuenking force-pushed the gcc12_build_error branch from 5e4af7d to a3b10aa Compare April 11, 2024 07:40

openjdk bot added the rfr Pull request is ready for review label Apr 11, 2024

kuenking closed this Apr 16, 2024

kuenking reopened this Apr 16, 2024

mirabilos suggested changes Apr 16, 2024

View reviewed changes

Kun Wang added 2 commits May 10, 2024 09:06

add the processing of the failure to obtain static fields by instance…

9f66d83

…Klass

revert gcc.make and cuncurrentMark.cpp

0d98297

kuenking force-pushed the gcc12_build_error branch from 0cab9c8 to 0d98297 Compare May 10, 2024 01:10

sendaoYan mentioned this pull request Jun 4, 2024

[riscv64-8u]error: ISO C++17 does not allow ‘register’ storage class specifier [-Werror=register] dragonwell-releng/issues-repo#30

Open

zzambers mentioned this pull request Jun 25, 2024

8281096: Flags introduced by configure script are not passed to ADLC build #357

Closed

4 tasks

bridgekeeper bot closed this Jul 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8329826: GCC 12 reports some compiler error when building jdk8 #479

8329826: GCC 12 reports some compiler error when building jdk8 #479

kuenking commented Apr 9, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Apr 9, 2024

openjdk bot commented Apr 9, 2024

openjdk bot commented Apr 11, 2024

mlbridge bot commented Apr 11, 2024 •

edited

Loading

gnu-andrew commented Apr 15, 2024

kuenking commented Apr 16, 2024 •

edited

Loading

mirabilos Apr 16, 2024 •

edited by bridgekeeper bot

Loading

kuenking May 8, 2024

mirabilos May 8, 2024

kuenking May 8, 2024

kuenking May 9, 2024

mirabilos Apr 16, 2024 •

edited by bridgekeeper bot

Loading

mirabilos Apr 16, 2024 •

edited by bridgekeeper bot

Loading

stooke May 7, 2024

mirabilos May 7, 2024

kuenking May 8, 2024

mlbridge bot commented May 8, 2024

mlbridge bot commented May 8, 2024

mlbridge bot commented May 8, 2024

openjdk bot commented May 10, 2024

bridgekeeper bot commented Jun 7, 2024

bridgekeeper bot commented Jul 5, 2024

zzambers commented Aug 19, 2024

8329826: GCC 12 reports some compiler error when building jdk8 #479

8329826: GCC 12 reports some compiler error when building jdk8 #479

Conversation

kuenking commented Apr 9, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewing

Webrev

bridgekeeper bot commented Apr 9, 2024

openjdk bot commented Apr 9, 2024

openjdk bot commented Apr 11, 2024

mlbridge bot commented Apr 11, 2024 • edited Loading

Webrevs

gnu-andrew commented Apr 15, 2024

kuenking commented Apr 16, 2024 • edited Loading

mirabilos Apr 16, 2024 • edited by bridgekeeper bot Loading

Choose a reason for hiding this comment

kuenking May 8, 2024

Choose a reason for hiding this comment

mirabilos May 8, 2024

Choose a reason for hiding this comment

kuenking May 8, 2024

Choose a reason for hiding this comment

kuenking May 9, 2024

Choose a reason for hiding this comment

mirabilos Apr 16, 2024 • edited by bridgekeeper bot Loading

Choose a reason for hiding this comment

mirabilos Apr 16, 2024 • edited by bridgekeeper bot Loading

Choose a reason for hiding this comment

stooke May 7, 2024

Choose a reason for hiding this comment

mirabilos May 7, 2024

Choose a reason for hiding this comment

kuenking May 8, 2024

Choose a reason for hiding this comment

mlbridge bot commented May 8, 2024

mlbridge bot commented May 8, 2024

mlbridge bot commented May 8, 2024

openjdk bot commented May 10, 2024

bridgekeeper bot commented Jun 7, 2024

bridgekeeper bot commented Jul 5, 2024

zzambers commented Aug 19, 2024

kuenking commented Apr 9, 2024 •

edited by openjdk bot

Loading

mlbridge bot commented Apr 11, 2024 •

edited

Loading

kuenking commented Apr 16, 2024 •

edited

Loading

mirabilos Apr 16, 2024 •

edited by bridgekeeper bot

Loading

mirabilos Apr 16, 2024 •

edited by bridgekeeper bot

Loading

mirabilos Apr 16, 2024 •

edited by bridgekeeper bot

Loading