Skip to content

Commit

Permalink
Merge pull request #7607 from chrisvest/3.0-native-access-check
Browse files Browse the repository at this point in the history
Add sanity checks on all native memory accesses
  • Loading branch information
MishaDemianenko committed Jul 20, 2016
2 parents 91a0796 + 6f3e619 commit 1853fd4
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 3 deletions.
2 changes: 1 addition & 1 deletion community/kernel/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ the relevant Commercial Agreement.
<plugin>
<artifactId>maven-failsafe-plugin</artifactId>
<configuration>
<argLine>-Xmx300m -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=target/test-data -Dorg.neo4j.unsafe.impl.internal.dragons.UnsafeUtil.DIRTY_MEMORY=true -Dorg.neo4j.io.pagecache.impl.muninn.usePreciseCursorErrorStackTraces=true -XX:+UnlockExperimentalVMOptions -XX:+TrustFinalNonStaticFields -XX:-OmitStackTraceInFastThrow</argLine>
<argLine>-Xmx300m -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=target/test-data -Dorg.neo4j.unsafe.impl.internal.dragons.UnsafeUtil.DIRTY_MEMORY=true -Dorg.neo4j.unsafe.impl.internal.dragons.UnsafeUtil.CHECK_NATIVE_ACCESS=true -Dorg.neo4j.io.pagecache.impl.muninn.usePreciseCursorErrorStackTraces=true -XX:+UnlockExperimentalVMOptions -XX:+TrustFinalNonStaticFields -XX:-OmitStackTraceInFastThrow</argLine>
</configuration>
</plugin>
<plugin>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,14 @@
import java.nio.ByteOrder;
import java.security.AccessController;
import java.security.PrivilegedExceptionAction;

import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentSkipListMap;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;

import static java.lang.String.format;
import static org.neo4j.unsafe.impl.internal.dragons.FeatureToggles.flag;

/**
Expand All @@ -50,6 +57,7 @@ public final class UnsafeUtil
* and verify that our code does not assume that memory is clean when allocated.
*/
private static final boolean DIRTY_MEMORY = flag( UnsafeUtil.class, "DIRTY_MEMORY", false );
private static final boolean CHECK_NATIVE_ACCESS = flag( UnsafeUtil.class, "CHECK_NATIVE_ACCESS", false );

private static final Unsafe unsafe;
private static final MethodHandle sharedStringConstructor;
Expand Down Expand Up @@ -310,6 +318,7 @@ public static long allocateMemory( long sizeInBytes )
{
setMemory( pointer, sizeInBytes, (byte) 0xA5 );
}
addAllocatedPointer( pointer, sizeInBytes );
return pointer;
}

Expand All @@ -318,9 +327,122 @@ public static long allocateMemory( long sizeInBytes )
*/
public static void free( long pointer )
{
checkFree( pointer );
unsafe.freeMemory( pointer );
}

private static final class FreeTrace extends Throwable implements Comparable<FreeTrace>
{
private final long pointer;
private final long size;
private final long id;
private final long nanoTime;
private long referenceTime;

private FreeTrace( long pointer, long size, long id )
{
this.pointer = pointer;
this.size = size;
this.id = id;
this.nanoTime = System.nanoTime();
}

private boolean contains( long pointer )
{
return this.pointer <= pointer && pointer <= this.pointer + size;
}

@Override
public int compareTo( FreeTrace that )
{
return Long.compare( this.id, that.id );
}

@Override
public String getMessage()
{
return format( "0x%x of %6d bytes, freed %s µs ago at", pointer, size, (referenceTime - nanoTime) / 1000 );
}
}

private static final ConcurrentSkipListMap<Long, Long> pointers = new ConcurrentSkipListMap<>();
private static final FreeTrace[] freeTraces = CHECK_NATIVE_ACCESS? new FreeTrace[4096] : null;
private static final AtomicLong freeTraceCounter = new AtomicLong();

private static void addAllocatedPointer( long pointer, long sizeInBytes )
{
if ( CHECK_NATIVE_ACCESS )
{
pointers.put( pointer, sizeInBytes );
}
}

private static void checkFree( long pointer )
{
if ( CHECK_NATIVE_ACCESS )
{
doCheckFree( pointer );
}
}

