8333887: ubsan: unsafe.cpp:247:13: runtime error: store to null pointer of type 'volatile int' #837

luwang1103 · 2024-07-12T01:29:00Z

hi @shipilev ,

This pull request contains a backport of commit 0d3a3771 from the master repository.

The commit being backported was authored by Matthias Baesken on 13 Jun 2024.

Thanks！

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
JDK-8333887 needs maintainer approval

Issue

JDK-8333887: ubsan: unsafe.cpp:247:13: runtime error: store to null pointer of type 'volatile int' (Bug - P4 - Approved)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 837

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2024-07-12T01:29:39Z

👋 Welcome back luwang1103! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-07-12T01:29:46Z

@luwang1103 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:

8333887: ubsan: unsafe.cpp:247:13: runtime error: store to null pointer of type 'volatile int'

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 17 new commits pushed to the master branch:

4e95699: 8328273: sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java failed with java.rmi.server.ExportException: Port already in use
ed1106b: Merge
d6e8788: 8335960: [21u] Remove designator DEFAULT_PROMOTED_VERSION_PRE=ea for release 21.0.4
dafbd18: 8320548: Improved loop handling
e69045f: 8327413: Enhance compilation efficiency
23b8753: 8324559: Improve 2D image handling
32a9294: 8323390: Enhance mask blit functionality
9e0a9ce: 8323231: Improve array management
28773f3: 8320097: Improve Image transformations
6a682ce: 8325600: Better symbol storage
... and 7 more: https://git.openjdk.org/jdk21u-dev/compare/c8db6fe9536148aee152710bad72cd025393e477...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this 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 · 2024-07-12T01:30:05Z

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

mlbridge · 2024-07-12T01:34:14Z

Webrevs

shipilev · 2024-07-12T08:00:07Z

Why bots do not think this is clean? The indenting is not the same as in original patch? Fix up the indenting and force-push.

shipilev · 2024-07-12T09:06:47Z

Hm, where is copyright header update comes from? This is confusing...

To be extra clear, this PR should be exactly like the upstream change: openjdk/jdk@0d3a377, unless there is a good reason to deviate. The indentation is still not fixed, AFAICS. How do you do the backports? Picking up and applying the full changsets, or making the patch by hand?

luwang1103 · 2024-07-12T09:58:27Z

Hm, where is copyright header update comes from? This is confusing...

To be extra clear, this PR should be exactly like the upstream change: openjdk/jdk@0d3a377, unless there is a good reason to deviate. The indentation is still not fixed, AFAICS. How do you do the backports? Picking up and applying the full changsets, or making the patch by hand?

In the master repository, there are other modifications to unsafe.cpp. Other modifications do not belong to this bug. What should I do next? This is my first time submitting code, and there are many areas that I am currently studying. Thank you for your guidance.

shipilev · 2024-07-12T10:07:42Z

Other modifications do not belong to this bug. What should I do next?

Right, so indenting for surrounding code had changed a bit. This is fine, we can make this one into unclean backport.

To proceed, I think you need to:

Drop the copyright year change from this PR.
Match the indenting of the original changeset: #if block should start a column 0.

shipilev · 2024-07-12T10:46:22Z

Indenting is still not fixed! Please look at the original changeset:

openjdk/jdk@0d3a377#diff-acaab5083c410130643986ea5285d68578a4a7cac8c7405a1c33374671752c9dR245-R249

Compare with yours:

https://github.com/openjdk/jdk21u-dev/pull/837/files#diff-acaab5083c410130643986ea5285d68578a4a7cac8c7405a1c33374671752c9dR222-R22

The indenting should match.

openjdk · 2024-07-12T10:47:33Z

@luwang1103 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.

shipilev · 2024-07-12T10:53:41Z

Spot the difference, this is original change:

  // we use this method at some places for writing to 0 e.g. to cause a crash;
  // ubsan does not know that this is the desired behavior
#if defined(__clang__) || defined(__GNUC__)
__attribute__((no_sanitize("undefined")))
#endif

Current PR misses two spaces before // comments:

// we use this method at some places for writing to 0 e.g. to cause a crash;
// ubsan does not know that this is the desired behavior
#if defined(__clang__) || defined(__GNUC__)
__attribute__((no_sanitize("undefined")))
#endif

This might sound like odd thing to get right, but matching the indenting exactly would make your follow-up backport in the same place clean. Make sure you have the new hunks exactly the same.

openjdk · 2024-07-15T02:26:50Z

@luwang1103 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.

openjdk · 2024-07-15T02:27:06Z

⚠️ @luwang1103 This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

luwang1103 · 2024-07-15T07:29:57Z

/approval request The code applies cleanly. Low risk: It is a corner case which is not triggered by the current code. In order to avoid bugs in the future, it is good to backport it.

openjdk · 2024-07-15T07:31:37Z

@luwang1103
8333887: The approval request has been created successfully.

openjdk · 2024-07-18T06:50:14Z

⚠️ @luwang1103 the full name on your profile does not match the author name in this pull requests' HEAD commit. If this pull request gets integrated then the author name from this pull requests' HEAD commit will be used for the resulting commit. If you wish to push a new commit with a different author name, then please run the following commands in a local repository of your personal fork:

$ git checkout ubsan-third
$ git commit --author='Preferred Full Name <you@example.com>' --allow-empty -m 'Update full name'
$ git push

