8347707: Standardise the use of os::snprintf and os::snprintf_checked

[dholmes-ora]

EDIT: I've just realized that the [[nodiscard]] attribute is not currently permitted. So I may have to revise this aspect of the changes.

This is a proposal to standardize on the use of os::snprintf and os::snprintf_checked across the hotspot code base, and to disallow use of the C library variants. (It does not touch use of jio_printf at all.)

From: https://bugs.openjdk.org/browse/JDK-8347707

The platform snprintf/vsnprintf returns -1 on error, else if the buffer is large enough returns the number of bytes written (excluding the null byte), else (buffer is too small) the number of characters (excluding the terminating null byte) which would have been written to the final string if enough space had been available. Thus, a return value of size or more means that the output was truncated.

To provide a consistent approach to error handling and truncation management, we provide os::xxx wrapper functions as described below and forbid the use of the library ::vsnprintf and ::snprintf.

The potential errors are, generally speaking, not something we should encounter in our own well-written code:

• encoding error: not applicable as we are not using extended character sets
• invalid parameters (null buffers, specifying a limit > size of the buffer [Windows], things of this nature)
• mal-formed formatting directives
• overflow error (POSIX) if the required buffer size exceeds INT_MAX (as we return int).

As these should simply never occur, we handle the checks for -1 at the lowest-level (os::vsnprintf) with an assertion, and accompanying precondition assertions.

The potential clients of this API then fall into a number of camps:

1. Those who have sized their buffer correctly, don't need the return value for subsequent use, and for whom truncation (if it were possible) would be a programming error.

For these clients we have void os::snprintf_checked - which returns nothing and asserts on truncation.

2. Those who have sized their buffer correctly, but do need the return value for subsequent operations (e.g. chains of snprintf where you advance the buffer pointer based on previous writes), but again for whom truncation should never happen.

For these clients we have os::snprintf, but they have to add their own assertion for no truncation.

3. Those who present a buffer but know that truncation is a possibility, but don't need to do anything about it themselves, and for whom the return value is of no use.

These clients also use os::snprintf_checked. The truncation assertion can be useful for guiding buffer sizing decisions, but in product mode truncation is not an error.

4. Those who present a buffer but know that truncation is a possibility, and either need to handle it themselves, or else need to use the return value in subsequent operations.

These clients are also directed to use os::snprintf.

In summary we provide the following API:

• [[nodiscard]] int os::vsnprintf is the building block for the other methods, it:
  • asserts on precondition failures
  • asserts on error
  • guarantees null-termination in the case of unexpected errors (as the standards are still unclear on that point
  • is declared [[nodiscard[]] so that callers cannot ignore the return value (they can explicitly cast to void to indicate they don't need it)
• void os::snprintf_checked
  • calls `os::vnsprintf`` so asserts on errors
  • asserts on truncation
• [[nodiscard]] int os::snprintf
  • calls os::vnsprintf so asserts on errors

In terms of the effects on the existing code we:

• Change callers of ::snprintf/os::snprintf that ignore the return value and ensure the buffer is large enough to use os::snprintf_checked
  • those that allow truncation to happen must use os::snprintf.
• Change all callers of ::snprintf/os::snprintf that use the return value to use os::snprintf, plus any additional assertions needed
• Change the 9 callers of os::snprintf_checked that do use the return value, to use os::snprintf with their own assertions added
• Callers of os::vnsprintf are adjusted as needed

The PR is comprising multiple dependent commits so that you can view things in stages. There are 46 modified files. The bulk of the changes replace calls to snprintf/os::snprintf with calls to os::snprintf_checked.

---

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-8347707: Standardise the use of os::snprintf and os::snprintf_checked (Enhancement - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26470

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

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

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@dholmes-ora The following labels will be automatically applied to this pull request:

• graal
• hotspot
• serviceability

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

[mlbridge]

Webrevs

• 00: Full (c10b6d89)