Skip to content

Commit

Permalink
[GR-42996] Refactor shadow heap verification and fix lazy scanned val…
Browse files Browse the repository at this point in the history
…ues.

PullRequest: graal/15503
  • Loading branch information
cstancu committed Sep 8, 2023
2 parents 365052a + 59751ac commit 28a4718
Show file tree
Hide file tree
Showing 13 changed files with 293 additions and 202 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.oracle.graal.pointsto.heap.HeapSnapshotVerifier;
import com.oracle.graal.pointsto.heap.ImageHeap;
import com.oracle.graal.pointsto.heap.ImageHeapScanner;
import com.oracle.graal.pointsto.infrastructure.UniverseMetaAccess;
import com.oracle.graal.pointsto.standalone.StandaloneObjectScanner;
import com.oracle.graal.pointsto.util.CompletionExecutor;

Expand All @@ -40,7 +41,7 @@ public StandaloneHeapSnapshotVerifier(BigBang bb, ImageHeap imageHeap, ImageHeap
}

@Override
protected ObjectScanner installObjectScanner(CompletionExecutor executor) {
protected ObjectScanner installObjectScanner(UniverseMetaAccess metaAccess, CompletionExecutor executor) {
StandaloneImageHeapScanner standaloneImageHeapScanner = (StandaloneImageHeapScanner) this.scanner;
return new StandaloneObjectScanner(bb, executor, scannedObjects, new ScanningObserver(), standaloneImageHeapScanner.getShouldScanConstant(),
standaloneImageHeapScanner.getShouldScanField());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,10 @@ public void runAnalysis(DebugContext debugContext, Function<AnalysisUniverse, Bo
protected abstract CompletionExecutor.Timing getTiming();

@SuppressWarnings("try")
private boolean analysisModified() throws InterruptedException {
private boolean analysisModified() {
boolean analysisModified;
try (Timer.StopTimer ignored = verifyHeapTimer.start()) {
analysisModified = universe.getHeapVerifier().requireAnalysisIteration(executor);
analysisModified = universe.getHeapVerifier().checkHeapSnapshot(metaAccess, executor, "after analysis", true);
}
/* Initialize for the next iteration. */
executor.init(getTiming());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void scanBootImageHeapRoots(Comparator<AnalysisField> fieldComparator, Co
fields = fieldsList;
}
for (AnalysisField field : fields) {
if (Modifier.isStatic(field.getModifiers()) && field.getJavaKind() == JavaKind.Object && field.isRead()) {
if (Modifier.isStatic(field.getModifiers()) && field.isRead()) {
execute(() -> scanRootField(field));
}
}
Expand Down Expand Up @@ -157,7 +157,9 @@ protected void scanField(AnalysisField field, JavaConstant receiver, ScanReason
/* The value is not available yet. */
return;
}
JavaConstant fieldValue = bb.getUniverse().getHeapScanner().readFieldValue(field, receiver);
assert isUnwrapped(receiver);

JavaConstant fieldValue = readFieldValue(field, receiver);
if (fieldValue == null) {
StringBuilder backtrace = new StringBuilder();
buildObjectBacktrace(bb, reason, backtrace);
Expand All @@ -179,20 +181,47 @@ protected void scanField(AnalysisField field, JavaConstant receiver, ScanReason
* referenced elements are being scanned.
*/
scanConstant(fieldValue, reason);
} else if (fieldValue.getJavaKind().isNumericInteger()) {
scanningObserver.forPrimitiveFieldValue(receiver, field, fieldValue, reason);
}

} catch (UnsupportedFeatureException ex) {
unsupportedFeatureDuringFieldScan(bb, field, receiver, ex, reason);
}
}

protected JavaConstant readFieldValue(AnalysisField field, JavaConstant receiver) {
return bb.getConstantReflectionProvider().readFieldValue(field, receiver);
}

/**
* Must unwrap the receiver if it is an ImageHeapConstant to scan the hosted value, if any, for
* verification, otherwise the verification just compares shadow heap with shadow heap for
* embedded roots, which is completely useless.
*/
private static JavaConstant maybeUnwrap(JavaConstant receiver) {
if (receiver instanceof ImageHeapConstant heapConstant && heapConstant.getHostedObject() != null) {
return heapConstant.getHostedObject();
}
return receiver;
}

private static boolean isUnwrapped(JavaConstant receiver) {
if (receiver instanceof ImageHeapConstant heapConstant) {
// Non hosted backed ImageHeapConstant is considered unwrapped
return heapConstant.getHostedObject() == null;
}
return true;
}

/**
* Scans constant arrays, one element at the time.
*
* @param array the array to be scanned
*/
protected final void scanArray(JavaConstant array, ScanReason prevReason) {

assert isUnwrapped(array);
AnalysisType arrayType = bb.getMetaAccess().lookupJavaType(array);
ScanReason reason = new ArrayScan(arrayType, array, prevReason);

Expand Down Expand Up @@ -246,13 +275,14 @@ public void scanConstant(JavaConstant value, ScanReason reason) {
bb.registerTypeAsInHeap(bb.getMetaAccess().lookupJavaType(value), reason);
return;
}
Object valueObj = (value instanceof ImageHeapConstant) ? value : constantAsObject(bb, value);
JavaConstant unwrappedValue = maybeUnwrap(value);
Object valueObj = unwrappedValue instanceof ImageHeapConstant ? unwrappedValue : constantAsObject(bb, unwrappedValue);
if (scannedObjects.putAndAcquire(valueObj) == null) {
try {
scanningObserver.forScannedConstant(value, reason);
scanningObserver.forScannedConstant(unwrappedValue, reason);
} finally {
scannedObjects.release(valueObj);
WorklistEntry worklistEntry = new WorklistEntry(value, reason);
WorklistEntry worklistEntry = new WorklistEntry(unwrappedValue, reason);
if (executor != null) {
executor.execute(debug -> doScan(worklistEntry));
} else {
Expand Down Expand Up @@ -339,12 +369,22 @@ public static String asString(BigBang bb, JavaConstant constant, boolean appendT
return "null";
}
AnalysisType type = bb.getMetaAccess().lookupJavaType(constant);
if (constant instanceof ImageHeapConstant) {
// Checkstyle: allow Class.getSimpleName
return constant.getClass().getSimpleName() + "<" + type.toJavaName() + ">";
// Checkstyle: disallow Class.getSimpleName
JavaConstant hosted = constant;
if (constant instanceof ImageHeapConstant heapConstant) {
JavaConstant hostedObject = heapConstant.getHostedObject();
if (hostedObject == null) {
// Checkstyle: allow Class.getSimpleName
return constant.getClass().getSimpleName() + "<" + type.toJavaName() + ">";
// Checkstyle: disallow Class.getSimpleName
}
hosted = hostedObject;
}

if (hosted.getJavaKind().isPrimitive()) {
return hosted.toValueString();
}
Object obj = constantAsObject(bb, constant);

Object obj = constantAsObject(bb, hosted);
String str = type.toJavaName() + '@' + Integer.toHexString(System.identityHashCode(obj));
if (appendToString) {
try {
Expand Down Expand Up @@ -381,7 +421,7 @@ private void doScan(WorklistEntry entry) {
/* Scan constant's instance fields. */
for (ResolvedJavaField javaField : type.getInstanceFields(true)) {
AnalysisField field = (AnalysisField) javaField;
if (field.getJavaKind() == JavaKind.Object && field.isRead()) {
if (field.isRead()) {
assert !Modifier.isStatic(field.getModifiers());
scanField(field, entry.constant, entry.reason);
}
Expand Down Expand Up @@ -589,15 +629,21 @@ public String toString() {

public static class ArrayScan extends ScanReason {
final AnalysisType arrayType;
final int idx;

public ArrayScan(AnalysisType arrayType, JavaConstant array, ScanReason previous) {
this(arrayType, array, previous, -1);
}

public ArrayScan(AnalysisType arrayType, JavaConstant array, ScanReason previous, int idx) {
super(previous, array);
this.arrayType = arrayType;
this.idx = idx;
}

@Override
public String toString(BigBang bb) {
return "indexing into array " + asString(bb, constant);
return "indexing into array " + asString(bb, constant) + (idx != -1 ? " at index " + idx : "");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ default boolean forRelocatedPointerFieldValue(JavaConstant receiver, AnalysisFie
return false;
}

/**
* Hook for scanned value of primitive field.
*/
default boolean forPrimitiveFieldValue(JavaConstant receiver, AnalysisField field, JavaConstant fieldValue, ScanReason reason) {
return false;
}

/**
* Hook for scanned null field value.
*/
Expand Down
Loading

0 comments on commit 28a4718

Please sign in to comment.