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

merge compiler warning fixes from jdk17u-dev #96

Closed
wants to merge 5 commits into from

Conversation

jankratochvil
Copy link
Contributor

@jankratochvil jankratochvil commented Jul 29, 2023

../../src/hotspot/share/gc/z/zReferenceProcessor.cpp: In member function 'oopDesc* ZReferenceProcessor::drop(oop, ReferenceType)':
../../src/hotspot/share/gc/z/zReferenceProcessor.cpp:270:22: error: '%s' directive argument is null [-Werror=format-overflow=]
  270 |   log_trace(gc, ref)("Dropped Reference: " PTR_FORMAT " (%s)", p2i(reference), reference_type_name(type));
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
commit 68456bb248378da2ada5ecb6de92302988115b7c
Author: Aleksey Shipilev <shade@openjdk.org>
Date:   Thu Jun 23 15:49:41 2022 +0000
    8288754: GCC 12 fails to build zReferenceProcessor.cpp
    Backport-of: 834d92dd72257ab5d8c6759028098ac0867c5752

../src/java.base/unix/native/libjli/java_md_common.c: In function 'Resolve':
../src/java.base/unix/native/libjli/java_md_common.c:125:43: error: '%s' directive output may be truncated writing up to 4095 bytes into a region of size between 2 and 4097 [-Werror=format-truncation=]
  125 |     JLI_Snprintf(name, sizeof(name), "%s%c%s", indir, FILE_SEPARATOR, cmd);
      |                                           ^~
In file included from ../src/java.base/unix/native/libjli/java_md.h:38,
                 from ../src/java.base/share/native/libjli/java.h:41,
                 from ../src/java.base/unix/native/libjli/java_md_common.c:26:
../src/java.base/share/native/libjli/jli_util.h:103:41: note: 'snprintf' output between 2 and 8192 bytes into a destination of size 4098
  103 | #define JLI_Snprintf                    snprintf
../src/java.base/unix/native/libjli/java_md_common.c:125:5: note: in expansion of macro 'JLI_Snprintf'
  125 |     JLI_Snprintf(name, sizeof(name), "%s%c%s", indir, FILE_SEPARATOR, cmd);
      |     ^~~~~~~~~~~~
cc1: all warnings being treated as errors
commit 46c1434d50f6f0fa371785496c4d37c0e192da16
Author: Yasumasa Suenaga <ysuenaga@openjdk.org>
Date:   Wed Jan 25 17:19:30 2023 +0000
    8286562: GCC 12 reports some compiler warnings
    Backport-of: 410a25d59a11b6a627bbb0a2c405c2c2be19f464

In function 'find_positions',
    inlined from 'find_file' at ../src/java.base/share/native/libjli/parse_manifest.c:364:9:
../src/java.base/share/native/libjli/parse_manifest.c:292:34: error: pointer 'endpos' used after 'free' [-Werror=use-after-free]
  292 |             pos = flen - (endpos - cp);
      |                          ~~~~~~~~^~~~~
../src/java.base/share/native/libjli/parse_manifest.c:291:13: note: call to 'free' here
  291 |             free(buffer);
      |             ^~~~~~~~~~~~
cc1: all warnings being treated as errors
commit 0056a6330bb2e806ea47da0c51af0ab095feb25d
Author: Yasumasa Suenaga <ysuenaga@openjdk.org>
Date:   Mon Jan 23 22:05:43 2023 +0000
    8286705: GCC 12 reports use-after-free potential bugs
    Backport-of: 0e4bece5b5143b8505496ea7430bbfa11e151aff

../src/java.base/share/native/libjli/java.c: In function 'TranslateApplicationArgs':
../src/java.base/share/native/libjli/java.c:1711:35: error: the comparison will always evaluate as 'false' for the pointer operand in 'arg + 2' must not be NULL [-Werror=address]
 1711 |             *nargv++ = ((arg + 2) == NULL) ? NULL : JLI_StringDup(arg + 2);
      |                                   ^~
