Skip to content

Commit

Permalink
review rework
Browse files Browse the repository at this point in the history
  • Loading branch information
@adinn
adinn committed Jul 18, 2022
1 parent 70de7ff commit f1b32e7
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 51 deletions.
6 changes: 3 additions & 3 deletions substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ClassEntry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,13 @@ public DebugTypeKind typeKind() {

@Override
public void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInfo, DebugContext debugContext) {
assert TypeEntry.canonicalize(debugTypeInfo.typeName()).equals(typeName);
assert debugTypeInfo.typeName().equals(typeName);
DebugInstanceTypeInfo debugInstanceTypeInfo = (DebugInstanceTypeInfo) debugTypeInfo;
/* Add details of super and interface classes */
ResolvedJavaType superType = debugInstanceTypeInfo.superClass();
String superName;
if (superType != null) {
superName = TypeEntry.canonicalize(superType.toJavaName());
superName = superType.toJavaName();
} else {
superName = "";
}
Expand Down Expand Up @@ -291,7 +291,7 @@ private void processInterface(ResolvedJavaType interfaceType, DebugInfoBase debu
protected MethodEntry processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
String methodName = debugMethodInfo.name();
ResolvedJavaType resultType = debugMethodInfo.valueType();
String resultTypeName = TypeEntry.canonicalize(resultType.toJavaName());
String resultTypeName = resultType.toJavaName();
int modifiers = debugMethodInfo.modifiers();
DebugLocalInfo[] paramInfos = debugMethodInfo.getParamInfo();
DebugLocalInfo thisParam = debugMethodInfo.getThisParamInfo();
Expand Down
12 changes: 6 additions & 6 deletions ...tratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/DebugInfoBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
/* Create all the types. */
debugInfoProvider.typeInfoProvider().forEach(debugTypeInfo -> debugTypeInfo.debugContext((debugContext) -> {
ResolvedJavaType idType = debugTypeInfo.idType();
String typeName = TypeEntry.canonicalize(debugTypeInfo.typeName());
String typeName = debugTypeInfo.typeName();
typeName = stringTable.uniqueDebugString(typeName);
DebugTypeKind typeKind = debugTypeInfo.typeKind();
int byteSize = debugTypeInfo.size();
Expand All @@ -246,7 +246,7 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
/* Now we can cross reference static and instance field details. */
debugInfoProvider.typeInfoProvider().forEach(debugTypeInfo -> debugTypeInfo.debugContext((debugContext) -> {
ResolvedJavaType idType = debugTypeInfo.idType();
String typeName = TypeEntry.canonicalize(debugTypeInfo.typeName());
String typeName = debugTypeInfo.typeName();
DebugTypeKind typeKind = debugTypeInfo.typeKind();

debugContext.log(DebugContext.INFO_LEVEL, "Process %s type %s ", typeKind.toString(), typeName);
Expand Down Expand Up @@ -348,15 +348,15 @@ private TypeEntry addTypeEntry(ResolvedJavaType idType, String typeName, String
public TypeEntry lookupTypeEntry(ResolvedJavaType type) {
TypeEntry typeEntry = typesIndex.get(type);
if (typeEntry == null) {
throw new RuntimeException("type entry not found " + TypeEntry.canonicalize(type.getName()));
throw new RuntimeException("type entry not found " + type.getName());
}
return typeEntry;
}

ClassEntry lookupClassEntry(ResolvedJavaType type) {
TypeEntry typeEntry = typesIndex.get(type);
if (typeEntry == null || !(typeEntry.isClass())) {
throw new RuntimeException("class entry not found " + TypeEntry.canonicalize(type.getName()));
throw new RuntimeException("class entry not found " + type.getName());
}
return (ClassEntry) typeEntry;
}
Expand Down Expand Up @@ -386,7 +386,7 @@ private Range addSubrange(DebugLocationInfo locationInfo, Range primaryRange, Cl
DebugLocationInfo callerLocationInfo = locationInfo.getCaller();
boolean isTopLevel = callerLocationInfo == null;
assert (!isTopLevel || (locationInfo.name().equals(primaryRange.getMethodName()) &&
TypeEntry.canonicalize(locationInfo.ownerType().toJavaName()).equals(primaryRange.getClassName())));
locationInfo.ownerType().toJavaName().equals(primaryRange.getClassName())));
Range caller = (isTopLevel ? primaryRange : subRangeIndex.get(callerLocationInfo));
// the frame tree is walked topdown so inline ranges should always have a caller range
assert caller != null;
Expand Down Expand Up @@ -428,7 +428,7 @@ private ClassEntry ensureClassEntry(ResolvedJavaType type) {
primaryClasses.add(classEntry);
primaryClassesIndex.put(type, classEntry);
}
assert (classEntry.getTypeName().equals(TypeEntry.canonicalize(type.toJavaName())));
assert (classEntry.getTypeName().equals(type.toJavaName()));
return classEntry;
}

Expand Down
2 changes: 1 addition & 1 deletion ...atevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/HeaderTypeEntry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public DebugTypeKind typeKind() {

@Override
public void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInfo, DebugContext debugContext) {
assert TypeEntry.canonicalize(debugTypeInfo.typeName()).equals(typeName);
assert debugTypeInfo.typeName().equals(typeName);
DebugHeaderTypeInfo debugHeaderTypeInfo = (DebugHeaderTypeInfo) debugTypeInfo;
debugHeaderTypeInfo.fieldInfoProvider().forEach(debugFieldInfo -> this.processField(debugFieldInfo, debugInfoBase, debugContext));
}
Expand Down
2 changes: 1 addition & 1 deletion ...vm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/StructureTypeEntry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ protected void processField(DebugFieldInfo debugFieldInfo, DebugInfoBase debugIn
protected FieldEntry addField(DebugFieldInfo debugFieldInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
String fieldName = debugInfoBase.uniqueDebugString(debugFieldInfo.name());
ResolvedJavaType valueType = debugFieldInfo.valueType();
String valueTypeName = TypeEntry.canonicalize(valueType.toJavaName());
String valueTypeName = valueType.toJavaName();
int fieldSize = debugFieldInfo.size();
int fieldoffset = debugFieldInfo.offset();
int fieldModifiers = debugFieldInfo.modifiers();
Expand Down
4 changes: 0 additions & 4 deletions substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/TypeEntry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,4 @@ public boolean isStructure() {
}

public abstract void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInfo, DebugContext debugContext);

public static String canonicalize(String typeName) {
return typeName.replace(" ", "__");
}
}
15 changes: 5 additions & 10 deletions ...m/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfInfoSectionImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,6 @@ private int writeHeaderField(DebugContext context, FieldEntry fieldEntry, byte[]
int pos = p;
String fieldName = fieldEntry.fieldName();
TypeEntry valueType = fieldEntry.getValueType();
String valueTypeName = valueType.getTypeName();
/* use the indirect type for the field so pointers get translated */
int valueTypeIdx = getIndirectTypeIndex(valueType);
log(context, " [0x%08x] header field", pos);
Expand All @@ -318,7 +317,7 @@ private int writeHeaderField(DebugContext context, FieldEntry fieldEntry, byte[]
pos = writeAbbrevCode(abbrevCode, buffer, pos);
log(context, " [0x%08x] name 0x%x (%s)", pos, debugStringIndex(fieldName), fieldName);
pos = writeAttrStrp(fieldName, buffer, pos);
log(context, " [0x%08x] type 0x%x (%s)", pos, valueTypeIdx, valueTypeName);
log(context, " [0x%08x] type 0x%x (%s)", pos, valueTypeIdx, valueType.getTypeName());
pos = writeAttrRefAddr(valueTypeIdx, buffer, pos);
byte offset = (byte) fieldEntry.getOffset();
int size = fieldEntry.getSize();
Expand Down Expand Up @@ -627,10 +626,9 @@ private int writeField(DebugContext context, StructureTypeEntry entry, FieldEntr
/* At present we definitely don't have line numbers. */
}
TypeEntry valueType = fieldEntry.getValueType();
String valueTypeName = valueType.getTypeName();
/* use the indirect type for the field so pointers get translated if needed */
int typeIdx = getIndirectTypeIndex(valueType);
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, valueTypeName);
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, valueType.getTypeName());
pos = writeAttrRefAddr(typeIdx, buffer, pos);
if (!isStatic) {
int memberOffset = fieldEntry.getOffset();
Expand Down Expand Up @@ -688,9 +686,8 @@ private int writeMethodDeclaration(DebugContext context, ClassEntry classEntry,
log(context, " [0x%08x] file 0x%x (%s)", pos, fileIdx, fileEntry.getFullName());
pos = writeAttrData2((short) fileIdx, buffer, pos);
TypeEntry returnType = method.getValueType();
String returnTypeName = returnType.getTypeName();
int retTypeIdx = getTypeIndex(returnType);
log(context, " [0x%08x] type 0x%x (%s)", pos, retTypeIdx, returnTypeName);
log(context, " [0x%08x] type 0x%x (%s)", pos, retTypeIdx, returnType.getTypeName());
pos = writeAttrRefAddr(retTypeIdx, buffer, pos);
log(context, " [0x%08x] artificial %s", pos, method.isDeopt() ? "true" : "false");
pos = writeFlag((method.isDeopt() ? (byte) 1 : (byte) 0), buffer, pos);
Expand Down Expand Up @@ -749,7 +746,6 @@ private int writeMethodParameterDeclaration(DebugContext context, DebugLocalInfo
int abbrevCode;
String paramName = paramInfo.name();
TypeEntry paramType = lookupType(paramInfo.valueType());
String paramTypeName = paramType.getTypeName();
int line = paramInfo.line();
if (artificial) {
abbrevCode = DwarfDebugInfo.DW_ABBREV_CODE_method_parameter_declaration1;
Expand All @@ -769,7 +765,7 @@ private int writeMethodParameterDeclaration(DebugContext context, DebugLocalInfo
pos = writeAttrData2((short) line, buffer, pos);
}
int typeIdx = getTypeIndex(paramType);
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, paramTypeName);
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, paramType.getTypeName());
pos = writeAttrRefAddr(typeIdx, buffer, pos);
if (abbrevCode == DwarfDebugInfo.DW_ABBREV_CODE_method_parameter_declaration1) {
log(context, " [0x%08x] artificial true", pos);
Expand Down Expand Up @@ -799,7 +795,6 @@ private int writeMethodLocalDeclaration(DebugContext context, DebugLocalInfo par
int abbrevCode;
String paramName = paramInfo.name();
TypeEntry paramType = lookupType(paramInfo.valueType());
String paramTypeName = paramType.getTypeName();
int line = paramInfo.line();
if (line >= 0) {
abbrevCode = DwarfDebugInfo.DW_ABBREV_CODE_method_local_declaration1;
Expand All @@ -817,7 +812,7 @@ private int writeMethodLocalDeclaration(DebugContext context, DebugLocalInfo par
pos = writeAttrData2((short) line, buffer, pos);
}
int typeIdx = getTypeIndex(paramType);
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, paramTypeName);
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, paramType.getTypeName());
pos = writeAttrRefAddr(typeIdx, buffer, pos);
log(context, " [0x%08x] declaration true", pos);
pos = writeFlag((byte) 1, buffer, pos);
Expand Down
63 changes: 37 additions & 26 deletions ...c/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@
import com.oracle.svm.core.heap.ObjectHeader;
import com.oracle.svm.core.image.ImageHeapPartition;
import com.oracle.svm.core.meta.SubstrateObjectConstant;
// import com.oracle.svm.hosted.annotation.CustomSubstitutionMethod;
import com.oracle.svm.hosted.annotation.CustomSubstitutionType;
import com.oracle.svm.hosted.image.NativeImageHeap.ObjectInfo;
import com.oracle.svm.hosted.image.sources.SourceManager;
Expand Down Expand Up @@ -168,17 +167,18 @@ class NativeImageDebugInfoProvider implements DebugInfoProvider {
private static HashMap<JavaKind, HostedType> initJavaKindToHostedTypes(HostedMetaAccess metaAccess) {
HashMap<JavaKind, HostedType> map = new HashMap<>();
for (JavaKind kind : JavaKind.values()) {
Class<?> clazz = kind.toJavaClass();
assert clazz != null || kind == JavaKind.Illegal || kind == JavaKind.Object;
HostedType javaType;
if (kind == JavaKind.Object) {
clazz = java.lang.Object.class;
}
if (clazz == null) {
javaType = null;
} else {
javaType = metaAccess.lookupJavaType(clazz);
Class<?> clazz;
switch (kind) {
case Illegal:
clazz = null;
break;
case Object:
clazz = java.lang.Object.class;
break;
default:
clazz = kind.toJavaClass();
}
HostedType javaType = clazz != null ? metaAccess.lookupJavaType(clazz) : null;
map.put(kind, javaType);
}
return map;
Expand Down Expand Up @@ -283,7 +283,7 @@ protected static ResolvedJavaType getDeclaringClass(HostedMethod hostedMethod, b
}
// we want a substituted target if there is one. if there is a substitution at the end of
// the method chain fetch the annotated target class
ResolvedJavaMethod javaMethod = getOriginal(hostedMethod);
ResolvedJavaMethod javaMethod = getAnnotatedOrOriginal(hostedMethod);
return javaMethod.getDeclaringClass();
}

Expand All @@ -308,19 +308,32 @@ private static ResolvedJavaType getOriginal(HostedType hostedType) {
return javaType;
}

private static ResolvedJavaMethod getOriginal(HostedMethod hostedMethod) {
private static ResolvedJavaMethod getAnnotatedOrOriginal(HostedMethod hostedMethod) {
ResolvedJavaMethod javaMethod = hostedMethod.getWrapped().getWrapped();
// This method is only used when identifying the modifiers or the declaring class
// of a HostedMethod. Normally the method unwraps to the underlying JVMCI method
// which is the one that provides bytecode to the compiler as well as, line numbers
// and local info. If we unwrap to a SubstitutionMethod then we use the annotated
// method, not the JVMCI method that the annotation refers to since that will be the
// one providing the bytecode etc used by the compiler. If we unwrap to any other,
// custom substitution method we simply use it rather than dereferencing to the
// original. The difference is that the annotated method's bytecode will be used to
// replace the original and the debugger needs to use it to identify the file and access
// permissions. A custom substitution may exist alongside the original, as is the case
// with some uses for reflection. So, we don't want to conflate the custom substituted
// method and the original. In this latter case the method code will be synthesized without
// reference to the bytecode of the original. Hence there is no associated file and the
// permissions need to be determined from the custom substitution method itself.

if (javaMethod instanceof SubstitutionMethod) {
SubstitutionMethod substitutionMethod = (SubstitutionMethod) javaMethod;
javaMethod = substitutionMethod.getAnnotated();
// } else if (javaMethod instanceof CustomSubstitutionMethod) {
// javaMethod = ((CustomSubstitutionMethod) javaMethod).getOriginal();
}
return javaMethod;
}

private static int getOriginalModifiers(HostedMethod hostedMethod) {
return getOriginal(hostedMethod).getModifiers();
return getAnnotatedOrOriginal(hostedMethod).getModifiers();
}

private final Path cachePath = SubstrateOptions.getDebugInfoSourceCacheRoot();
Expand Down Expand Up @@ -656,12 +669,11 @@ public Stream<DebugMethodInfo> methodInfoProvider() {
public ResolvedJavaType superClass() {
HostedClass superClass = hostedType.getSuperclass();
/*
* HostedType wraps an AnalysisType and both HostedType and AnalysisType punt calls to
* getSourceFilename to the wrapped class so for consistency we need to do the path
* lookup relative to the doubly unwrapped HostedType.
* Unwrap the hosted type's super class to the original to provide the correct identity
* type.
*/
if (superClass != null) {
return getDeclaringClass(superClass, true);
return getOriginal(superClass);
}
return null;
}
Expand Down Expand Up @@ -922,12 +934,10 @@ private ResolvedJavaMethod originalMethod() {
while (targetMethod instanceof WrappedJavaMethod) {
targetMethod = ((WrappedJavaMethod) targetMethod).getWrapped();
}
// if we hit these two substitutions then we can translate to the original
// if we hit a substitution then we can translate to the original
// for identity otherwise we use whatever we unwrapped to.
if (targetMethod instanceof SubstitutionMethod) {
targetMethod = ((SubstitutionMethod) targetMethod).getOriginal();
// } else if (targetMethod instanceof CustomSubstitutionMethod) {
// targetMethod = ((CustomSubstitutionMethod) targetMethod).getOriginal();
}
return targetMethod;
}
Expand Down Expand Up @@ -2128,14 +2138,14 @@ public class NativeImageDebugLocalValueInfo implements DebugLocalValueInfo {
this(name, Value.ILLEGAL, 0, kind, type, slot, line);
}

NativeImageDebugLocalValueInfo(String name, JavaValue value, int framesize, JavaKind kind, ResolvedJavaType t, int slot, int line) {
NativeImageDebugLocalValueInfo(String name, JavaValue value, int framesize, JavaKind kind, ResolvedJavaType resolvedType, int slot, int line) {
this.name = name;
this.kind = kind;
this.slot = slot;
this.line = line;
// if we don't have a type default it for the JavaKind
// it may still end up null when kind is Undefined.
this.type = (t != null ? t : hostedTypeForKind(kind));
this.type = (resolvedType != null ? resolvedType : hostedTypeForKind(kind));
if (value instanceof RegisterValue) {
this.localKind = LocalKind.REGISTER;
this.value = new NativeImageDebugRegisterValue((RegisterValue) value);
Expand Down Expand Up @@ -2230,7 +2240,8 @@ public String name() {

@Override
public String typeName() {
return valueType().toJavaName();
ResolvedJavaType valueType = valueType();
return (valueType == null ? "" : valueType().toJavaName());
}

@Override
Expand Down

0 comments on commit f1b32e7

Please sign in to comment.