private static void doCheckFree( long pointer )
{
Long size = pointers.remove( pointer );
if ( size == null )
{
StringBuilder sb = new StringBuilder( format( "Bad free: 0x%x, valid pointers are:", pointer ) );
pointers.forEach( (k,v) -> sb.append( '\n' ).append( k ) );
throw new AssertionError( sb.toString() );
}
long count = freeTraceCounter.getAndIncrement();
int idx = (int) (count & 4095);
freeTraces[idx] = new FreeTrace( pointer, size, count );
}

private static void checkAccess( long pointer, int size )
{
if ( CHECK_NATIVE_ACCESS )
{
doCheckAccess( pointer, size );
}
}

private static void doCheckAccess( long pointer, int size )
{
long now = System.nanoTime();
Map.Entry<Long,Long> fentry = pointers.floorEntry( pointer + size );
Map.Entry<Long,Long> centry = pointers.ceilingEntry( pointer );
if ( fentry == null || fentry.getKey() + fentry.getValue() < pointer + size )
{
long faddr = fentry == null? 0 : fentry.getKey();
long fsize = fentry == null? 0 : fentry.getValue();
long foffset = pointer - (faddr + fsize);
long caddr = centry == null? 0 : centry.getKey();
long csize = centry == null? 0 : centry.getValue();
long coffset = caddr - (pointer + size);
boolean floorIsNearest = foffset < coffset;
long naddr = floorIsNearest? faddr : caddr;
long nsize = floorIsNearest? fsize : csize;
long noffset = floorIsNearest? foffset : coffset;
List<FreeTrace> recentFrees = Arrays.stream( freeTraces )
.filter( trace -> trace != null )
.filter( trace -> trace.contains( pointer ) )
.sorted()
.collect( Collectors.toList() );
AssertionError error = new AssertionError( format(
"Bad access at address 0x%x with size %s, nearest valid allocation is " +
"0x%x (%s bytes, off by %s bytes). " +
"Recent relevant frees (of %s) are attached as suppressed exceptions.",
pointer, size, naddr, nsize, noffset, freeTraceCounter.get() ) );
for ( FreeTrace recentFree : recentFrees )
{
recentFree.referenceTime = now;
error.addSuppressed( recentFree );
}
throw error;
}
}

/**
* Return the power-of-2 native memory page size.
*/
Expand Down Expand Up @@ -351,21 +473,25 @@ public static boolean getBooleanVolatile( Object obj, long offset )

public static void putByte( long address, byte value )
{
checkAccess( address, Byte.BYTES );
unsafe.putByte( address, value );
}

public static byte getByte( long address )
{
checkAccess( address, Byte.BYTES );
return unsafe.getByte( address );
}

public static void putByteVolatile( long address, byte value )
{
checkAccess( address, Byte.BYTES );
unsafe.putByteVolatile( null, address, value );
}

public static byte getByteVolatile( long address )
{
checkAccess( address, Byte.BYTES );
return unsafe.getByteVolatile( null, address );
}

Expand All @@ -391,21 +517,25 @@ public static void putByteVolatile( Object obj, long offset, byte value )

public static void putShort( long address, short value )
{
checkAccess( address, Short.BYTES );
unsafe.putShort( address, value );
}

public static short getShort( long address )
{
checkAccess( address, Short.BYTES );
return unsafe.getShort( address );
}

public static void putShortVolatile( long address, short value )
{
checkAccess( address, Short.BYTES );
unsafe.putShortVolatile( null, address, value );
}

public static short getShortVolatile( long address )
{
checkAccess( address, Short.BYTES );
return unsafe.getShortVolatile( null, address );
}

Expand All @@ -431,21 +561,25 @@ public static short getShortVolatile( Object obj, long offset )

public static void putFloat( long address, float value )
{
checkAccess( address, Float.BYTES );
unsafe.putFloat( address, value );
}

public static float getFloat( long address )
{
checkAccess( address, Float.BYTES );
return unsafe.getFloat( address );
}

public static void putFloatVolatile( long address, float value )
{
checkAccess( address, Float.BYTES );
unsafe.putFloatVolatile( null, address, value );
}

