Skip to content

[GR-45718] Improve heap and stack verification. #6475

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ private boolean doCollectOnce(GCCause cause, long requestingNanoTime, boolean co
}

private void verifyBeforeGC() {
if (SubstrateGCOptions.VerifyHeap.getValue()) {
if (SubstrateGCOptions.VerifyHeap.getValue() && SerialGCOptions.VerifyBeforeGC.getValue()) {
Timer verifyBeforeTimer = timers.verifyBefore.open();
try {
boolean success = true;
Expand All @@ -332,7 +332,7 @@ private void verifyBeforeGC() {
}

private void verifyAfterGC() {
if (SubstrateGCOptions.VerifyHeap.getValue()) {
if (SubstrateGCOptions.VerifyHeap.getValue() && SerialGCOptions.VerifyAfterGC.getValue()) {
Timer verifyAfterTime = timers.verifyAfter.open();
try {
boolean success = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import com.oracle.svm.core.AlwaysInline;
import com.oracle.svm.core.genscavenge.remset.RememberedSet;
import com.oracle.svm.core.heap.ObjectHeader;
import com.oracle.svm.core.heap.ObjectReferenceVisitor;
import com.oracle.svm.core.heap.ReferenceAccess;
import com.oracle.svm.core.hub.LayoutEncoding;
Expand Down Expand Up @@ -86,7 +87,7 @@ public boolean visitObjectReferenceInline(Pointer objRef, int innerOffset, boole
// This is the most expensive check as it accesses the heap fairly randomly, which results
// in a lot of cache misses.
ObjectHeaderImpl ohi = ObjectHeaderImpl.getObjectHeaderImpl();
Word header = ohi.readHeaderFromPointer(p);
Word header = ObjectHeader.readHeaderFromPointer(p);
if (GCImpl.getGCImpl().isCompleteCollection() || !RememberedSet.get().hasRememberedSet(header)) {

if (ObjectHeaderImpl.isForwardedHeader(header)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@
import org.graalvm.word.UnsignedWord;
import org.graalvm.word.WordFactory;

import com.oracle.svm.core.MemoryWalker;
import com.oracle.svm.core.AlwaysInline;
import com.oracle.svm.core.MemoryWalker;
import com.oracle.svm.core.NeverInline;
import com.oracle.svm.core.Uninterruptible;
import com.oracle.svm.core.c.struct.PinnedObjectField;
Expand Down Expand Up @@ -332,7 +332,7 @@ public static Pointer asPointer(Header<?> that) {
public static HeapChunk.Header<?> getEnclosingHeapChunk(Object obj) {
if (!GraalDirectives.inIntrinsic()) {
assert !HeapImpl.getHeapImpl().isInImageHeap(obj) || HeapImpl.usesImageHeapChunks() : "Must be checked before calling this method";
assert !ObjectHeaderImpl.getObjectHeaderImpl().isPointerToForwardedObject(Word.objectToUntrackedPointer(obj)) : "Forwarded objects must be a pointer and not an object";
assert !ObjectHeaderImpl.isPointerToForwardedObject(Word.objectToUntrackedPointer(obj)) : "Forwarded objects must be a pointer and not an object";
}
if (ObjectHeaderImpl.isAlignedObject(obj)) {
return AlignedHeapChunk.getEnclosingChunk(obj);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,16 @@
import org.graalvm.word.UnsignedWord;

import com.oracle.svm.core.MemoryWalker;
import com.oracle.svm.core.NeverInline;
import com.oracle.svm.core.SubstrateDiagnostics;
import com.oracle.svm.core.SubstrateDiagnostics.DiagnosticThunk;
import com.oracle.svm.core.SubstrateDiagnostics.DiagnosticThunkRegistry;
import com.oracle.svm.core.SubstrateDiagnostics.ErrorContext;
import com.oracle.svm.core.SubstrateOptions;
import com.oracle.svm.core.SubstrateUtil;
import com.oracle.svm.core.NeverInline;
import com.oracle.svm.core.heap.RestrictHeapAccess;
import com.oracle.svm.core.Uninterruptible;
import com.oracle.svm.core.annotate.Substitute;
import com.oracle.svm.core.annotate.TargetClass;
import com.oracle.svm.core.Uninterruptible;
import com.oracle.svm.core.config.ConfigurationValues;
import com.oracle.svm.core.genscavenge.AlignedHeapChunk.AlignedHeader;
import com.oracle.svm.core.genscavenge.ThreadLocalAllocation.Descriptor;
Expand All @@ -69,7 +68,9 @@
import com.oracle.svm.core.heap.ReferenceHandler;
import com.oracle.svm.core.heap.ReferenceHandlerThread;
import com.oracle.svm.core.heap.ReferenceInternals;
import com.oracle.svm.core.heap.RestrictHeapAccess;
import com.oracle.svm.core.heap.RuntimeCodeInfoGCSupport;
import com.oracle.svm.core.hub.DynamicHub;
import com.oracle.svm.core.jdk.UninterruptibleUtils.AtomicReference;
import com.oracle.svm.core.locks.VMCondition;
import com.oracle.svm.core.locks.VMMutex;
Expand Down Expand Up @@ -641,6 +642,13 @@ public boolean printLocationInfo(Log log, UnsignedWord value, boolean allowJavaH
}
}

if (objectHeaderImpl.isEncodedObjectHeader((Word) value)) {
log.string("is the encoded object header for an object of type ");
DynamicHub hub = objectHeaderImpl.dynamicHubFromObjectHeader((Word) value);
log.string(hub.getName());
return true;
}

Pointer ptr = (Pointer) value;
if (printLocationInfo(log, ptr, allowJavaHeapAccess, allowUnsafeOperations)) {
if (allowJavaHeapAccess && objectHeaderImpl.pointsToObjectHeader(ptr)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.oracle.svm.core.genscavenge.AlignedHeapChunk.AlignedHeader;
import com.oracle.svm.core.genscavenge.UnalignedHeapChunk.UnalignedHeader;
import com.oracle.svm.core.genscavenge.remset.RememberedSet;
import com.oracle.svm.core.heap.ObjectHeader;
import com.oracle.svm.core.heap.ObjectReferenceVisitor;
import com.oracle.svm.core.heap.ObjectVisitor;
import com.oracle.svm.core.heap.ReferenceAccess;
Expand Down Expand Up @@ -257,7 +258,7 @@ private static boolean verifyObject(Object obj, AlignedHeader aChunk, UnalignedH
return false;
}

Word header = ObjectHeaderImpl.getObjectHeaderImpl().readHeaderFromPointer(ptr);
Word header = ObjectHeader.readHeaderFromPointer(ptr);
if (ObjectHeaderImpl.isProducedHeapChunkZapped(header) || ObjectHeaderImpl.isConsumedHeapChunkZapped(header)) {
Log.log().string("Object ").zhex(ptr).string(" has a zapped header: ").zhex(header).newline();
return false;
Expand Down Expand Up @@ -325,11 +326,11 @@ private static boolean verifyObject(Object obj, AlignedHeader aChunk, UnalignedH

// This method is executed exactly once per object in the heap.
private static boolean verifyReferences(Object obj) {
if (!SerialGCOptions.VerifyReferences.getValue()) {
if (!SerialGCOptions.VerifyReferences.getValue() && !SerialGCOptions.VerifyReferencesPointIntoValidChunk.getValue()) {
return true;
}

REFERENCE_VERIFIER.initialize(obj);
REFERENCE_VERIFIER.initialize();
InteriorObjRefWalker.walkObject(obj, REFERENCE_VERIFIER);

boolean success = REFERENCE_VERIFIER.result;
Expand Down Expand Up @@ -358,27 +359,27 @@ private static boolean verifyReference(Object parentObject, Pointer reference, P
return true;
}

if (!HeapImpl.getHeapImpl().isInHeap(referencedObject)) {
if (SerialGCOptions.VerifyReferencesPointIntoValidChunk.getValue() && !HeapImpl.getHeapImpl().isInHeap(referencedObject)) {
Log.log().string("Object reference at ").zhex(reference).string(" points outside the Java heap: ").zhex(referencedObject).string(". ");
printParentObject(parentObject);
printParent(parentObject);
return false;
}

if (!ObjectHeaderImpl.getObjectHeaderImpl().pointsToObjectHeader(referencedObject)) {
Log.log().string("Object reference at ").zhex(reference).string(" does not point to a Java object or the object header of the Java object is invalid: ").zhex(referencedObject)
.string(". ");
printParentObject(parentObject);
printParent(parentObject);
return false;
}

return true;
}

private static void printParentObject(Object parentObject) {
private static void printParent(Object parentObject) {
if (parentObject != null) {
Log.log().string("The object that contains the invalid reference is of type ").string(parentObject.getClass().getName()).newline();
} else {
Log.log().string("The invalid reference is on the stack.").newline();
Log.log().string("The invalid reference is on the stack").newline();
}
}

Expand Down Expand Up @@ -446,22 +447,20 @@ public boolean visitObject(Object object) {
}

private static class ObjectReferenceVerifier implements ObjectReferenceVisitor {
private Object parentObject;
private boolean result;

@Platforms(Platform.HOSTED_ONLY.class)
ObjectReferenceVerifier() {
}

@SuppressWarnings("hiding")
public void initialize(Object parentObject) {
this.parentObject = parentObject;
public void initialize() {
this.result = true;
}

@Override
public boolean visitObjectReference(Pointer objRef, boolean compressed, Object holderObject) {
result &= verifyReference(parentObject, objRef, compressed);
result &= verifyReference(holderObject, objRef, compressed);
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ public final class ObjectHeaderImpl extends ObjectHeader {
private static final UnsignedWord IDHASH_STATE_FROM_ADDRESS = WordFactory.unsigned(0b01);
private static final UnsignedWord IDHASH_STATE_IN_FIELD = WordFactory.unsigned(0b10);

private final int numReservedBits;
private final int numAlignmentBits;
private final int numReservedBits;
private final int numReservedExtraBits;

private final int reservedBitsMask;
Expand Down Expand Up @@ -114,52 +114,9 @@ public int getReservedBitsMask() {
return reservedBitsMask;
}

/**
* Read the header of the object at the specified address. When compressed references are
* enabled, the specified address must be the uncompressed absolute address of the object in
* memory.
*/
@Override
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public Word readHeaderFromPointer(Pointer objectPointer) {
if (getReferenceSize() == Integer.BYTES) {
return WordFactory.unsigned(objectPointer.readInt(getHubOffset()));
} else {
return objectPointer.readWord(getHubOffset());
}
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public static Word readHeaderFromObject(Object o) {
if (getReferenceSize() == Integer.BYTES) {
return WordFactory.unsigned(ObjectAccess.readInt(o, getHubOffset()));
} else {
return ObjectAccess.readWord(o, getHubOffset());
}
}

@Override
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public DynamicHub readDynamicHubFromPointer(Pointer ptr) {
Word header = readHeaderFromPointer(ptr);
return dynamicHubFromObjectHeader(header);
}

@Override
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public DynamicHub dynamicHubFromObjectHeader(Word header) {
return (DynamicHub) extractPotentialDynamicHubFromHeader(header).toObject();
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
@Override
public Pointer readPotentialDynamicHubFromPointer(Pointer ptr) {
Word potentialHeader = readHeaderFromPointer(ptr);
return extractPotentialDynamicHubFromHeader(potentialHeader);
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
private Pointer extractPotentialDynamicHubFromHeader(UnsignedWord header) {
public Pointer extractPotentialDynamicHubFromHeader(Word header) {
if (ReferenceAccess.singleton().haveCompressedReferences()) {
UnsignedWord hubBits = header.unsignedShiftRight(numReservedBits);
UnsignedWord baseRelativeBits = hubBits.shiftLeft(numAlignmentBits);
Expand Down Expand Up @@ -369,7 +326,7 @@ public static boolean isAlignedHeader(UnsignedWord header) {

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public static boolean isUnalignedObject(Object obj) {
UnsignedWord header = ObjectHeaderImpl.readHeaderFromObject(obj);
UnsignedWord header = readHeaderFromObject(obj);
return isUnalignedHeader(header);
}

Expand All @@ -389,7 +346,7 @@ public static boolean hasRememberedSet(UnsignedWord header) {
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
boolean isPointerToForwardedObject(Pointer p) {
static boolean isPointerToForwardedObject(Pointer p) {
Word header = readHeaderFromPointer(p);
return isForwardedHeader(header);
}
Expand Down Expand Up @@ -450,16 +407,6 @@ private UnsignedWord getHeaderBitsFromHeader(UnsignedWord header) {
return header.and(reservedBitsMask);
}

@Fold
static int getHubOffset() {
return ConfigurationValues.getObjectLayout().getHubOffset();
}

@Fold
static int getReferenceSize() {
return ConfigurationValues.getObjectLayout().getReferenceSize();
}

@Fold
static boolean hasFixedIdentityHashField() {
return ConfigurationValues.getObjectLayout().hasFixedIdentityHashField();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.oracle.svm.core.AlwaysInline;
import com.oracle.svm.core.genscavenge.remset.RememberedSet;
import com.oracle.svm.core.heap.Heap;
import com.oracle.svm.core.heap.ObjectHeader;
import com.oracle.svm.core.heap.ObjectReferenceVisitor;
import com.oracle.svm.core.heap.ReferenceInternals;
import com.oracle.svm.core.hub.DynamicHub;
Expand Down Expand Up @@ -219,7 +220,7 @@ private static boolean processRememberedRef(Reference<?> dr) {

private static boolean maybeUpdateForwardedReference(Reference<?> dr, Pointer referentAddr) {
ObjectHeaderImpl ohi = ObjectHeaderImpl.getObjectHeaderImpl();
UnsignedWord header = ohi.readHeaderFromPointer(referentAddr);
UnsignedWord header = ObjectHeader.readHeaderFromPointer(referentAddr);
if (ObjectHeaderImpl.isForwardedHeader(header)) {
Object forwardedObj = ohi.getForwardedObject(referentAddr);
ReferenceInternals.setReferent(dr, forwardedObj);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@
import org.graalvm.nativeimage.Platforms;
import org.graalvm.word.Pointer;

import com.oracle.svm.core.util.DuplicatedInNativeCode;
import com.oracle.svm.core.heap.ObjectHeader;
import com.oracle.svm.core.heap.ObjectReferenceVisitor;
import com.oracle.svm.core.heap.ReferenceAccess;
import com.oracle.svm.core.heap.RuntimeCodeCacheCleaner;
import com.oracle.svm.core.hub.DynamicHub;
import com.oracle.svm.core.util.DuplicatedInNativeCode;

@DuplicatedInNativeCode
final class RuntimeCodeCacheReachabilityAnalyzer implements ObjectReferenceVisitor {
Expand Down Expand Up @@ -69,7 +70,7 @@ public static boolean isReachable(Pointer ptrToObj) {
}

ObjectHeaderImpl ohi = ObjectHeaderImpl.getObjectHeaderImpl();
Word header = ohi.readHeaderFromPointer(ptrToObj);
Word header = ObjectHeader.readHeaderFromPointer(ptrToObj);
if (ObjectHeaderImpl.isForwardedHeader(header)) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,21 @@ public Integer getValue(OptionValues values) {
@Option(help = "Instrument write barriers with counters. Serial GC only.", type = OptionType.Debug)//
public static final HostedOptionKey<Boolean> CountWriteBarriers = new HostedOptionKey<>(false, SerialGCOptions::serialGCOnly);

@Option(help = "Verify the heap before doing a garbage collection if VerifyHeap is enabled. Serial GC only.", type = OptionType.Debug)//
public static final HostedOptionKey<Boolean> VerifyBeforeGC = new HostedOptionKey<>(true, SerialGCOptions::serialGCOnly);

@Option(help = "Verify the heap after doing a garbage collection if VerifyHeap is enabled. Serial GC only.", type = OptionType.Debug)//
public static final HostedOptionKey<Boolean> VerifyAfterGC = new HostedOptionKey<>(true, SerialGCOptions::serialGCOnly);

@Option(help = "Verify the remembered set if VerifyHeap is enabled. Serial GC only.", type = OptionType.Debug)//
public static final HostedOptionKey<Boolean> VerifyRememberedSet = new HostedOptionKey<>(true, SerialGCOptions::serialGCOnly);

@Option(help = "Verify all object references if VerifyHeap is enabled. Serial GC only.", type = OptionType.Debug)//
public static final HostedOptionKey<Boolean> VerifyReferences = new HostedOptionKey<>(true, SerialGCOptions::serialGCOnly);

@Option(help = "Verify that object references point into valid heap chunks if VerifyHeap is enabled. Serial GC only.", type = OptionType.Debug)//
public static final HostedOptionKey<Boolean> VerifyReferencesPointIntoValidChunk = new HostedOptionKey<>(false, SerialGCOptions::serialGCOnly);

@Option(help = "Verify write barriers. Serial GC only.", type = OptionType.Debug)//
public static final HostedOptionKey<Boolean> VerifyWriteBarriers = new HostedOptionKey<>(false, SerialGCOptions::serialGCOnly);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.oracle.svm.core.config.ConfigurationValues;
import com.oracle.svm.core.genscavenge.GCImpl.ChunkReleaser;
import com.oracle.svm.core.genscavenge.remset.RememberedSet;
import com.oracle.svm.core.heap.ObjectHeader;
import com.oracle.svm.core.heap.ObjectVisitor;
import com.oracle.svm.core.hub.LayoutEncoding;
import com.oracle.svm.core.identityhashcode.IdentityHashCodeSupport;
Expand Down Expand Up @@ -369,7 +370,7 @@ private Object copyAlignedObject(Object originalObj) {
UnsignedWord copySize = originalSize;
boolean addIdentityHashField = false;
if (!ConfigurationValues.getObjectLayout().hasFixedIdentityHashField()) {
Word header = ObjectHeaderImpl.readHeaderFromObject(originalObj);
Word header = ObjectHeader.readHeaderFromObject(originalObj);
if (probability(SLOW_PATH_PROBABILITY, ObjectHeaderImpl.hasIdentityHashFromAddressInline(header))) {
addIdentityHashField = true;
copySize = LayoutEncoding.getSizeFromObjectInlineInGC(originalObj, true);
Expand Down
Loading