On Wed, 29 Jun 2022 13:40:01 GMT, Jorn Vernee <[email protected]> wrote:
> This patch changes all VaList implementations to throw
> `NoSuchElementException` when out of bounds reads occur on a VaList that is
> created using the Java builder API. The docs are updated accordingly.
>
> For VaLists that are created from native addresses, we don't know their
> bounds, so we can not throw exceptions in that case.
>
> Testing: `jdk_foreign` test suite on all platforms that implement VaList.
Looks good - I've added some comments, esp. in the javadoc
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>```
src/java.base/share/classes/java/lang/foreign/VaList.java line 99:
> 97: * @throws WrongThreadException if this method is called from a
> thread other than the thread owning
> 98: * the {@linkplain #session() session} associated with this variable
> argument list.
> 99: * @throws NoSuchElementException if an out-of-bounds read is
> detected.
I'd insert a link to the safety section, where the link text is "out-of-bounds".
src/java.base/share/classes/java/lang/foreign/VaList.java line 202:
> 200: * restricted methods, and use safe and supported functionalities,
> where possible.
> 201: *
> 202: * @implNote variable argument lists created using this method can
> not detect out-of-bound reads.
Links here too
src/java.base/share/classes/jdk/internal/foreign/abi/x64/sysv/SysVVaList.java
line 291:
> 289: }
> 290:
> 291: private void checkRegAreaElement(MemoryLayout layout, TypeClass
> typeClass) {
should this be `checkRegSaveAreaElement` ?
test/jdk/java/foreign/valist/VaListTest.java line 827:
> 825:
> 826: @DataProvider
> 827: public static Object[][] overflow() {
nice test!
-------------
PR: https://git.openjdk.org/jdk19/pull/91