public static float getFloatVolatile( long address )
{
checkAccess( address, Float.BYTES );
return unsafe.getFloatVolatile( null, address );
}

Expand All @@ -471,21 +605,25 @@ public static float getFloatVolatile( Object obj, long offset )

public static void putChar( long address, char value )
{
checkAccess( address, Character.BYTES );
unsafe.putChar( address, value );
}

public static char getChar( long address )
{
checkAccess( address, Character.BYTES );
return unsafe.getChar( address );
}

public static void putCharVolatile( long address, char value )
{
checkAccess( address, Character.BYTES );
unsafe.putCharVolatile( null, address, value );
}

public static char getCharVolatile( long address )
{
checkAccess( address, Character.BYTES );
return unsafe.getCharVolatile( null, address );
}

Expand All @@ -511,21 +649,25 @@ public static char getCharVolatile( Object obj, long offset )

public static void putInt( long address, int value )
{
checkAccess( address, Integer.BYTES );
unsafe.putInt( address, value );
}

public static int getInt( long address )
{
checkAccess( address, Integer.BYTES );
return unsafe.getInt( address );
}

public static void putIntVolatile( long address, int value )
{
checkAccess( address, Integer.BYTES );
unsafe.putIntVolatile( null, address, value );
}

public static int getIntVolatile( long address )
{
checkAccess( address, Integer.BYTES );
return unsafe.getIntVolatile( null, address );
}

Expand All @@ -551,21 +693,25 @@ public static int getIntVolatile( Object obj, long offset )

public static void putLongVolatile( long address, long value )
{
checkAccess( address, Long.BYTES );
unsafe.putLongVolatile( null, address, value );
}

public static long getLongVolatile( long address )
{
checkAccess( address, Long.BYTES );
return unsafe.getLongVolatile( null, address );
}

public static void putLong( long address, long value )
{
checkAccess( address, Long.BYTES );
unsafe.putLong( address, value );
}

public static long getLong( long address )
{
checkAccess( address, Long.BYTES );
return unsafe.getLong( address );
}

Expand All @@ -591,21 +737,25 @@ public static long getLongVolatile( Object obj, long offset )

public static void putDouble( long address, double value )
{
checkAccess( address, Double.BYTES );
unsafe.putDouble( address, value );
}

public static double getDouble( long address )
{
checkAccess( address, Double.BYTES );
return unsafe.getDouble( address );
}

public static void putDoubleVolatile( long address, double value )
{
checkAccess( address, Double.BYTES );
unsafe.putDoubleVolatile( null, address, value );
}

public static double getDoubleVolatile( long address )
{
checkAccess( address, Double.BYTES );
return unsafe.getDoubleVolatile( null, address );
}

Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<bouncycastle.version>1.53</bouncycastle.version>
<generate-config-docs-phase>prepare-package</generate-config-docs-phase>
<hsqldb.version>2.3.2</hsqldb.version>
<test.runner.jvm.settings>-Xmx1G -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=target/test-data -Dorg.neo4j.unsafe.impl.internal.dragons.UnsafeUtil.DIRTY_MEMORY=true -Dorg.neo4j.io.pagecache.impl.muninn.usePreciseCursorErrorStackTraces=true -XX:+UnlockExperimentalVMOptions -XX:+TrustFinalNonStaticFields</test.runner.jvm.settings>
<test.runner.jvm.settings>-Xmx1G -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=target/test-data -Dorg.neo4j.unsafe.impl.internal.dragons.UnsafeUtil.DIRTY_MEMORY=true -Dorg.neo4j.unsafe.impl.internal.dragons.UnsafeUtil.CHECK_NATIVE_ACCESS=true -Dorg.neo4j.io.pagecache.impl.muninn.usePreciseCursorErrorStackTraces=true -XX:+UnlockExperimentalVMOptions -XX:+TrustFinalNonStaticFields</test.runner.jvm.settings>
<doclint-groups>reference</doclint-groups>
<jetty.version>9.2.9.v20150224</jetty.version>
<findbugs.version>3.0.0</findbugs.version>
Expand Down

0 comments on commit 1853fd4

Please sign in to comment.