On Tue, 5 Jul 2022 15:07:32 GMT, Jorn Vernee <[email protected]> wrote:
>> src/java.base/share/classes/java/lang/foreign/VaList.java line 61:
>>
>>> 59: * Particularly, variable argument lists created using {@link
>>> #make(Consumer, MemorySession)} are able to detect out-of-bounds reads,
>>> 60: * while variable argument lists crearted using {@link
>>> #ofAddress(MemoryAddress, MemorySession)} are not.
>>> 61: * <p>
>>
>> Suggestion:
>>
>> * <h2>Safety considerations</h2>
>> * A variable argument list can be thought of as a cursor into a memory
>> segment which stores one or
>> * more elements, with given layouts. The {@linkplain
>> #nextVarg(ValueLayout.OfInt) methods} used to retrieve elements from
>> * a variable argument list update the cursor position; as such it is
>> possible for clients to access elements
>> * outside the spatial bounds of the variable argument list.
>> * <p>
>> * A variable argument list attempts to detect out-of-bounds access on a
>> best-effort basis.
>> * 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 built on top of
>> * native memory segments that are sized correctly. As such, they are
>> capable of detecting out-of-bounds reads;</li>
>> * <li>Variable argument lists created <em>unsafely</em>, using {@link
>> #ofAddress(MemoryAddress, MemorySession)} are built on top
>> * of segments whose size is unknown. As such they cannot detect
>> out-of-bounds reads</li>
>> * <p>```
>
> 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.
-------------
PR: https://git.openjdk.org/jdk19/pull/91
