From 06b78760a465b20989e8bd9a2b0998b9ade6091f Mon Sep 17 00:00:00 2001 From: Bernhard Urban-Forster Date: Thu, 13 Jun 2024 08:36:48 +0200 Subject: [PATCH] [GR-53007] Emit linker relocations for MethodPointers As they can live in a different section than `.text`, and thus we cannot always resolve them eagerly. We still do not want to have dynamic relocations in the `.text` section, this is ensured by: * passing `-o text` the linker (on Linux only) * by `-H:+NoDirectRelocationsInText` (on by default) This partially reverts 946090a618d0269fea79da165fe738096577e3d1 --- .../aarch64/AArch64HostedPatcherFeature.java | 16 +++++++++--- .../code/amd64/AMD64HostedPatcherFeature.java | 26 +++++++++++++++---- .../hosted/image/LIRNativeImageCodeCache.java | 26 ++++++------------- .../MethodPointerRelocationProvider.java | 5 ++-- .../oracle/svm/hosted/image/NativeImage.java | 14 +++++++--- 5 files changed, 55 insertions(+), 32 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/AArch64HostedPatcherFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/AArch64HostedPatcherFeature.java index 0c7606a08a8f..847311eac18c 100755 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/AArch64HostedPatcherFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/AArch64HostedPatcherFeature.java @@ -26,11 +26,14 @@ import java.util.function.Consumer; +import com.oracle.svm.core.meta.MethodPointer; +import com.oracle.svm.hosted.meta.HostedMethod; import jdk.graal.compiler.asm.Assembler.CodeAnnotation; import jdk.graal.compiler.asm.aarch64.AArch64Assembler.SingleInstructionAnnotation; import jdk.graal.compiler.asm.aarch64.AArch64MacroAssembler; import jdk.graal.compiler.asm.aarch64.AArch64MacroAssembler.MovSequenceAnnotation.MovAction; import jdk.graal.compiler.code.CompilationResult; +import jdk.vm.ci.meta.VMConstant; import org.graalvm.nativeimage.ImageSingletons; import org.graalvm.nativeimage.Platform; import org.graalvm.nativeimage.Platforms; @@ -169,17 +172,24 @@ class AdrpAddMacroInstructionHostedPatcher extends CompilationResult.CodeAnnotat @Override public void relocate(Reference ref, RelocatableBuffer relocs, int compStart) { + Object relocVal = ref; if (ref instanceof ConstantReference constantRef) { - VMError.guarantee(!(constantRef.getConstant() instanceof SubstrateMethodPointerConstant), "SubstrateMethodPointerConstants should not be relocated %s", constantRef); + VMConstant constant = constantRef.getConstant(); + if (constant instanceof SubstrateMethodPointerConstant methodPointerConstant) { + MethodPointer pointer = methodPointerConstant.pointer(); + HostedMethod hMethod = (HostedMethod) pointer.getMethod(); + VMError.guarantee(hMethod.isCompiled(), "Method %s is not compiled although there is a method pointer constant created for it.", hMethod); + relocVal = pointer; + } } else { VMError.guarantee(ref instanceof DataSectionReference || ref instanceof CGlobalDataReference, "Unexpected reference: %s", ref); } int siteOffset = compStart + macroInstruction.instructionPosition; - relocs.addRelocationWithoutAddend(siteOffset, RelocationKind.AARCH64_R_AARCH64_ADR_PREL_PG_HI21, ref); + relocs.addRelocationWithoutAddend(siteOffset, RelocationKind.AARCH64_R_AARCH64_ADR_PREL_PG_HI21, relocVal); siteOffset += 4; - relocs.addRelocationWithoutAddend(siteOffset, RelocationKind.AARCH64_R_AARCH64_ADD_ABS_LO12_NC, ref); + relocs.addRelocationWithoutAddend(siteOffset, RelocationKind.AARCH64_R_AARCH64_ADD_ABS_LO12_NC, relocVal); } @Uninterruptible(reason = ".") diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/amd64/AMD64HostedPatcherFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/amd64/AMD64HostedPatcherFeature.java index 2356789077a4..cc957e03f926 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/amd64/AMD64HostedPatcherFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/amd64/AMD64HostedPatcherFeature.java @@ -30,7 +30,7 @@ import org.graalvm.nativeimage.Platform; import org.graalvm.nativeimage.Platforms; -import com.oracle.objectfile.ObjectFile; +import com.oracle.objectfile.ObjectFile.RelocationKind; import com.oracle.svm.core.Uninterruptible; import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature; import com.oracle.svm.core.feature.InternalFeature; @@ -38,11 +38,13 @@ import com.oracle.svm.core.graal.code.PatchConsumerFactory; import com.oracle.svm.core.layeredimagesingleton.FeatureSingleton; import com.oracle.svm.core.layeredimagesingleton.UnsavedSingleton; +import com.oracle.svm.core.meta.MethodPointer; import com.oracle.svm.core.meta.SubstrateMethodPointerConstant; import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.code.HostedImageHeapConstantPatch; import com.oracle.svm.hosted.code.HostedPatcher; import com.oracle.svm.hosted.image.RelocatableBuffer; +import com.oracle.svm.hosted.meta.HostedMethod; import jdk.graal.compiler.asm.Assembler; import jdk.graal.compiler.asm.amd64.AMD64BaseAssembler.AddressDisplacementAnnotation; @@ -52,6 +54,7 @@ import jdk.vm.ci.code.site.DataSectionReference; import jdk.vm.ci.code.site.Reference; import jdk.vm.ci.meta.JavaConstant; +import jdk.vm.ci.meta.VMConstant; @AutomaticallyRegisteredFeature @Platforms({Platform.AMD64.class}) @@ -120,10 +123,23 @@ public void relocate(Reference ref, RelocatableBuffer relocs, int compStart) { * the relocation site, we want to subtract n bytes from our addend. */ long addend = (annotation.nextInstructionPosition - annotation.operandPosition); - relocs.addRelocationWithAddend((int) siteOffset, ObjectFile.RelocationKind.getPCRelative(annotation.operandSize), addend, ref); - } else if (ref instanceof ConstantReference constantRef) { - VMError.guarantee(!(constantRef.getConstant() instanceof SubstrateMethodPointerConstant), "SubstrateMethodPointerConstants should not be relocated %s", constantRef); - relocs.addRelocationWithoutAddend((int) siteOffset, ObjectFile.RelocationKind.getDirect(annotation.operandSize), ref); + assert addend == annotation.operandSize; + relocs.addRelocationWithAddend((int) siteOffset, RelocationKind.getPCRelative(annotation.operandSize), addend, ref); + } else if (ref instanceof ConstantReference constantReference) { + VMConstant constant = constantReference.getConstant(); + if (constant instanceof SubstrateMethodPointerConstant methodPointerConstant) { + MethodPointer pointer = methodPointerConstant.pointer(); + HostedMethod hMethod = (HostedMethod) pointer.getMethod(); + VMError.guarantee(hMethod.isCompiled(), "Method %s is not compiled although there is a method pointer constant created for it.", hMethod); + + RelocationKind kindPCRelative = RelocationKind.getPCRelative(annotation.operandSize); + // lea instruction using rip relative addressing, account for additional offset + long addend = -RelocationKind.getRelocationSize(kindPCRelative); + relocs.addRelocationWithAddend((int) siteOffset, kindPCRelative, addend, pointer); + } else { + RelocationKind kindDirect = RelocationKind.getDirect(annotation.operandSize); + relocs.addRelocationWithoutAddend((int) siteOffset, kindDirect, constantReference); + } } else { throw VMError.shouldNotReachHere("Unknown type of reference in code"); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/LIRNativeImageCodeCache.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/LIRNativeImageCodeCache.java index abd090ad6569..77541bf16220 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/LIRNativeImageCodeCache.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/LIRNativeImageCodeCache.java @@ -40,7 +40,6 @@ import com.oracle.objectfile.ObjectFile; import com.oracle.svm.core.SubstrateOptions; import com.oracle.svm.core.config.ConfigurationValues; -import com.oracle.svm.core.meta.SubstrateMethodPointerConstant; import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.code.HostedDirectCallTrampolineSupport; import com.oracle.svm.hosted.code.HostedImageHeapConstantPatch; @@ -55,7 +54,6 @@ import jdk.graal.compiler.debug.Indent; import jdk.vm.ci.code.TargetDescription; import jdk.vm.ci.code.site.Call; -import jdk.vm.ci.code.site.ConstantReference; import jdk.vm.ci.code.site.DataPatch; import jdk.vm.ci.code.site.Infopoint; import jdk.vm.ci.code.site.Reference; @@ -397,26 +395,18 @@ public void patchMethods(DebugContext debug, RelocatableBuffer relocs, ObjectFil patchesHandled++; } } + for (DataPatch dataPatch : compilation.getDataPatches()) { assert dataPatch.note == null : "Unexpected note: " + dataPatch.note; Reference ref = dataPatch.reference; var patcher = patches.get(dataPatch.pcOffset); - if (ref instanceof ConstantReference constant && constant.getConstant() instanceof SubstrateMethodPointerConstant methodPtrConstant) { - /* - * We directly patch SubstrateMethodPointerConstants. - */ - HostedMethod hMethod = (HostedMethod) methodPtrConstant.pointer().getMethod(); - VMError.guarantee(hMethod.isCompiled(), "Method %s is not compiled although there is a method pointer constant created for it.", hMethod); - int targetOffset = hMethod.getCodeAddressOffset(); - int pcDisplacement = targetOffset - (compStart + dataPatch.pcOffset); - patcher.patch(compStart, pcDisplacement, compilation.getTargetCode()); - } else { - /* - * Constants are allocated offsets in a separate space, which can be emitted as - * read-only (.rodata) section. - */ - patcher.relocate(ref, relocs, compStart); - } + /* + * Constants are (1) allocated offsets in a separate space, which can be emitted as + * read-only (.rodata) section, or (2) method pointers that are computed relative to + * the PC. + */ + patcher.relocate(ref, relocs, compStart); + boolean noPriorMatch = patchedOffsets.add(dataPatch.pcOffset); VMError.guarantee(noPriorMatch, "Patching same offset twice."); patchesHandled++; diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/MethodPointerRelocationProvider.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/MethodPointerRelocationProvider.java index df2a544ea117..7455f82544e3 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/MethodPointerRelocationProvider.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/MethodPointerRelocationProvider.java @@ -39,10 +39,9 @@ public static MethodPointerRelocationProvider singleton() { } public void markMethodPointerRelocation(ObjectFile.ProgbitsSectionImpl section, int offset, ObjectFile.RelocationKind relocationKind, HostedMethod target, - @SuppressWarnings("unused") boolean isStaticallyResolved) { - section.markRelocationSite(offset, relocationKind, localSymbolNameForMethod(target), 0L); + long addend, @SuppressWarnings("unused") boolean isStaticallyResolved) { + section.markRelocationSite(offset, relocationKind, localSymbolNameForMethod(target), addend); } - } @AutomaticallyRegisteredFeature diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImage.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImage.java index deb2a3dcbe81..3cdb572d48e4 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImage.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImage.java @@ -603,6 +603,14 @@ private static void validateNoDirectRelocationsInTextSection(RelocatableBuffer.I } } + private static boolean checkMethodPointerRelocationKind(Info info) { + int wordSize = ConfigurationValues.getTarget().arch.getWordSize(); + int relocationSize = info.getRelocationSize(); + RelocationKind relocationKind = info.getRelocationKind(); + + return (relocationSize == wordSize && RelocationKind.isDirect(relocationKind)) || (relocationSize == 4 && RelocationKind.isPCRelative(relocationKind)); + } + private void markFunctionRelocationSite(final ProgbitsSectionImpl sectionImpl, final int offset, final RelocatableBuffer.Info info) { assert info.getTargetObject() instanceof CFunctionPointer : "Wrong type for FunctionPointer relocation: " + info.getTargetObject().toString(); @@ -617,10 +625,10 @@ private void markFunctionRelocationSite(final ProgbitsSectionImpl sectionImpl, f if (!target.isCompiled() && !target.wrapped.isInBaseLayer()) { target = metaAccess.lookupJavaMethod(InvalidMethodPointerHandler.METHOD_POINTER_NOT_COMPILED_HANDLER_METHOD); } + + assert checkMethodPointerRelocationKind(info); // A reference to a method. Mark the relocation site using the symbol name. - Architecture arch = ConfigurationValues.getTarget().arch; - assert (arch instanceof AArch64) || RelocationKind.getDirect(arch.getWordSize()) == info.getRelocationKind(); - relocationProvider.markMethodPointerRelocation(sectionImpl, offset, info.getRelocationKind(), target, methodPointer.isAbsolute()); + relocationProvider.markMethodPointerRelocation(sectionImpl, offset, info.getRelocationKind(), target, info.getAddend(), methodPointer.isAbsolute()); } private static boolean isAddendAligned(Architecture arch, long addend, RelocationKind kind) {