From 9ba04e453eb62bea59f7da1dc9f963ab6537b1e7 Mon Sep 17 00:00:00 2001 From: Foivos Zakkak Date: Wed, 7 Oct 2020 03:14:21 +0300 Subject: [PATCH 1/4] DebugInfo: Add test (in debuginfotest) for substituted methods --- substratevm/mx.substratevm/suite.py | 4 +- substratevm/mx.substratevm/testhello.py | 12 +++++- .../Target_hello_Hello_DefaultGreeter.java | 40 +++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 substratevm/src/com.oracle.svm.test/src/hello/Target_hello_Hello_DefaultGreeter.java diff --git a/substratevm/mx.substratevm/suite.py b/substratevm/mx.substratevm/suite.py index 94064f8ddf78..eecaa50341b0 100644 --- a/substratevm/mx.substratevm/suite.py +++ b/substratevm/mx.substratevm/suite.py @@ -587,7 +587,7 @@ "sourceDirs": ["src"], "dependencies": [ "mx:JUNIT_TOOL", - "sdk:GRAAL_SDK", + "SVM", ], "checkstyle": "com.oracle.svm.core", "workingSets": "SVM", @@ -1321,7 +1321,7 @@ ], "distDependencies": [ "mx:JUNIT_TOOL", - "sdk:GRAAL_SDK", + "SVM", "SVM_CONFIGURE", ], "testDistribution" : True, diff --git a/substratevm/mx.substratevm/testhello.py b/substratevm/mx.substratevm/testhello.py index 878b27807421..71703247fed9 100644 --- a/substratevm/mx.substratevm/testhello.py +++ b/substratevm/mx.substratevm/testhello.py @@ -263,6 +263,13 @@ def test(): r"%s = 1000"%(wildcard_pattern)) checker.check(exec_string, skip_fails=False) + # set a break point at hello.Hello$DefaultGreeter::greet + # expect "Breakpoint 2 at 0x[0-9a-f]+: file hello/Target_Hello_DefaultGreeter.java, line [0-9]+." + exec_string = execute("break hello.Hello$DefaultGreeter::greet") + rexp = r"Breakpoint 2 at %s: file hello/Target_hello_Hello_DefaultGreeter\.java, line %s\."%(address_pattern, digits_pattern) + checker = Checker("break on substituted method", rexp) + checker.check(exec_string, skip_fails=False) + # look up PrintStream.println methods # expect "All functions matching regular expression "java.io.PrintStream.println":" # expect "" @@ -284,7 +291,7 @@ def test(): # set a break point at PrintStream.println(String) # expect "Breakpoint 2 at 0x[0-9a-f]+: java.base/java/io/PrintStream.java, line [0-9]+." exec_string = execute("break java.io.PrintStream::println(java.lang.String *)") - rexp = r"Breakpoint 2 at %s: file .*java/io/PrintStream\.java, line %s\."%(address_pattern, digits_pattern) + rexp = r"Breakpoint 3 at %s: file .*java/io/PrintStream\.java, line %s\."%(address_pattern, digits_pattern) checker = Checker('break println', rexp) checker.check(exec_string, skip_fails=False) @@ -400,7 +407,8 @@ def test(): print(checker) sys.exit(1) - # continue to next breakpoint + # continue to next 2 breakpoints + execute("continue") execute("continue") # run backtrace to check we are in java.io.PrintStream::println(java.lang.String) diff --git a/substratevm/src/com.oracle.svm.test/src/hello/Target_hello_Hello_DefaultGreeter.java b/substratevm/src/com.oracle.svm.test/src/hello/Target_hello_Hello_DefaultGreeter.java new file mode 100644 index 000000000000..eb921a384e73 --- /dev/null +++ b/substratevm/src/com.oracle.svm.test/src/hello/Target_hello_Hello_DefaultGreeter.java @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2020, Red Hat Inc. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package hello; + +import com.oracle.svm.core.annotate.Substitute; +import com.oracle.svm.core.annotate.TargetClass; + +@TargetClass(value = Hello.DefaultGreeter.class) +final class Target_hello_Hello_DefaultGreeter { + @Substitute + public void greet() { + // Checkstyle: stop + System.out.println("Hello, substituted world!"); + // Checkstyle: resume + } +} From 1b7f36202d98ec1636b968298c2fa5d3eac725c5 Mon Sep 17 00:00:00 2001 From: Foivos Zakkak Date: Mon, 17 May 2021 18:24:50 +0300 Subject: [PATCH 2/4] Augment MethodEntry with field indicating whether method is a substitution --- .../objectfile/debugentry/ClassEntry.java | 5 +++-- .../objectfile/debugentry/MethodEntry.java | 14 ++++++++++++-- .../debuginfo/DebugInfoProvider.java | 5 +++++ .../image/NativeImageDebugInfoProvider.java | 18 ++++++++++++++++++ 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ClassEntry.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ClassEntry.java index 83322f45cc37..8247c36f3a5b 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ClassEntry.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ClassEntry.java @@ -302,7 +302,7 @@ protected MethodEntry processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBa * substitution */ FileEntry methodFileEntry = debugInfoBase.ensureFileEntry(fileName, filePath, cachePath); - return new MethodEntry(methodFileEntry, methodName, this, resultType, paramTypeArray, paramNameArray, modifiers, debugMethodInfo.isDeoptTarget()); + return new MethodEntry(methodFileEntry, methodName, this, resultType, paramTypeArray, paramNameArray, modifiers, debugMethodInfo.isDeoptTarget(), debugMethodInfo.isSubstitutionMethod()); } @Override @@ -357,10 +357,11 @@ public MethodEntry getMethodEntry(DebugMethodInfo debugMethodInfo, DebugInfoBase String methodName = debugInfoBase.uniqueDebugString(debugMethodInfo.name()); String paramSignature = debugMethodInfo.paramSignature(); String returnTypeName = debugMethodInfo.valueType(); + boolean substitutionMethod = debugMethodInfo.isSubstitutionMethod(); ListIterator methodIterator = methods.listIterator(); while (methodIterator.hasNext()) { MethodEntry methodEntry = methodIterator.next(); - int comparisonResult = methodEntry.compareTo(methodName, paramSignature, returnTypeName); + int comparisonResult = methodEntry.compareTo(methodName, paramSignature, returnTypeName, substitutionMethod); if (comparisonResult == 0) { return methodEntry; } else if (comparisonResult > 0) { diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/MethodEntry.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/MethodEntry.java index cca95588a76e..c7d93473c79c 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/MethodEntry.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/MethodEntry.java @@ -30,14 +30,18 @@ public class MethodEntry extends MemberEntry implements Comparable final TypeEntry[] paramTypes; final String[] paramNames; final boolean isDeoptTarget; + final boolean isSubstitution; - public MethodEntry(FileEntry fileEntry, String methodName, ClassEntry ownerType, TypeEntry valueType, TypeEntry[] paramTypes, String[] paramNames, int modifiers, boolean isDeoptTarget) { + public MethodEntry(FileEntry fileEntry, String methodName, ClassEntry ownerType, TypeEntry valueType, + TypeEntry[] paramTypes, String[] paramNames, int modifiers, boolean isDeoptTarget, + boolean isSubstitutionMethod) { super(fileEntry, methodName, ownerType, valueType, modifiers); assert ((paramTypes == null && paramNames == null) || (paramTypes != null && paramNames != null && paramTypes.length == paramNames.length)); this.paramTypes = paramTypes; this.paramNames = paramNames; this.isDeoptTarget = isDeoptTarget; + this.isSubstitution = isSubstitutionMethod; } public String methodName() { @@ -74,7 +78,7 @@ public String getParamName(int idx) { return paramNames[idx]; } - public int compareTo(String methodName, String paramSignature, String returnTypeName) { + public int compareTo(String methodName, String paramSignature, String returnTypeName, boolean isSubstitutionMethod) { int nameComparison = memberName.compareTo(methodName); if (nameComparison != 0) { return nameComparison; @@ -100,6 +104,9 @@ public int compareTo(String methodName, String paramSignature, String returnType return paraComparison; } } + if (this.isSubstitution != isSubstitutionMethod) { + return this.isSubstitution ? 1 : -1; + } return 0; } @@ -128,6 +135,9 @@ public int compareTo(MethodEntry other) { return paramComparison; } } + if (this.isSubstitution != other.isSubstitution) { + return this.isSubstitution ? 1 : -1; + } return 0; } } diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debuginfo/DebugInfoProvider.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debuginfo/DebugInfoProvider.java index 5c5729de8eb2..845229d7a0c0 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debuginfo/DebugInfoProvider.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debuginfo/DebugInfoProvider.java @@ -225,6 +225,11 @@ interface DebugMethodInfo extends DebugMemberInfo { * @return true if this method has been compiled in as a deoptimization target */ boolean isDeoptTarget(); + + /** + * @return true if this method substitutes another method + */ + boolean isSubstitutionMethod(); } /** diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java index 2c75d3df49ec..13eb58892614 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java @@ -726,6 +726,11 @@ public boolean isDeoptTarget() { return hostedMethod.isDeoptTarget(); } + @Override + public boolean isSubstitutionMethod() { + return hostedMethod.getWrapped().getWrapped() instanceof SubstitutionMethod; + } + @Override public int modifiers() { return hostedMethod.getModifiers(); @@ -987,6 +992,11 @@ public boolean isDeoptTarget() { return hostedMethod.isDeoptTarget(); } + @Override + public boolean isSubstitutionMethod() { + return hostedMethod.getWrapped().getWrapped() instanceof SubstitutionMethod; + } + @Override public List paramTypes() { Signature signature = hostedMethod.getSignature(); @@ -1134,6 +1144,14 @@ public boolean isDeoptTarget() { return name().endsWith(HostedMethod.METHOD_NAME_DEOPT_SUFFIX); } + @Override + public boolean isSubstitutionMethod() { + if (method instanceof HostedMethod) { + return ((HostedMethod) method).getWrapped().getWrapped() instanceof SubstitutionMethod; + } + return method instanceof SubstitutionMethod; + } + @Override public int addressLo() { return lo; From 8407beb43c76378737a95fc9885980a1561a44e7 Mon Sep 17 00:00:00 2001 From: Foivos Zakkak Date: Mon, 17 May 2021 18:27:26 +0300 Subject: [PATCH 3/4] Fix computation of full file path for substituted methods --- .../image/NativeImageDebugInfoProvider.java | 47 +++++++++---------- .../hosted/image/sources/SourceManager.java | 6 ++- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java index 13eb58892614..7bba39e055e0 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java @@ -45,9 +45,7 @@ import org.graalvm.compiler.graph.NodeSourcePosition; import org.graalvm.nativeimage.ImageSingletons; -import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider; import com.oracle.graal.pointsto.infrastructure.WrappedJavaMethod; -import com.oracle.graal.pointsto.meta.AnalysisType; import com.oracle.objectfile.debuginfo.DebugInfoProvider; import com.oracle.svm.core.StaticFieldsSupport; import com.oracle.svm.core.SubstrateOptions; @@ -197,6 +195,20 @@ protected static ResolvedJavaType getJavaType(HostedType hostedType, boolean wan return hostedType.getWrapped().getWrapped(); } + private static ResolvedJavaType getJavaType(ResolvedJavaMethod method, boolean wantOriginal) { + if (method instanceof HostedMethod) { + return getJavaType((HostedMethod) method, wantOriginal); + } + ResolvedJavaMethod wrappedMethod = method; + while (wrappedMethod instanceof WrappedJavaMethod) { + wrappedMethod = ((WrappedJavaMethod) wrappedMethod).getWrapped(); + } + if (wrappedMethod instanceof SubstitutionMethod) { + return ((SubstitutionMethod) wrappedMethod).getAnnotated().getDeclaringClass(); + } + return wrappedMethod.getDeclaringClass(); + } + protected static ResolvedJavaType getJavaType(HostedMethod hostedMethod, boolean wantOriginal) { if (wantOriginal) { /* Check for wholesale replacement of the original class. */ @@ -216,6 +228,9 @@ protected static ResolvedJavaType getJavaType(HostedMethod hostedMethod, boolean return hostedType.getWrapped().getWrapped(); } ResolvedJavaMethod javaMethod = hostedMethod.getWrapped().getWrapped(); + if (javaMethod instanceof SubstitutionMethod) { + return ((SubstitutionMethod) javaMethod).getAnnotated().getDeclaringClass(); + } return javaMethod.getDeclaringClass(); } @@ -267,10 +282,9 @@ private abstract class NativeImageDebugFileInfo implements DebugFileInfo { @SuppressWarnings("try") NativeImageDebugFileInfo(HostedType hostedType) { ResolvedJavaType javaType = getJavaType(hostedType, false); - Class clazz = hostedType.getJavaClass(); SourceManager sourceManager = ImageSingletons.lookup(SourceManager.class); try (DebugContext.Scope s = debugContext.scope("DebugFileInfo", hostedType)) { - fullFilePath = sourceManager.findAndCacheSource(javaType, clazz, debugContext); + fullFilePath = sourceManager.findAndCacheSource(javaType, debugContext); } catch (Throwable e) { throw debugContext.handle(e); } @@ -280,10 +294,9 @@ private abstract class NativeImageDebugFileInfo implements DebugFileInfo { NativeImageDebugFileInfo(HostedMethod hostedMethod) { ResolvedJavaType javaType = getJavaType(hostedMethod, false); HostedType hostedType = hostedMethod.getDeclaringClass(); - Class clazz = hostedType.getJavaClass(); SourceManager sourceManager = ImageSingletons.lookup(SourceManager.class); try (DebugContext.Scope s = debugContext.scope("DebugFileInfo", hostedType)) { - fullFilePath = sourceManager.findAndCacheSource(javaType, clazz, debugContext); + fullFilePath = sourceManager.findAndCacheSource(javaType, debugContext); } catch (Throwable e) { throw debugContext.handle(e); } @@ -293,10 +306,9 @@ private abstract class NativeImageDebugFileInfo implements DebugFileInfo { NativeImageDebugFileInfo(HostedField hostedField) { ResolvedJavaType javaType = getJavaType(hostedField, false); HostedType hostedType = hostedField.getDeclaringClass(); - Class clazz = hostedType.getJavaClass(); SourceManager sourceManager = ImageSingletons.lookup(SourceManager.class); try (DebugContext.Scope s = debugContext.scope("DebugFileInfo", hostedType)) { - fullFilePath = sourceManager.findAndCacheSource(javaType, clazz, debugContext); + fullFilePath = sourceManager.findAndCacheSource(javaType, debugContext); } catch (Throwable e) { throw debugContext.handle(e); } @@ -1202,25 +1214,10 @@ public int modifiers() { @SuppressWarnings("try") private void computeFullFilePath() { - ResolvedJavaType declaringClass = method.getDeclaringClass(); - Class clazz = null; - if (declaringClass instanceof OriginalClassProvider) { - clazz = ((OriginalClassProvider) declaringClass).getJavaClass(); - } - /* - * 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 or singly unwrapped AnalysisType. - */ - if (declaringClass instanceof HostedType) { - declaringClass = ((HostedType) declaringClass).getWrapped(); - } - if (declaringClass instanceof AnalysisType) { - declaringClass = ((AnalysisType) declaringClass).getWrapped(); - } + ResolvedJavaType declaringClass = getJavaType(method, false); SourceManager sourceManager = ImageSingletons.lookup(SourceManager.class); try (DebugContext.Scope s = debugContext.scope("DebugCodeInfo", declaringClass)) { - fullFilePath = sourceManager.findAndCacheSource(declaringClass, clazz, debugContext); + fullFilePath = sourceManager.findAndCacheSource(declaringClass, debugContext); } catch (Throwable e) { throw debugContext.handle(e); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/sources/SourceManager.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/sources/SourceManager.java index 167501d0f9b4..543a4512245a 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/sources/SourceManager.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/sources/SourceManager.java @@ -26,6 +26,8 @@ package com.oracle.svm.hosted.image.sources; +import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider; +import com.oracle.svm.hosted.c.GraalAccess; import jdk.vm.ci.meta.ResolvedJavaType; import org.graalvm.compiler.debug.DebugContext; @@ -46,12 +48,11 @@ public class SourceManager { * the source. * * @param resolvedType the Java type whose source file should be located and cached - * @param clazz the Java class associated with the resolved type * @param debugContext context for logging details of any lookup failure * @return a path identifying the location of a successfully cached file for inclusion in the * generated debug info or null if a source file cannot be found or cached. */ - public Path findAndCacheSource(ResolvedJavaType resolvedType, Class clazz, DebugContext debugContext) { + public Path findAndCacheSource(ResolvedJavaType resolvedType, DebugContext debugContext) { /* short circuit if we have already seen this type */ Path path = verifiedPaths.get(resolvedType); if (path != null) { @@ -68,6 +69,7 @@ public Path findAndCacheSource(ResolvedJavaType resolvedType, Class clazz, De */ if (resolvedType.isInstanceClass() || resolvedType.isInterface()) { String packageName = computePackageName(resolvedType); + Class clazz = OriginalClassProvider.getJavaClass(GraalAccess.getOriginalSnippetReflection(), resolvedType); path = locateSource(fileName, packageName, clazz); if (path == null) { // as a last ditch effort derive path from the Java class name From da378cd87d3cd8584e2424d4923e5c3a2528239d Mon Sep 17 00:00:00 2001 From: Foivos Zakkak Date: Mon, 17 May 2021 18:28:19 +0300 Subject: [PATCH 4/4] Fix file indexes in ClassEntry for substituted methods --- .../com/oracle/objectfile/elf/dwarf/DwarfInfoSectionImpl.java | 2 +- .../com/oracle/objectfile/elf/dwarf/DwarfLineSectionImpl.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfInfoSectionImpl.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfInfoSectionImpl.java index 00e88b8626d7..add129377c3c 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfInfoSectionImpl.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfInfoSectionImpl.java @@ -682,7 +682,7 @@ private int writeMethodDeclaration(DebugContext context, ClassEntry classEntry, String name = uniqueDebugString(range.getMethodName()); log(context, " [0x%08x] name 0x%x (%s)", pos, debugStringIndex(name), name); pos = writeAttrStrp(name, buffer, pos); - int fileIdx = classEntry.localFilesIdx(); + int fileIdx = classEntry.localFilesIdx(range.getFileEntry()); log(context, " [0x%08x] file 0x%x (%s)", pos, fileIdx, range.getFileEntry().getFullName()); pos = writeAttrData2((short) fileIdx, buffer, pos); String returnTypeName = range.getMethodReturnTypeName(); diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfLineSectionImpl.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfLineSectionImpl.java index bfc14cbd7d71..4409b436b8bc 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfLineSectionImpl.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfLineSectionImpl.java @@ -436,7 +436,6 @@ private int writeLineNumberTable(DebugContext context, ClassEntry classEntry, by String primaryClassName = classEntry.getTypeName(); String primaryFileName = classEntry.getFileName(); String file = primaryFileName; - int fileIdx = 1; log(context, " [0x%08x] primary class %s", pos, primaryClassName); log(context, " [0x%08x] primary file %s", pos, primaryFileName); for (PrimaryEntry primaryEntry : classEntry.getPrimaryEntries()) { @@ -464,6 +463,7 @@ private int writeLineNumberTable(DebugContext context, ClassEntry classEntry, by /* * Initialize and write a row for the start of the primary method. */ + int fileIdx = classEntry.localFilesIdx(primaryRange.getFileEntry()); pos = writeSetFileOp(context, file, fileIdx, buffer, pos); pos = writeSetBasicBlockOp(context, buffer, pos); /*