Skip to content

Ensure compiled methods are presented in ascending address order. #6300

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

Closed
wants to merge 1 commit into from
Closed
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 @@ -460,4 +460,65 @@ public int hipcDeopt() {
assert hasDeoptCompiledEntries();
return deoptCompiledEntries.get(deoptCompiledEntries.size() - 1).getPrimary().getHi();
}

/**
* Verify that the normal and deopt entries lists have each been presented in order of ascending
* address range and that all deopt methods are in a higher range than all non-deopt methods.
*
* @return false if any method is out of order otherwise true.
*/
boolean verifyCompiledMethodOrder() {
int lo = 0;
for (CompiledMethodEntry c : normalCompiledEntries) {
int next = c.getPrimary().getLo();
if (next < lo) {
assert false : "invalid method address order %s 0x%x -> 0x%x".formatted(c.getPrimary().getFullMethodNameWithParams(), lo, next);
return false;
}
lo = next;
}
if (deoptCompiledEntries != null) {
for (CompiledMethodEntry c : deoptCompiledEntries) {
int next = c.getPrimary().getLo();
if (next < lo) {
assert false : "invalid method address order %s 0x%x -> 0x%x".formatted(c.getPrimary().getFullMethodNameWithParams(), lo, next);
return false;
}
lo = next;
}
}
return true;
}

public boolean verifyCompiledMethodRange(ClassEntry other) {
boolean check = lowpc() >= other.hipc() || hipc() <= other.lowpc();
if (!check) {
assert false : "detected address range overlap %s [0x%x,0x%x], %s [0x%x,0x%x]".formatted(getTypeName(), lowpc(), hipc(), other.getTypeName(), other.lowpc(), other.hipc());
return false;
}
if (hasDeoptCompiledEntries()) {
check = lowpcDeopt() >= other.hipc();
if (!check) {
assert false : "detected deopt vs normal address range overlap %s [0x%x,0x%x], %s [0x%x,0x%x]".formatted(getTypeName(), lowpcDeopt(), hipcDeopt(), other.getTypeName(), other.lowpc(),
other.hipc());
return false;
}
if (other.hasDeoptCompiledEntries()) {
check = lowpcDeopt() >= other.hipcDeopt() || other.lowpcDeopt() >= hipcDeopt();
if (!check) {
assert false : "detected deopt vs deopt address range overlap %s [0x%x,0x%x], %s [0x%x,0x%x]".formatted(getTypeName(), lowpcDeopt(), hipcDeopt(), other.getTypeName(),
other.lowpcDeopt(), other.hipcDeopt());
return false;
}
}
} else if (other.hasDeoptCompiledEntries()) {
check = hipc() <= other.lowpcDeopt();
if (!check) {
assert false : "detected normal vs deopt address range overlap %s [0x%x,0x%x], %s [0x%x,0x%x]".formatted(getTypeName(), lowpc(), hipc(), other.getTypeName(), other.lowpcDeopt(),
other.hipcDeopt());
return false;
}
}
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -315,13 +315,16 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
PrimaryRange primaryRange = Range.createPrimary(methodEntry, lo, hi, primaryLine);
debugContext.log(DebugContext.INFO_LEVEL, "PrimaryRange %s.%s %s %s:%d [0x%x, 0x%x]", ownerType.toJavaName(), methodName, filePath, fileName, primaryLine, lo, hi);
classEntry.indexPrimary(primaryRange, debugCodeInfo.getFrameSizeChanges(), debugCodeInfo.getFrameSize());
// verify that compiled methods have been recorded in ascending address order
assert classEntry.verifyCompiledMethodOrder() : "compiled methods must be presented in ascending order";
/*
* Record all subranges even if they have no line or file so we at least get a symbol
* for them and don't see a break in the address range.
*/
EconomicMap<DebugLocationInfo, SubRange> subRangeIndex = EconomicMap.create();
debugCodeInfo.locationInfoProvider().forEach(debugLocationInfo -> addSubrange(debugLocationInfo, primaryRange, classEntry, subRangeIndex, debugContext));
}));
assert verifyCompiledMethodRanges() : "each class must have its own unique compiled method code address range";

debugInfoProvider.dataInfoProvider().forEach(debugDataInfo -> debugDataInfo.debugContext((debugContext) -> {
String provenance = debugDataInfo.getProvenance();
Expand Down Expand Up @@ -670,4 +673,33 @@ public int classLayoutAbbrevCode(ClassEntry classEntry) {
public ClassEntry getHubClassEntry() {
return hubClassEntry;
}

/**
* Check that each class's compiled full address range is unique to that class.
*
* Specifically, verify that for any two instance classes A and B whose compiled methods sets
* {ma_1, ... ma_k} and {mb_1, ... mb_l} lie in the respective, minimal, enclosing address
* ranges [a_lo,a_hi] and [b_lo,b_hi] the following constraint is met:
*
* [a_lo,a_hi] intersect [b_lo,b_hi] == empty.
*/
private boolean verifyCompiledMethodRanges() {
for (ClassEntry classEntry : getInstanceClasses()) {
if (!classEntry.hasCompiledEntries()) {
continue;
}
for (ClassEntry otherClassEntry : getInstanceClasses()) {
if (classEntry == otherClassEntry) {
break;
}
if (!otherClassEntry.hasCompiledEntries()) {
continue;
}
if (!classEntry.verifyCompiledMethodRange(otherClassEntry)) {
return false;
}
}
}
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,12 @@ public Stream<DebugTypeInfo> typeInfoProvider() {

@Override
public Stream<DebugCodeInfo> codeInfoProvider() {
return codeCache.getOrderedCompilations().stream().map(pair -> new NativeImageDebugCodeInfo(pair.getLeft(), pair.getRight()));
// despite what it says on the tin method getOrderedCompilations may not actually present
// compiled methods in ascending address range order, whether globally or within a class.
// so we need to sort them here
return codeCache.getOrderedCompilations().stream()
.map(pair -> (DebugCodeInfo) new NativeImageDebugCodeInfo(pair.getLeft(), pair.getRight()))
.sorted((c1, c2) -> (c1.addressLo() - c2.addressLo()));
}

@Override
Expand Down