JDK-8327971: Multiple ASAN errors reported for metaspace

[tstuefe]

We have multiple issues that cause the ASAN build in fastdebug to not work, or to fail gtests.

1. When adding ASAN support for metaspace, we decided to track poisoning not at the OS border (map) but at the border between metaspace and outer JVM code. More precisely, at the border of ChunkManager chunk allocation/deallocation. Chunks handed out by the chunk manager are unpoisoned and poisoned when handed back.

This causes some problems since within metaspace, we access memory that is committed but not yet handed to the user (for zapping or for verification in debug builds). So, from the ASANs' view, those accesses are invalid, whereas for Metaspace, those accesses are perfectly valid.

In these cases, we need to unpoison the memory or exclude the code section, if possible, for ASAN builds. This affects both JVM code and gtests.

2. NMT gtests sometimes do dangerous things deliberately (accessing invalid memory to track error recognition), and these tests should be disabled with ASAN

/issue JDK-8327988

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issues

• JDK-8327971: Multiple ASAN errors reported for metaspace (Bug - P4)
• JDK-8327988: When running ASAN, disable dangerous NMT test (Bug - P4)

Reviewers

• Johan Sjölen (@jdksjolen - Reviewer) ⚠️ Review applies to 61e5a602
• Roman Kennke (@rkennke - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18230

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18230.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back stuefe! 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]

@tstuefe The following label will be automatically applied to this pull request:

• hotspot-runtime

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 06: Full - Incremental (c125b7e8)
• 05: Full - Incremental (61e5a602)
• 04: Full - Incremental (d99e8fe5)
• 03: Full - Incremental (03b788a4)
• 02: Full - Incremental (fa39ba67)
• 01: Full - Incremental (88ec8414)
• 00: Full (db197d8d)

[tstuefe]

@lgxbslgx Okay, I moved the code around. Would appreciate for this PR to be done quickly, since there are follow up PRs and its really not that complex.

[lgxbslgx]

I just provided some thoughts or suggestions based on my understanding. If my understanding is not right or low, please ignore it. Thanks again for your guidance.

[tstuefe]

I just provided some thoughts or suggestions based on my understanding. If my understanding is not right or low, please ignore it. Thanks again for your guidance.

No problem. Thank you for your review.

[openjdk]

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

8327971: Multiple ASAN errors reported for metaspace
8327988: When running ASAN, disable dangerous NMT test

Reviewed-by: jsjolen, rkennke

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 master 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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[tstuefe]

friendly ping

@jcking maybe?

[openjdk]

@tstuefe
Adding additional issue to issue list: 8327988: When running ASAN, disable dangerous NMT test.

[tstuefe]

Ping...

[jdksjolen]

Hi,

It seems like we only disable code which is relevant for debug build diagnostics, tests, or code otherwise used for debugging (print_location). Considering this, and ASAN's role in finding bugs in production code, it seems reasonable to do these changes.

So I'm approving this.

[tstuefe]

Hi,

It seems like we only disable code which is relevant for debug build diagnostics, tests, or code otherwise used for debugging (print_location). Considering this, and ASAN's role in finding bugs in production code, it seems reasonable to do these changes.

So I'm approving this.

Many thanks, @jdksjolen!

[rkennke]

Looks good, just one small question.

[rkennke]

Looks good now, thanks!

[tstuefe]

Thanks, Roman!

/integrate

[openjdk]

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

• 8fc9097: 8315575: Retransform of record class with record component annotation fails with CFE
• 2725405: 8327991: Improve HttpClient documentation with regard to reclaiming resources
• db01fcd: 8328827: Convert java/awt/print/PrinterJob/PrinterDialogsModalityTest/PrinterDialogsModalityTest.html applet test to main
• 89e0889: 8328858: More runtime/stack tests fail intermittently on libgraal
• 153410f: 8328930: [AIX] remove pase related coding
• da8a095: 8328928: Serial: Use IsGCActiveMark instead of AutoModifyRestore in SerialHeap::do_collection
• 472fcb6: 8328508: Unify the signatures of the methods address_for_index() and index_for() in BOT implementations
• cc1800f: 8327998: Enable java/lang/ProcessBuilder/JspawnhelperProtocol.java on Mac

Your commit was automatically rebased without conflicts.

[openjdk]

@tstuefe Pushed as commit 9e566d7.

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