8346914: UB issue in scalbnA

[kimbarrett]

Please review this change that replaces uses of our scalbnA function with
using the standard scalbn function. Removed scalbnA, and also copysignA.

For more details, see first comment and JBS.

Testing: mach5 tier1-6. Though from discussion in
#25656, it's hard to get to our uses of
scalbn/scalbnA.

Before removing it, I tested scalbnA via a gtest that is attached to the JBS
issue.

/label hotspot

---

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

Issue

• JDK-8346914: UB issue in scalbnA (Bug - P4)

Reviewers

• Andrew Haley (@theRealAph - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25917

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[kimbarrett]

Long ago we had our own scalbn and copysign, because we couldn't get those
functions from <math.h>. They are C99 functions. For gcc/clang we were using
C++98/03, which only includes C89 library functions. So gcc/clang <math.h>
version restricted them out. And MSVC++ <math.h> didn't have them at all.
Later, MSVC++ added them, without any version restriction since they didn't do
Standard versions back then. This collided with ours, so we renamed ours.
Later still we switched to C++14, which includes C99 library functions, so
they are no longer version restricted by gcc/clang.

scalbnA was recently being looked at because of possibile signed integer
overflow UB. And testing showed that was indeed the case. But testing also
showed that it was (as intended) pretty much compatible with scalbn. Since we
can now get scalbn from <math.h>, there's no reason to keep our own. We should
just use scalbn.

Our copysignA was similarly compatible with copysign, and was also only being
used by scalbnA. So there's no reason to keep it either.

[bridgekeeper]

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

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

8346914: UB issue in scalbnA

Reviewed-by: aph

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

• 81985d4: 8358526: Clarify behavior of java.awt.HeadlessException constructed with no-args
• d627282: 8360143: ProblemList runtime/NMT/VirtualAllocTestType.java
• 96f71a9: 8255082: HotSpot Style Guide should permit noexcept
• ... and 5 more: https://git.openjdk.org/jdk/compare/33970629ac63eea6009fca7a34c8f333f1a60a37...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]

@kimbarrett
The hotspot label was successfully added.

[mlbridge]

Webrevs

• 00: Full (d611f320)

[kimbarrett]

Long ago we had our own scalbn and copysign, because we couldn't get those functions from <math.h>.

There may have been similar issues with the AIX compiler. But since the current AIX compiler is based on a
reasonably modern version of clang, and we're using C++14, it should work there too. Though I forgot to
ask the maintainers of the aix-ppc port to verify that. I'm sure they'll let me know if this doesn't work for them.

[theRealAph]

That makes sense.