Merge pull request #1326 from matthiasblaesing/share_internal_memory

Ensure pointers indirected from Memory and pointing into Memory should retain originating object
java-native-access · Mar 14, 2021 · 0de51b3 · 0de51b3
2 parents 25194e1 + 5d23330
commit 0de51b3
Show file tree

Hide file tree

Showing 6 changed files with 344 additions and 7 deletions.
diff --git a/.gitignore b/.gitignore
@@ -30,4 +30,5 @@ pom-jna-platform.xml.asc
 /contrib/platform/${build.generated.sources.dir}/
 /contrib/platform/${build}/
 /contrib/platform/nbproject/private/
-/nbproject/private/
+/nbproject/private/
+/native/.ccls-cache/
diff --git a/CHANGES.md b/CHANGES.md
@@ -14,6 +14,7 @@ Bug Fixes
 ---------
 * [#1317](https://github.com/java-native-access/jna/pull/1317): Change the maven coordinates of the JPMS artifacts from classifier `jpms` to custom artifact ids `jna-jpms` and `jna-platform-jpms` - [@matthiasblaesing](https://github.com/matthiasblaesing).
 * [#1322](https://github.com/java-native-access/jna/pull/1322): Handle 0-length domain names in `c.s.j.p.win32.Advapi32Util#getAccountBySid` - [@dbwiddis](https://github.com/dbwiddis).
+* [#1326](https://github.com/java-native-access/jna/pull/1326): Ensure pointers indirected from Memory and pointing into Memory retain originating object - [@matthiasblaesing](https://github.com/matthiasblaesing).
 
 Important Changes
 -----------------

diff --git a/nbproject/ide-file-targets.xml b/nbproject/ide-file-targets.xml
@@ -0,0 +1,65 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project basedir=".." name="JNA-IDE">
+    <target name="run-selected-file-in-src">
+        <fail unless="run.class">Must set property 'run.class'</fail>
+
+        <subant antfile="build.xml" buildpath="${basedir}" target="compile-tests"/>
+
+        <java classname="${run.class}" failonerror="true" fork="true">
+            <classpath>
+                <pathelement path="build/classes"/>
+                <pathelement path="build/test-classes"/>
+            </classpath>
+        </java>
+    </target>
+    <target name="debug-selected-file-in-src">
+        <fail unless="debug.class">Must set property 'debug.class'</fail>
+
+        <subant antfile="build.xml" buildpath="${basedir}" target="compile-tests"/>
+
+        <path id="cp">
+            <pathelement path="build/classes"/>
+            <pathelement path="build/test-classes"/>
+        </path>
+        <nbjpdastart addressproperty="jpda.address" name="JNA" transport="dt_socket">
+            <classpath refid="cp"/>
+        </nbjpdastart>
+        <java classname="${debug.class}" fork="true">
+            <classpath refid="cp"/>
+            <jvmarg value="-agentlib:jdwp=transport=dt_socket,address=${jpda.address}"/>
+        </java>
+    </target>
+
+    <target name="run-selected-file-in-test">
+        <fail unless="run.class">Must set property 'run.class'</fail>
+
+        <subant antfile="build.xml" buildpath="${basedir}" target="compile-tests"/>
+
+        <java classname="${run.class}" failonerror="true" fork="true">
+            <classpath>
+                <pathelement path="lib/hamcrest-core-1.3.jar:lib/junit.jar:lib/test/dom4j-1.6.1.jar:lib/test/guava-27.1-jre.jar:lib/test/javassist-3.12.1.GA.jar:lib/test/reflections-0.9.11.jar:lib/test/slf4j-api-1.6.1.jar"/>
+                <pathelement path="build/jna.jar"/>
+                <pathelement path="build/test-classes"/>
+            </classpath>
+        </java>
+    </target>
+
+    <target name="debug-selected-file-in-test">
+        <fail unless="debug.class">Must set property 'debug.class'</fail>
+
+        <subant antfile="build.xml" buildpath="${basedir}" target="compile-tests"/>
+
+        <path id="cp">
+            <pathelement path="lib/hamcrest-core-1.3.jar:lib/junit.jar:lib/test/dom4j-1.6.1.jar:lib/test/guava-27.1-jre.jar:lib/test/javassist-3.12.1.GA.jar:lib/test/reflections-0.9.11.jar:lib/test/slf4j-api-1.6.1.jar"/>
+            <pathelement path="build/jna.jar"/>
+            <pathelement path="build/test-classes"/>
+        </path>
+        <nbjpdastart addressproperty="jpda.address" name="JNA" transport="dt_socket">
+            <classpath refid="cp"/>
+        </nbjpdastart>
+        <java classname="${debug.class}" fork="true">
+            <classpath refid="cp"/>
+            <jvmarg value="-agentlib:jdwp=transport=dt_socket,address=${jpda.address}"/>
+        </java>
+    </target>
+</project>
diff --git a/nbproject/project.xml b/nbproject/project.xml
@@ -56,6 +56,58 @@ auxiliary.show.customizer.message=<message>
                     <target>clean</target>
                     <target>jar</target>
                 </action>
+                <action name="run.single">
+                    <script>nbproject/ide-file-targets.xml</script>
+                    <target>run-selected-file-in-src</target>
+                    <context>
+                        <property>run.class</property>
+                        <folder>src</folder>
+                        <pattern>\.java$</pattern>
+                        <format>java-name</format>
+                        <arity>
+                            <one-file-only/>
+                        </arity>
+                    </context>
+                </action>
+                <action name="debug.single">
+                    <script>nbproject/ide-file-targets.xml</script>
+                    <target>debug-selected-file-in-src</target>
+                    <context>
+                        <property>debug.class</property>
+                        <folder>src</folder>
+                        <pattern>\.java$</pattern>
+                        <format>java-name</format>
+                        <arity>
+                            <one-file-only/>
+                        </arity>
+                    </context>
+                </action>
+                <action name="debug.single">
+                    <script>nbproject/ide-file-targets.xml</script>
+                    <target>debug-selected-file-in-test</target>
+                    <context>
+                        <property>debug.class</property>
+                        <folder>test</folder>
+                        <pattern>\.java$</pattern>
+                        <format>java-name</format>
+                        <arity>
+                            <one-file-only/>
+                        </arity>
+                    </context>
+                </action>
+                <action name="run.single">
+                    <script>nbproject/ide-file-targets.xml</script>
+                    <target>run-selected-file-in-test</target>
+                    <context>
+                        <property>run.class</property>
+                        <folder>test</folder>
+                        <pattern>\.java$</pattern>
+                        <format>java-name</format>
+                        <arity>
+                            <one-file-only/>
+                        </arity>
+                    </context>
+                </action>
             </ide-actions>
             <view>
                 <items>
@@ -94,7 +146,7 @@ auxiliary.show.customizer.message=<message>
             <compilation-unit>
                 <package-root>test</package-root>
                 <unit-tests/>
-                <classpath mode="compile">lib/hamcrest-core-1.3.jar:lib/junit.jar:lib/test/dom4j-1.6.1.jar:lib/test/guava-11.0.2.jar:lib/test/javassist-3.12.1.GA.jar:lib/test/reflections-0.9.11.jar:lib/test/slf4j-api-1.6.1.jar:src</classpath>
+                <classpath mode="compile">lib/hamcrest-core-1.3.jar:lib/junit.jar:lib/test/dom4j-1.6.1.jar:lib/test/guava-27.1-jre.jar:lib/test/javassist-3.12.1.GA.jar:lib/test/reflections-0.9.11.jar:lib/test/slf4j-api-1.6.1.jar:src</classpath>
                 <source-level>1.8</source-level>
             </compilation-unit>
             <compilation-unit>

diff --git a/src/com/sun/jna/Memory.java b/src/com/sun/jna/Memory.java
@@ -400,7 +400,7 @@ public void read(long bOff, short[] buf, int index, int length) {
      */
     @Override
     public void read(long bOff, char[] buf, int index, int length) {
-        boundsCheck(bOff, length * 2L);
+        boundsCheck(bOff, length * Native.WCHAR_SIZE);
         super.read(bOff, buf, index, length);
     }
 
@@ -460,6 +460,20 @@ public void read(long bOff, double[] buf, int index, int length) {
         super.read(bOff, buf, index, length);
     }
 
+    /**
+     * Indirect the native pointer to <code>malloc</code> space, a la
+     * <code>Pointer.read</code>.  But this method performs a bounds checks to
+     * ensure that the indirection does not cause memory outside the
+     * <code>malloc</code>ed space to be accessed.
+     *
+     * @see Pointer#read(long,Pointer[],int,int)
+     */
+    @Override
+    public void read(long bOff, Pointer[] buf, int index, int length) {
+        boundsCheck(bOff, length * Native.POINTER_SIZE);
+        super.read(bOff, buf, index, length);
+    }
+
     //////////////////////////////////////////////////////////////////////////
     // Raw write methods
     //////////////////////////////////////////////////////////////////////////
@@ -502,7 +516,7 @@ public void write(long bOff, short[] buf, int index, int length) {
      */
     @Override
     public void write(long bOff, char[] buf, int index, int length) {
-        boundsCheck(bOff, length * 2L);
+        boundsCheck(bOff, length * Native.WCHAR_SIZE);
         super.write(bOff, buf, index, length);
     }
 
@@ -562,6 +576,20 @@ public void write(long bOff, double[] buf, int index, int length) {
         super.write(bOff, buf, index, length);
     }
 
+    /**
+     * Indirect the native pointer to <code>malloc</code> space, a la
+     * <code>Pointer.write</code>.  But this method performs a bounds
+     * checks to ensure that the indirection does not cause memory outside the
+     * <code>malloc</code>ed space to be accessed.
+     *
+     * @see Pointer#write(long,Pointer[],int,int)
+     */
+    @Override
+    public void write(long bOff, Pointer[] buf, int index, int length) {
+        boundsCheck(bOff, length * Native.POINTER_SIZE);
+        super.write(bOff, buf, index, length);
+    }
+
     //////////////////////////////////////////////////////////////////////////
     // Java type read methods
     //////////////////////////////////////////////////////////////////////////
@@ -590,7 +618,7 @@ public byte getByte(long offset) {
      */
     @Override
     public char getChar(long offset) {
-        boundsCheck(offset, 1);
+        boundsCheck(offset, Native.WCHAR_SIZE);
         return super.getChar(offset);
     }
 
@@ -675,7 +703,7 @@ public double getDouble(long offset) {
     @Override
     public Pointer getPointer(long offset) {
         boundsCheck(offset, Native.POINTER_SIZE);
-        return super.getPointer(offset);
+        return shareReferenceIfInBounds(super.getPointer(offset));
     }
 
     /**
@@ -862,4 +890,26 @@ protected static long malloc(long size) {
     public String dump() {
         return dump(0, (int)size());
     }
+
+    /**
+     * Check whether the supplied Pointer object points into the memory region
+     * backed by this memory object. The intention is to prevent premature GC
+     * of the Memory object.
+     *
+     * @param target Pointer to check
+     * @return {@code target} if target does not point into the region covered
+     * by this memory object, a newly {@code SharedMemory} object, if the pointer
+     * points to memory backed by this Memory object.
+     */
+    private Pointer shareReferenceIfInBounds(Pointer target) {
+        if(target == null) {
+            return null;
+        }
+        long offset = target.peer - this.peer;
+        if (offset >= 0 && offset < this.size) {
+            return this.share(offset);
+        } else {
+            return target;
+        }
+    }
 }