8360478: libjsig related tier3 jtreg tests fail when asan is configured

[MBaesken]

There are a number of :tier3 HS jtreg tests using libjsig. Unfortunately they clash with asan, so it should be avoided to run them if asan is configured.

Examples :
runtime/signal/TestSigalrm.java
runtime/signal/TestSigbus.java

They run into errors like this

 stdout: [];
 stderr: [==59482==ASan runtime does not come first in initial library list; you should either link runtime to your application or manually preload it with LD_PRELOAD.

One option would be to avoid running those tests when asan is enabled.
Another option is to avoid the so called 'link order check' .
https://github.com/google/sanitizers/wiki/addresssanitizerflags

verify_asan_link_order - Check position of ASan runtime in library list (needs to be disabled when other library has to be preloaded system-wide)

---

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

Warning

 ⚠️ Found leading lowercase letter in issue title for 8360478: libjsig related tier3 jtreg tests fail when asan is configured

Issue

• JDK-8360478: libjsig related tier3 jtreg tests fail when asan is configured (Bug - P4)

Reviewers

• David Holmes (@dholmes-ora - Reviewer)
• Magnus Ihse Bursie (@magicus - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25978

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

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

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

8360478: libjsig related tier3 jtreg tests fail when asan is configured

Reviewed-by: dholmes, ihse

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

• 12196ba: 8358624: ImmutableDescriptor violates equals/hashCode contract after deserialization
• a471fe9: 8360539: DTLS handshakes fails due to improper cookie validation logic
• 839cede: 8357289: Break down the String constructor into smaller methods
• ... and 31 more: https://git.openjdk.org/jdk/compare/1fa090524a7c3bb5f2c92fb0f7217b9277ade9d9...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.

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

[openjdk]

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

• build

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.

[MBaesken]

With this change, all HS :tier3 jtreg tests pass on my Linux ppc64le test machine .

[mlbridge]

Webrevs

• 02: Full - Incremental (e72b4791)
• 01: Full - Incremental (06efab18)
• 00: Full (099a2c58)

[dholmes-ora]

If this fixes the problem then it seems fine. I don't know why ASAN cares about the library order or the implications thereof.

[MBaesken]

Could maybe someone with MSVC give it a try? My MSVC installation does not like ASAN (in theory it should work there).

[magicus]

Looks fine to me.

[magicus]

I don't know why ASAN cares about the library order or the implications thereof.

My understanding is that it wants to be injected first, and if you do LD_PRELOAD then another library might interfere with asan, and thus they want to warn for that.

[MBaesken]

Thanks for the reviews !

/integrate

[openjdk]

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

• 3525a40: 8359596: Behavior change when both -Xlint:options and -Xlint:-options flags are given
• 712d866: 8360312: Serviceability Agent tests fail with JFR enabled due to unknown thread type JfrRecorderThread
• da7080f: 8359761: JDK 25 RDP1 L10n resource files update
• ... and 35 more: https://git.openjdk.org/jdk/compare/1fa090524a7c3bb5f2c92fb0f7217b9277ade9d9...master

Your commit was automatically rebased without conflicts.

[openjdk]

@MBaesken Pushed as commit a23de2e.

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

[tstuefe]

Saw this too late. I am unsure about this change, because it disables a rather necessary warning. ASAN interposes a number of APIs, e.g. malloc/free. Bad things can happen if that interposition does not cover the full JVM process.

E.g. you preload library1, which comes before ASAN interposition is in place, so it uses libc malloc. You then load ASAN, then the rest of JVM. Anything loaded after ASAN will route through ASAN's version of malloc/free. So different parts of the process could use different versions of malloc/free. That could be trouble since we don't know whether pointers returned from raw libc malloc are compatible with ASAN's version of free, and vice versa. Even if nothing like that happens, it would quietly disable ASAN for a part of the libjvm, e.g. in case of libjsig for the libjsig.

Unless we fully understand what would happen in these cases, I would not switch off these warning for the full process. Rather just switch off tests on ASAN builds that rely on LD_PRELOAD. We don't have that many (should only be libjsig, no?)

This is just my 5 cents. I think ASAN is really useful, and the bugs @MBaesken already found proves that.

[MBaesken]

We could also flag the tests with @requires "!vm.asan" , that would be less intrusive but needs to be done per test .

Probably in practise both approaches work, for production we do not use ASAN anyways.

[tstuefe]

We could also flag the tests with @requires "!vm.asan" , that would be less intrusive but needs to be done per test .

I'd be totally fine with that

Probably in practise both approaches work, for production we do not use ASAN anyways.

Certainly.