[jdk19] RFR: 8289148: j.l.foreign.VaList::nextVarg call could throw IndexOutOfBoundsException or even crash the VM [v2]

[Jorn Vernee]

On Tue, 5 Jul 2022 15:14:20 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> This seems a bit too much.
>> 
>> The class javadoc further up already describes a va list as "a stateful cursor used to iterate over a set of arguments". If that description is insufficient, I think it should be amended at that point.
>> 
>> Also, I don't think we have to go into describing how va lists returned by different factories are implemented (in fact, I think we should avoid that in order not to make accidental false promises when things change). It seems like noise next to the safety concerns (if someone really wants to know how the specified semantics are implemented, they can look at the code, or ask on the mailing list).
>> 
>> I'd suggest something simpler like this:
>> 
>> Suggestion:
>> 
>>  * It is possible for clients to access elements outside the spatial bounds of a variable argument list. Variable argument list
>>  * implementations will try to detect out-of-bounds reads on a best-effort basis.
>>  * <p>
>>  * Whether this detection succeeds depends on the factory method used to create the variable argument list:
>>  * <ul>
>>  *     <li>Variable argument lists created <em>safely</em>, using {@link #make(Consumer, MemorySession)} are capable of detecting out-of-bounds reads;</li>
>>  *     <li>Variable argument lists created <em>unsafely</em>, using {@link #ofAddress(MemoryAddress, MemorySession)} are not capable of detecting out-of-bounds reads</li>
>>  * </ul>
>>  * <p>
>> 
>> 
>> I'm also fine with changing the section title to "Safety considerations".
>
> i.e. I'd like to just specify the behavior, and avoid explaining why we have that behavior.

I've updated the PR with this suggestion for now. Please let me know if it looks OK to you as well.

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

PR: https://git.openjdk.org/jdk19/pull/91