luwang1103 · 2024-07-18T07:05:07Z

/integrate

openjdk · 2024-07-18T07:06:15Z

@luwang1103
Your change (at version ec5c744) is now ready to be sponsored by a Committer.

shipilev · 2024-07-18T07:10:51Z

/sponsor

openjdk · 2024-07-18T07:12:34Z

Going to push as commit 9a02aa6.
Since your change was applied there have been 18 commits pushed to the master branch:

be519cd: 8327424: ProblemList serviceability/sa/TestJmapCore.java on all platforms with ZGC
4e95699: 8328273: sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java failed with java.rmi.server.ExportException: Port already in use
ed1106b: Merge
d6e8788: 8335960: [21u] Remove designator DEFAULT_PROMOTED_VERSION_PRE=ea for release 21.0.4
dafbd18: 8320548: Improved loop handling
e69045f: 8327413: Enhance compilation efficiency
23b8753: 8324559: Improve 2D image handling
32a9294: 8323390: Enhance mask blit functionality
9e0a9ce: 8323231: Improve array management
28773f3: 8320097: Improve Image transformations
... and 8 more: https://git.openjdk.org/jdk21u-dev/compare/c8db6fe9536148aee152710bad72cd025393e477...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-07-18T07:12:44Z

@shipilev @luwang1103 Pushed as commit 9a02aa6.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Backport 0d3a3771c3777d3dd1fec8dc8faed5fd02b06830

6794dd2

openjdk bot changed the title ~~Backport 0d3a3771c3777d3dd1fec8dc8faed5fd02b06830~~ 8333887: ubsan: unsafe.cpp:247:13: runtime error: store to null pointer of type 'volatile int' Jul 12, 2024

openjdk bot added backport rfr Pull request is ready for review labels Jul 12, 2024

luwang1103 force-pushed the ubsan-third branch from 7d4cfd1 to 6794dd2 Compare July 12, 2024 10:39

luwang1103 force-pushed the ubsan-third branch from 771502b to 6794dd2 Compare July 15, 2024 02:02

adjust indentation

ec5c744

openjdk bot added the clean label Jul 15, 2024

openjdk bot added the approval label Jul 15, 2024

openjdk bot added ready Pull request is ready to be integrated and removed approval labels Jul 18, 2024

openjdk bot added the sponsor Pull request is ready to be sponsored label Jul 18, 2024

openjdk bot added the integrated Pull request has been integrated label Jul 18, 2024

openjdk bot closed this Jul 18, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Jul 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8333887: ubsan: unsafe.cpp:247:13: runtime error: store to null pointer of type 'volatile int' #837

8333887: ubsan: unsafe.cpp:247:13: runtime error: store to null pointer of type 'volatile int' #837

luwang1103 commented Jul 12, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jul 12, 2024

openjdk bot commented Jul 12, 2024 •

edited

Loading

openjdk bot commented Jul 12, 2024

mlbridge bot commented Jul 12, 2024 •

edited

Loading

shipilev commented Jul 12, 2024 •

edited

Loading

shipilev commented Jul 12, 2024

luwang1103 commented Jul 12, 2024

shipilev commented Jul 12, 2024

shipilev commented Jul 12, 2024

openjdk bot commented Jul 12, 2024

shipilev commented Jul 12, 2024

openjdk bot commented Jul 15, 2024

openjdk bot commented Jul 15, 2024

luwang1103 commented Jul 15, 2024

openjdk bot commented Jul 15, 2024

openjdk bot commented Jul 18, 2024

luwang1103 commented Jul 18, 2024

openjdk bot commented Jul 18, 2024

shipilev commented Jul 18, 2024

openjdk bot commented Jul 18, 2024

openjdk bot commented Jul 18, 2024

8333887: ubsan: unsafe.cpp:247:13: runtime error: store to null pointer of type 'volatile int' #837

8333887: ubsan: unsafe.cpp:247:13: runtime error: store to null pointer of type 'volatile int' #837

Conversation

luwang1103 commented Jul 12, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewing

Webrev

bridgekeeper bot commented Jul 12, 2024

openjdk bot commented Jul 12, 2024 • edited Loading

openjdk bot commented Jul 12, 2024

mlbridge bot commented Jul 12, 2024 • edited Loading

Webrevs

shipilev commented Jul 12, 2024 • edited Loading

shipilev commented Jul 12, 2024

luwang1103 commented Jul 12, 2024

shipilev commented Jul 12, 2024

shipilev commented Jul 12, 2024

openjdk bot commented Jul 12, 2024

shipilev commented Jul 12, 2024

openjdk bot commented Jul 15, 2024

openjdk bot commented Jul 15, 2024

luwang1103 commented Jul 15, 2024

openjdk bot commented Jul 15, 2024

openjdk bot commented Jul 18, 2024

luwang1103 commented Jul 18, 2024

openjdk bot commented Jul 18, 2024

shipilev commented Jul 18, 2024

openjdk bot commented Jul 18, 2024

openjdk bot commented Jul 18, 2024

luwang1103 commented Jul 12, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Jul 12, 2024 •

edited

Loading

mlbridge bot commented Jul 12, 2024 •

edited

Loading

shipilev commented Jul 12, 2024 •

edited

Loading