Skip to content

Commit e79a4b7

Browse files
adinnadinn
committed
review rework
1 parent 1b2bfb8 commit e79a4b7

File tree

7 files changed

+53
-51
lines changed

7 files changed

+53
-51
lines changed

‎substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ClassEntry.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,13 @@ public DebugTypeKind typeKind() {
130130

131131
@Override
132132
public void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInfo, DebugContext debugContext) {
133-
assert TypeEntry.canonicalize(debugTypeInfo.typeName()).equals(typeName);
133+
assert debugTypeInfo.typeName().equals(typeName);
134134
DebugInstanceTypeInfo debugInstanceTypeInfo = (DebugInstanceTypeInfo) debugTypeInfo;
135135
/* Add details of super and interface classes */
136136
ResolvedJavaType superType = debugInstanceTypeInfo.superClass();
137137
String superName;
138138
if (superType != null) {
139-
superName = TypeEntry.canonicalize(superType.toJavaName());
139+
superName = superType.toJavaName();
140140
} else {
141141
superName = "";
142142
}
@@ -291,7 +291,7 @@ private void processInterface(ResolvedJavaType interfaceType, DebugInfoBase debu
291291
protected MethodEntry processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
292292
String methodName = debugMethodInfo.name();
293293
ResolvedJavaType resultType = debugMethodInfo.valueType();
294-
String resultTypeName = TypeEntry.canonicalize(resultType.toJavaName());
294+
String resultTypeName = resultType.toJavaName();
295295
int modifiers = debugMethodInfo.modifiers();
296296
DebugLocalInfo[] paramInfos = debugMethodInfo.getParamInfo();
297297
DebugLocalInfo thisParam = debugMethodInfo.getThisParamInfo();

‎substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/DebugInfoBase.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
231231
/* Create all the types. */
232232
debugInfoProvider.typeInfoProvider().forEach(debugTypeInfo -> debugTypeInfo.debugContext((debugContext) -> {
233233
ResolvedJavaType idType = debugTypeInfo.idType();
234-
String typeName = TypeEntry.canonicalize(debugTypeInfo.typeName());
234+
String typeName = debugTypeInfo.typeName();
235235
typeName = stringTable.uniqueDebugString(typeName);
236236
DebugTypeKind typeKind = debugTypeInfo.typeKind();
237237
int byteSize = debugTypeInfo.size();
@@ -246,7 +246,7 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
246246
/* Now we can cross reference static and instance field details. */
247247
debugInfoProvider.typeInfoProvider().forEach(debugTypeInfo -> debugTypeInfo.debugContext((debugContext) -> {
248248
ResolvedJavaType idType = debugTypeInfo.idType();
249-
String typeName = TypeEntry.canonicalize(debugTypeInfo.typeName());
249+
String typeName = debugTypeInfo.typeName();
250250
DebugTypeKind typeKind = debugTypeInfo.typeKind();
251251

252252
debugContext.log(DebugContext.INFO_LEVEL, "Process %s type %s ", typeKind.toString(), typeName);
@@ -348,15 +348,15 @@ private TypeEntry addTypeEntry(ResolvedJavaType idType, String typeName, String
348348
public TypeEntry lookupTypeEntry(ResolvedJavaType type) {
349349
TypeEntry typeEntry = typesIndex.get(type);
350350
if (typeEntry == null) {
351-
throw new RuntimeException("type entry not found " + TypeEntry.canonicalize(type.getName()));
351+
throw new RuntimeException("type entry not found " + type.getName());
352352
}
353353
return typeEntry;
354354
}
355355

356356
ClassEntry lookupClassEntry(ResolvedJavaType type) {
357357
TypeEntry typeEntry = typesIndex.get(type);
358358
if (typeEntry == null || !(typeEntry.isClass())) {
359-
throw new RuntimeException("class entry not found " + TypeEntry.canonicalize(type.getName()));
359+
throw new RuntimeException("class entry not found " + type.getName());
360360
}
361361
return (ClassEntry) typeEntry;
362362
}
@@ -386,7 +386,7 @@ private Range addSubrange(DebugLocationInfo locationInfo, Range primaryRange, Cl
386386
DebugLocationInfo callerLocationInfo = locationInfo.getCaller();
387387
boolean isTopLevel = callerLocationInfo == null;
388388
assert (!isTopLevel || (locationInfo.name().equals(primaryRange.getMethodName()) &&
389-
TypeEntry.canonicalize(locationInfo.ownerType().toJavaName()).equals(primaryRange.getClassName())));
389+
locationInfo.ownerType().toJavaName().equals(primaryRange.getClassName())));
390390
Range caller = (isTopLevel ? primaryRange : subRangeIndex.get(callerLocationInfo));
391391
// the frame tree is walked topdown so inline ranges should always have a caller range
392392
assert caller != null;
@@ -428,7 +428,7 @@ private ClassEntry ensureClassEntry(ResolvedJavaType type) {
428428
primaryClasses.add(classEntry);
429429
primaryClassesIndex.put(type, classEntry);
430430
}
431-
assert (classEntry.getTypeName().equals(TypeEntry.canonicalize(type.toJavaName())));
431+
assert (classEntry.getTypeName().equals(type.toJavaName()));
432432
return classEntry;
433433
}
434434

‎substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/HeaderTypeEntry.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public DebugTypeKind typeKind() {
4545

4646
@Override
4747
public void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInfo, DebugContext debugContext) {
48-
assert TypeEntry.canonicalize(debugTypeInfo.typeName()).equals(typeName);
48+
assert debugTypeInfo.typeName().equals(typeName);
4949
DebugHeaderTypeInfo debugHeaderTypeInfo = (DebugHeaderTypeInfo) debugTypeInfo;
5050
debugHeaderTypeInfo.fieldInfoProvider().forEach(debugFieldInfo -> this.processField(debugFieldInfo, debugInfoBase, debugContext));
5151
}

‎substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/StructureTypeEntry.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ protected void processField(DebugFieldInfo debugFieldInfo, DebugInfoBase debugIn
6262
protected FieldEntry addField(DebugFieldInfo debugFieldInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
6363
String fieldName = debugInfoBase.uniqueDebugString(debugFieldInfo.name());
6464
ResolvedJavaType valueType = debugFieldInfo.valueType();
65-
String valueTypeName = TypeEntry.canonicalize(valueType.toJavaName());
65+
String valueTypeName = valueType.toJavaName();
6666
int fieldSize = debugFieldInfo.size();
6767
int fieldoffset = debugFieldInfo.offset();
6868
int fieldModifiers = debugFieldInfo.modifiers();

‎substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/TypeEntry.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,4 @@ public boolean isStructure() {
9696
}
9797

9898
public abstract void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInfo, DebugContext debugContext);
99-
100-
public static String canonicalize(String typeName) {
101-
return typeName.replace(" ", "__");
102-
}
10399
}

‎substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfInfoSectionImpl.java‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,6 @@ private int writeHeaderField(DebugContext context, FieldEntry fieldEntry, byte[]
309309
int pos = p;
310310
String fieldName = fieldEntry.fieldName();
311311
TypeEntry valueType = fieldEntry.getValueType();
312-
String valueTypeName = valueType.getTypeName();
313312
/* use the indirect type for the field so pointers get translated */
314313
int valueTypeIdx = getIndirectTypeIndex(valueType);
315314
log(context, " [0x%08x] header field", pos);
@@ -318,7 +317,7 @@ private int writeHeaderField(DebugContext context, FieldEntry fieldEntry, byte[]
318317
pos = writeAbbrevCode(abbrevCode, buffer, pos);
319318
log(context, " [0x%08x] name 0x%x (%s)", pos, debugStringIndex(fieldName), fieldName);
320319
pos = writeAttrStrp(fieldName, buffer, pos);
321-
log(context, " [0x%08x] type 0x%x (%s)", pos, valueTypeIdx, valueTypeName);
320+
log(context, " [0x%08x] type 0x%x (%s)", pos, valueTypeIdx, valueType.getTypeName());
322321
pos = writeAttrRefAddr(valueTypeIdx, buffer, pos);
323322
byte offset = (byte) fieldEntry.getOffset();
324323
int size = fieldEntry.getSize();
@@ -627,10 +626,9 @@ private int writeField(DebugContext context, StructureTypeEntry entry, FieldEntr
627626
/* At present we definitely don't have line numbers. */
628627
}
629628
TypeEntry valueType = fieldEntry.getValueType();
630-
String valueTypeName = valueType.getTypeName();
631629
/* use the indirect type for the field so pointers get translated if needed */
632630
int typeIdx = getIndirectTypeIndex(valueType);
633-
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, valueTypeName);
631+
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, valueType.getTypeName());
634632
pos = writeAttrRefAddr(typeIdx, buffer, pos);
635633
if (!isStatic) {
636634
int memberOffset = fieldEntry.getOffset();
@@ -688,9 +686,8 @@ private int writeMethodDeclaration(DebugContext context, ClassEntry classEntry,
688686
log(context, " [0x%08x] file 0x%x (%s)", pos, fileIdx, fileEntry.getFullName());
689687
pos = writeAttrData2((short) fileIdx, buffer, pos);
690688
TypeEntry returnType = method.getValueType();
691-
String returnTypeName = returnType.getTypeName();
692689
int retTypeIdx = getTypeIndex(returnType);
693-
log(context, " [0x%08x] type 0x%x (%s)", pos, retTypeIdx, returnTypeName);
690+
log(context, " [0x%08x] type 0x%x (%s)", pos, retTypeIdx, returnType.getTypeName());
694691
pos = writeAttrRefAddr(retTypeIdx, buffer, pos);
695692
log(context, " [0x%08x] artificial %s", pos, method.isDeopt() ? "true" : "false");
696693
pos = writeFlag((method.isDeopt() ? (byte) 1 : (byte) 0), buffer, pos);
@@ -749,7 +746,6 @@ private int writeMethodParameterDeclaration(DebugContext context, DebugLocalInfo
749746
int abbrevCode;
750747
String paramName = paramInfo.name();
751748
TypeEntry paramType = lookupType(paramInfo.valueType());
752-
String paramTypeName = paramType.getTypeName();
753749
int line = paramInfo.line();
754750
if (artificial) {
755751
abbrevCode = DwarfDebugInfo.DW_ABBREV_CODE_method_parameter_declaration1;
@@ -769,7 +765,7 @@ private int writeMethodParameterDeclaration(DebugContext context, DebugLocalInfo
769765
pos = writeAttrData2((short) line, buffer, pos);
770766
}
771767
int typeIdx = getTypeIndex(paramType);
772-
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, paramTypeName);
768+
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, paramType.getTypeName());
773769
pos = writeAttrRefAddr(typeIdx, buffer, pos);
774770
if (abbrevCode == DwarfDebugInfo.DW_ABBREV_CODE_method_parameter_declaration1) {
775771
log(context, " [0x%08x] artificial true", pos);
@@ -799,7 +795,6 @@ private int writeMethodLocalDeclaration(DebugContext context, DebugLocalInfo par
799795
int abbrevCode;
800796
String paramName = paramInfo.name();
801797
TypeEntry paramType = lookupType(paramInfo.valueType());
802-
String paramTypeName = paramType.getTypeName();
803798
int line = paramInfo.line();
804799
if (line >= 0) {
805800
abbrevCode = DwarfDebugInfo.DW_ABBREV_CODE_method_local_declaration1;
@@ -817,7 +812,7 @@ private int writeMethodLocalDeclaration(DebugContext context, DebugLocalInfo par
817812
pos = writeAttrData2((short) line, buffer, pos);
818813
}
819814
int typeIdx = getTypeIndex(paramType);
820-
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, paramTypeName);
815+
log(context, " [0x%08x] type 0x%x (%s)", pos, typeIdx, paramType.getTypeName());
821816
pos = writeAttrRefAddr(typeIdx, buffer, pos);
822817
log(context, " [0x%08x] declaration true", pos);
823818
pos = writeFlag((byte) 1, buffer, pos);

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java‎

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import com.oracle.svm.core.heap.ObjectHeader;
4545
import com.oracle.svm.core.image.ImageHeapPartition;
4646
import com.oracle.svm.core.meta.SubstrateObjectConstant;
47-
// import com.oracle.svm.hosted.annotation.CustomSubstitutionMethod;
4847
import com.oracle.svm.hosted.annotation.CustomSubstitutionType;
4948
import com.oracle.svm.hosted.image.NativeImageHeap.ObjectInfo;
5049
import com.oracle.svm.hosted.image.sources.SourceManager;
@@ -166,17 +165,18 @@ class NativeImageDebugInfoProvider implements DebugInfoProvider {
166165
private static HashMap<JavaKind, HostedType> initJavaKindToHostedTypes(HostedMetaAccess metaAccess) {
167166
HashMap<JavaKind, HostedType> map = new HashMap<>();
168167
for (JavaKind kind : JavaKind.values()) {
169-
Class<?> clazz = kind.toJavaClass();
170-
assert clazz != null || kind == JavaKind.Illegal || kind == JavaKind.Object;
171-
HostedType javaType;
172-
if (kind == JavaKind.Object) {
173-
clazz = java.lang.Object.class;
174-
}
175-
if (clazz == null) {
176-
javaType = null;
177-
} else {
178-
javaType = metaAccess.lookupJavaType(clazz);
168+
Class<?> clazz;
169+
switch (kind) {
170+
case Illegal:
171+
clazz = null;
172+
break;
173+
case Object:
174+
clazz = java.lang.Object.class;
175+
break;
176+
default:
177+
clazz = kind.toJavaClass();
179178
}
179+
HostedType javaType = clazz != null ? metaAccess.lookupJavaType(clazz) : null;
180180
map.put(kind, javaType);
181181
}
182182
return map;
@@ -281,7 +281,7 @@ protected static ResolvedJavaType getDeclaringClass(HostedMethod hostedMethod, b
281281
}
282282
// we want a substituted target if there is one. if there is a substitution at the end of
283283
// the method chain fetch the annotated target class
284-
ResolvedJavaMethod javaMethod = getOriginal(hostedMethod);
284+
ResolvedJavaMethod javaMethod = getAnnotatedOrOriginal(hostedMethod);
285285
return javaMethod.getDeclaringClass();
286286
}
287287

@@ -306,19 +306,32 @@ private static ResolvedJavaType getOriginal(HostedType hostedType) {
306306
return javaType;
307307
}
308308

309-
private static ResolvedJavaMethod getOriginal(HostedMethod hostedMethod) {
309+
private static ResolvedJavaMethod getAnnotatedOrOriginal(HostedMethod hostedMethod) {
310310
ResolvedJavaMethod javaMethod = hostedMethod.getWrapped().getWrapped();
311+
// This method is only used when identifying the modifiers or the declaring class
312+
// of a HostedMethod. Normally the method unwraps to the underlying JVMCI method
313+
// which is the one that provides bytecode to the compiler as well as, line numbers
314+
// and local info. If we unwrap to a SubstitutionMethod then we use the annotated
315+
// method, not the JVMCI method that the annotation refers to since that will be the
316+
// one providing the bytecode etc used by the compiler. If we unwrap to any other,
317+
// custom substitution method we simply use it rather than dereferencing to the
318+
// original. The difference is that the annotated method's bytecode will be used to
319+
// replace the original and the debugger needs to use it to identify the file and access
320+
// permissions. A custom substitution may exist alongside the original, as is the case
321+
// with some uses for reflection. So, we don't want to conflate the custom substituted
322+
// method and the original. In this latter case the method code will be synthesized without
323+
// reference to the bytecode of the original. Hence there is no associated file and the
324+
// permissions need to be determined from the custom substitution method itself.
325+
311326
if (javaMethod instanceof SubstitutionMethod) {
312327
SubstitutionMethod substitutionMethod = (SubstitutionMethod) javaMethod;
313328
javaMethod = substitutionMethod.getAnnotated();
314-
// } else if (javaMethod instanceof CustomSubstitutionMethod) {
315-
// javaMethod = ((CustomSubstitutionMethod) javaMethod).getOriginal();
316329
}
317330
return javaMethod;
318331
}
319332

320333
private static int getOriginalModifiers(HostedMethod hostedMethod) {
321-
return getOriginal(hostedMethod).getModifiers();
334+
return getAnnotatedOrOriginal(hostedMethod).getModifiers();
322335
}
323336

324337
private final Path cachePath = SubstrateOptions.getDebugInfoSourceCacheRoot();
@@ -654,12 +667,11 @@ public Stream<DebugMethodInfo> methodInfoProvider() {
654667
public ResolvedJavaType superClass() {
655668
HostedClass superClass = hostedType.getSuperclass();
656669
/*
657-
* HostedType wraps an AnalysisType and both HostedType and AnalysisType punt calls to
658-
* getSourceFilename to the wrapped class so for consistency we need to do the path
659-
* lookup relative to the doubly unwrapped HostedType.
670+
* Unwrap the hosted type's super class to the original to provide the correct identity
671+
* type.
660672
*/
661673
if (superClass != null) {
662-
return getDeclaringClass(superClass, true);
674+
return getOriginal(superClass);
663675
}
664676
return null;
665677
}
@@ -920,12 +932,10 @@ private ResolvedJavaMethod originalMethod() {
920932
while (targetMethod instanceof WrappedJavaMethod) {
921933
targetMethod = ((WrappedJavaMethod) targetMethod).getWrapped();
922934
}
923-
// if we hit these two substitutions then we can translate to the original
935+
// if we hit a substitution then we can translate to the original
924936
// for identity otherwise we use whatever we unwrapped to.
925937
if (targetMethod instanceof SubstitutionMethod) {
926938
targetMethod = ((SubstitutionMethod) targetMethod).getOriginal();
927-
// } else if (targetMethod instanceof CustomSubstitutionMethod) {
928-
// targetMethod = ((CustomSubstitutionMethod) targetMethod).getOriginal();
929939
}
930940
return targetMethod;
931941
}
@@ -2126,14 +2136,14 @@ public class NativeImageDebugLocalValueInfo implements DebugLocalValueInfo {
21262136
this(name, Value.ILLEGAL, 0, kind, type, slot, line);
21272137
}
21282138

2129-
NativeImageDebugLocalValueInfo(String name, JavaValue value, int framesize, JavaKind kind, ResolvedJavaType t, int slot, int line) {
2139+
NativeImageDebugLocalValueInfo(String name, JavaValue value, int framesize, JavaKind kind, ResolvedJavaType resolvedType, int slot, int line) {
21302140
this.name = name;
21312141
this.kind = kind;
21322142
this.slot = slot;
21332143
this.line = line;
21342144
// if we don't have a type default it for the JavaKind
21352145
// it may still end up null when kind is Undefined.
2136-
this.type = (t != null ? t : hostedTypeForKind(kind));
2146+
this.type = (resolvedType != null ? resolvedType : hostedTypeForKind(kind));
21372147
if (value instanceof RegisterValue) {
21382148
this.localKind = LocalKind.REGISTER;
21392149
this.value = new NativeImageDebugRegisterValue((RegisterValue) value);
@@ -2228,7 +2238,8 @@ public String name() {
22282238

22292239
@Override
22302240
public String typeName() {
2231-
return valueType().toJavaName();
2241+
ResolvedJavaType valueType = valueType();
2242+
return (valueType == null ? "" : valueType().toJavaName());
22322243
}
22332244

22342245
@Override

0 commit comments

Comments
 (0)