[lworld] RFR: 8353236: [lworld] Better documentation for Valhalla Unsafe intrinsics [v2]

[Roger Riggs]

On Thu, 10 Apr 2025 05:33:39 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Hi,
>> 
>> This patch clarifies the meaning of undefined behavior when working with `Unsafe` and specifies the requirements when working with `Unsafe::makePrivateBuffer` and `Unsafe::finishPrivateBuffer`.
>> 
>> Please take a look and leave your suggestions, thanks a lot.
>
> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   more explanation

src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 94:

> 92:  * <p>
> 93:  * By default, usage of all methods in this class exhibits undefined behavior,
> 94:  * unless otherwise explicitly specified.

The "by default" and "unless otherwise specified" seems to be redundant.

Suggestion:

 * Unless otherwise explicitly specified, all methods in this class can exhibit undefined behavior.
 * .

src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 442:

> 440:      * effectively final as well as definitely assigned to with the return
> 441:      * value of this method. The object must also be not assigned to another
> 442:      * local variable.

Why this restriction? What assumption is violated if there is a second assignment?

src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 447:

> 445:      * other usage, such as loading from or returning it, is illegal. The only
> 446:      * exception is the implicit check cast inserted by the compiler on the
> 447:      * return value of this method. Explicit check casts are not allowed.

"not allowed"?  But there is no enforcement; unpredictable behavior yes.

src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 449:

> 447:      * return value of this method. Explicit check casts are not allowed.
> 448:      * </ul>
> 449:      * Illegal usage of this method exhibits undefined behavior even if the

may or can; but not always.

src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 470:

> 468:      * <li>After the invocation of this method, the variable that holds the
> 469:      * argument passed into this method must not be used.
> 470:      * </ul>

I would add the exhortation that every field of the value object must have a value.

src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 472:

> 470:      * </ul>
> 471:      * Illegal usage of this method exhibits undefined behavior even if the
> 472:      * illegal statements are never actually reached at runtime.

Another can/may exhibit case; improve the phrase everywhere.

-------------

PR Review Comment: https://git.openjdk.org/valhalla/pull/1425#discussion_r2037359302
PR Review Comment: https://git.openjdk.org/valhalla/pull/1425#discussion_r2037392238
PR Review Comment: https://git.openjdk.org/valhalla/pull/1425#discussion_r2037395701
PR Review Comment: https://git.openjdk.org/valhalla/pull/1425#discussion_r2037398261
PR Review Comment: https://git.openjdk.org/valhalla/pull/1425#discussion_r2037405322
PR Review Comment: https://git.openjdk.org/valhalla/pull/1425#discussion_r2037402279