cc1: all warnings being treated as errors
commit 651ba865c1afe2b29adc5b0ca428117200313912
Author: Dan Lutker <lutkerd@amazon.com>
Date:   Fri Jan 27 16:04:34 2023 +0000
    8286694: Incorrect argument processing in java launcher
    Backport-of: 26c7c92bc93f3eecf7ce69c69f1999ba879d1d60

In file included from /home/azul/azul/crac-git/src/hotspot/share/classfile/stackMapFrame.hpp:29,
                 from /home/azul/azul/crac-git/src/hotspot/share/classfile/stackMapTable.hpp:28,
                 from /home/azul/azul/crac-git/src/hotspot/share/classfile/verifier.cpp:30:
In member function 'void ClassVerifier::set_method_signatures_table(method_signatures_table_type*)',
    inlined from 'void ClassVerifier::verify_class(JavaThread*)' at /home/azul/azul/crac-git/src/hotspot/share/classfile/verifier.cpp:630:30:
/home/azul/azul/crac-git/src/hotspot/share/classfile/verifier.hpp:446:30: error: storing the address of local variable 'method_signatures_table' in '*this.ClassVerifier::_method_signatures_table' [-Werror=dangling-pointer=]
  446 |     _method_signatures_table = method_signatures_table;
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
/home/azul/azul/crac-git/src/hotspot/share/classfile/verifier.cpp: In member function 'void ClassVerifier::verify_class(JavaThread*)':
/home/azul/azul/crac-git/src/hotspot/share/classfile/verifier.cpp:629:32: note: 'method_signatures_table' declared here
  629 |   method_signatures_table_type method_signatures_table;
      |                                ^~~~~~~~~~~~~~~~~~~~~~~
/home/azul/azul/crac-git/src/hotspot/share/classfile/verifier.cpp:622:39: note: 'this' declared here
  622 | void ClassVerifier::verify_class(TRAPS) {
      |                                       ^
cc1plus: all warnings being treated as errors
commit 39a6b55847be24e85a9d0e9ddb423808e7d1d57f
Author: Aleksey Shipilev <shade@openjdk.org>
Date:   Tue May 23 11:57:15 2023 +0000
    8287854: Dangling reference in ClassVerifier::verify_class
    Backport-of: 3fa99844a69401f84677e7d512ffd937f7f16898

Disabling warnings can lead to bugs like #94.

One could rather do a full merge with 17u-dev but there were some conflicts so I am not sure if that is wanted.


Progress

  • Change must not contain extraneous whitespace

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/crac.git pull/96/head:pull/96
$ git checkout pull/96

Update a local copy of the PR:
$ git checkout pull/96
$ git pull https://git.openjdk.org/crac.git pull/96/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 96

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/crac/pull/96.diff

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Jul 29, 2023

👋 Welcome back jkratochvil! A progress list of the required criteria for merging this PR into crac 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 29, 2023

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

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

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

merge compiler warning fixes from jdk17u-dev

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 no new commits pushed to the crac branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential 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.

➡️ 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 added ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jul 29, 2023
@mlbridge
Copy link

mlbridge bot commented Jul 29, 2023

Webrevs

@jankratochvil
Copy link
Contributor Author

It may be irrelevant due to the crac-master effort.

@AntonKozlov
Copy link
Member

Probably it will be easier to merge the whole jdk17u-dev to the crac-17 branch. Need to say, I did experienced merge conflicts near altstack from #37. But once all CRaC development goes to another branch, I see no reasons not to merge jdk17u-dev to simplify developer's life.

@jankratochvil
Copy link
Contributor Author

Probably it will be easier to merge the whole jdk17u-dev to the crac-17 branch. Need to say, I did experienced merge conflicts near altstack from #37.

There are more conflicts than just #37.

But once all CRaC development goes to another branch, I see no reasons not to merge jdk17u-dev to simplify developer's life.

IIUC then this merge will be already pointless. Which it is now. I found out about the master merge only after this backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Pull request is ready to be integrated rfr Pull request is ready for review
5 participants