Skip to content

Commit

Permalink
[GR-43389] [darwin-amd64] Workaround for buggy ld64 versions
Browse files Browse the repository at this point in the history 
Some versions of `ld64` cannot deal with DIRECT8 relocations in the `.text` section. Approximately this is since version 820.1 (Xcode 14). Starting with Xcode15 beta3 the default linker has been replaced with "the new linker" (version 902.11) which does not suffer from this bug anymore.

However, Xcode also ships a `ld-classic` which reassembles "the old linker" and is still affected by this bug (and it of course prints the same version number).

See some more in-depth analysis of this ld64 bug in https://openradar.appspot.com/FB11942354

The workaround: Instead of emitting the address of a HostedMethod inlined, it will be put in the data section and thus requires a memory load to obtain it.
  • Loading branch information
@lewurm
lewurm committed Sep 6, 2023
1 parent f66c6bc commit e30698b
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 25 deletions.
12 changes: 10 additions & 2 deletions ...ore.graal.amd64/src/com/oracle/svm/core/graal/amd64/AMD64LoadMethodPointerConstantOp.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.HINT;
import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.REG;

import com.oracle.svm.core.FrameAccess;
import org.graalvm.compiler.asm.amd64.AMD64Address;
import org.graalvm.compiler.asm.amd64.AMD64MacroAssembler;
import org.graalvm.compiler.lir.LIRInstructionClass;
import org.graalvm.compiler.lir.StandardOp;
Expand All @@ -38,6 +40,7 @@

import jdk.vm.ci.code.Register;
import jdk.vm.ci.meta.AllocatableValue;
import org.graalvm.nativeimage.Platform;

public final class AMD64LoadMethodPointerConstantOp extends AMD64LIRInstruction implements StandardOp.LoadConstantOp {
public static final LIRInstructionClass<AMD64LoadMethodPointerConstantOp> TYPE = LIRInstructionClass.create(AMD64LoadMethodPointerConstantOp.class);
Expand All @@ -53,8 +56,13 @@ public final class AMD64LoadMethodPointerConstantOp extends AMD64LIRInstruction
@Override
public void emitCode(CompilationResultBuilder crb, AMD64MacroAssembler masm) {
Register resultReg = asRegister(result);
crb.recordInlineDataInCode(constant);
masm.movq(resultReg, 0L, true);
if (!Platform.includedIn(Platform.DARWIN_AMD64.class)) {
crb.recordInlineDataInCode(constant);
masm.movq(resultReg, 0L, true);
} else {
/* [GR-43389] ld64 bug does not allow direct8 relocations in .text on darwin */
masm.movq(resultReg, (AMD64Address) crb.recordDataReferenceInCode(constant, FrameAccess.wordSize()));
}
}

@Override
Expand Down
27 changes: 17 additions & 10 deletions ...tevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/code/SubstrateDataBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import java.nio.ByteBuffer;

import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
import org.graalvm.compiler.code.DataSection.Data;
import org.graalvm.compiler.code.DataSection.Patches;
import org.graalvm.compiler.core.common.type.CompressibleConstant;
Expand All @@ -50,8 +51,11 @@ public class SubstrateDataBuilder extends DataBuilder {
@Override
public Data createDataItem(Constant constant) {
int size;
if (constant instanceof VMConstant vmConstant) {
assert constant instanceof JavaConstant && constant instanceof CompressibleConstant && constant instanceof TypedConstant : constant;
if (constant instanceof SubstrateMethodPointerConstant methodPointerConstant) {
size = FrameAccess.wordSize();
return new ObjectData(size, size, methodPointerConstant);
} else if (constant instanceof VMConstant vmConstant) {
assert constant instanceof CompressibleConstant && constant instanceof TypedConstant : constant;
return new ObjectData(vmConstant);
} else if (JavaConstant.isNull(constant)) {
if (SubstrateObjectConstant.isCompressed((JavaConstant) constant)) {
Expand All @@ -60,9 +64,8 @@ public Data createDataItem(Constant constant) {
size = FrameAccess.uncompressedReferenceSize();
}
return createZeroData(size, size);
} else if (constant instanceof SerializableConstant) {
SerializableConstant s = (SerializableConstant) constant;
return createSerializableData(s);
} else if (constant instanceof SerializableConstant serializableConstant) {
return createSerializableData(serializableConstant);
} else {
throw new JVMCIError(String.valueOf(constant));
}
Expand All @@ -71,15 +74,19 @@ public Data createDataItem(Constant constant) {
public static class ObjectData extends Data {
private final VMConstant constant;

protected ObjectData(int alignment, int size, VMConstant constant) {
super(alignment, size);
this.constant = constant;
}

protected ObjectData(VMConstant constant) {
super(ConfigurationValues.getObjectLayout().getReferenceSize(), ConfigurationValues.getObjectLayout().getReferenceSize());
this(ConfigurationValues.getObjectLayout().getReferenceSize(), ConfigurationValues.getObjectLayout().getReferenceSize(), constant);
assert ((CompressibleConstant) constant).isCompressed() == ReferenceAccess.singleton()
.haveCompressedReferences() : "Constant object references in compiled code must be compressed (base-relative)";
this.constant = constant;
}

public JavaConstant getConstant() {
return (JavaConstant) constant;
public VMConstant getConstant() {
return constant;
}

@Override
Expand All @@ -92,7 +99,7 @@ public static void emit(ByteBuffer buffer, Patches patches, int size, VMConstant
if (size == Integer.BYTES) {
buffer.putInt(0);
} else if (size == Long.BYTES) {
buffer.putLong(0L);
buffer.putLong(0);
} else {
shouldNotReachHere("Unsupported object constant reference size: " + size);
}
Expand Down
29 changes: 22 additions & 7 deletions substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ForkJoinPool;

import com.oracle.svm.core.graal.code.SubstrateDataBuilder;
import org.graalvm.collections.EconomicMap;
import org.graalvm.compiler.api.replacements.Fold;
import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
Expand Down Expand Up @@ -122,7 +123,6 @@
import com.oracle.svm.core.graal.phases.OptimizeExceptionPathsPhase;
import com.oracle.svm.core.heap.RestrictHeapAccess;
import com.oracle.svm.core.heap.RestrictHeapAccessCallees;
import com.oracle.svm.core.meta.MethodPointer;
import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
import com.oracle.svm.core.util.InterruptImageBuilding;
import com.oracle.svm.core.util.VMError;
Expand Down Expand Up @@ -151,6 +151,7 @@
import jdk.vm.ci.meta.MetaAccessProvider;
import jdk.vm.ci.meta.ResolvedJavaMethod;
import jdk.vm.ci.meta.VMConstant;
import org.graalvm.nativeimage.Platform;

public class CompileQueue {

Expand Down Expand Up @@ -1368,15 +1369,29 @@ protected void removeDeoptTargetOptimizations(LIRSuites lirSuites) {
DeoptimizationUtils.removeDeoptTargetOptimizations(lirSuites);
}

private void ensureCompiledForMethodPointerConstant(HostedMethod method, CompileReason reason, SubstrateMethodPointerConstant methodPointerConstant) {
HostedMethod referencedMethod = (HostedMethod) methodPointerConstant.pointer().getMethod();
ensureCompiled(referencedMethod, new MethodPointerConstantReason(method, referencedMethod, reason));
}

protected final void ensureCompiledForMethodPointerConstants(HostedMethod method, CompileReason reason, CompilationResult result) {
for (DataPatch dataPatch : result.getDataPatches()) {
Reference reference = dataPatch.reference;
if (reference instanceof ConstantReference) {
VMConstant constant = ((ConstantReference) reference).getConstant();
if (constant instanceof SubstrateMethodPointerConstant) {
MethodPointer pointer = ((SubstrateMethodPointerConstant) constant).pointer();
HostedMethod referencedMethod = (HostedMethod) pointer.getMethod();
ensureCompiled(referencedMethod, new MethodPointerConstantReason(method, referencedMethod, reason));
if (reference instanceof ConstantReference constantReference) {
VMConstant vmConstant = constantReference.getConstant();
if (vmConstant instanceof SubstrateMethodPointerConstant methodPointerConstant) {
ensureCompiledForMethodPointerConstant(method, reason, methodPointerConstant);
}
}
}

for (DataSection.Data data : result.getDataSection()) {
if (data instanceof SubstrateDataBuilder.ObjectData objectData) {
VMConstant vmConstant = objectData.getConstant();
if (vmConstant instanceof SubstrateMethodPointerConstant methodPointerConstant) {
/* [GR-43389] Only reachable with ld64 workaround on */
VMError.guarantee(Platform.includedIn(Platform.DARWIN_AMD64.class));
ensureCompiledForMethodPointerConstant(method, reason, methodPointerConstant);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions ...com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/amd64/AMD64HostedPatcherFeature.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ public void relocate(Reference ref, RelocatableBuffer relocs, int compStart) {
VMConstant constant = ((ConstantReference) ref).getConstant();
Object relocVal = ref;
if (constant instanceof SubstrateMethodPointerConstant) {
VMError.guarantee(!Platform.includedIn(Platform.DARWIN_AMD64.class), "[GR-43389] method pointer relocations should not be inlined.");
MethodPointer pointer = ((SubstrateMethodPointerConstant) constant).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);
Expand Down
6 changes: 3 additions & 3 deletions ...atevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageCodeCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ public void layoutConstants() {
CompilationResult compilation = pair.getRight();
for (DataSection.Data data : compilation.getDataSection()) {
if (data instanceof SubstrateDataBuilder.ObjectData) {
JavaConstant constant = ((SubstrateDataBuilder.ObjectData) data).getConstant();
VMConstant constant = ((SubstrateDataBuilder.ObjectData) data).getConstant();
constantReasons.put(constant, compilation.getName());
}
}
Expand All @@ -207,7 +207,7 @@ public void layoutConstants() {
public void addConstantsToHeap() {
for (DataSection.Data data : dataSection) {
if (data instanceof SubstrateDataBuilder.ObjectData) {
JavaConstant constant = ((SubstrateDataBuilder.ObjectData) data).getConstant();
VMConstant constant = ((SubstrateDataBuilder.ObjectData) data).getConstant();
addConstantToHeap(constant, NativeImageHeap.HeapInclusionReason.DataSection);
}
}
Expand Down Expand Up @@ -585,7 +585,7 @@ protected boolean verifyMethods(HostedUniverse hUniverse, ForkJoinPool threadPoo
public void writeConstants(NativeImageHeapWriter writer, RelocatableBuffer buffer) {
ByteBuffer bb = buffer.getByteBuffer();
dataSection.buildDataSection(bb, (position, constant) -> {
writer.writeReference(buffer, position, (JavaConstant) constant, "VMConstant: " + constant);
writer.writeReference(buffer, position, constant, "VMConstant: " + constant);
});
}

Expand Down
18 changes: 15 additions & 3 deletions ...tevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageHeapWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,15 @@
import java.lang.reflect.Modifier;
import java.nio.ByteBuffer;

import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
import jdk.vm.ci.meta.Constant;
import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
import org.graalvm.compiler.core.common.CompressEncoding;
import org.graalvm.compiler.core.common.NumUtil;
import org.graalvm.compiler.debug.DebugContext;
import org.graalvm.compiler.debug.Indent;
import org.graalvm.nativeimage.ImageSingletons;
import org.graalvm.nativeimage.Platform;
import org.graalvm.nativeimage.c.function.CFunctionPointer;
import org.graalvm.nativeimage.c.function.RelocatedPointer;
import org.graalvm.word.WordBase;
Expand Down Expand Up @@ -165,10 +168,16 @@ private void write(RelocatableBuffer buffer, int index, JavaConstant con, Object
private final boolean useHeapBase = NativeImageHeap.useHeapBase();
private final CompressEncoding compressEncoding = ImageSingletons.lookup(CompressEncoding.class);

void writeReference(RelocatableBuffer buffer, int index, JavaConstant target, Object reason) {
assert !(heap.hMetaAccess.isInstanceOf(target, WordBase.class)) : "word values are not references";
void writeReference(RelocatableBuffer buffer, int index, Constant constant, Object reason) {
mustBeReferenceAligned(index);
if (target.isNonNull()) {

if (constant instanceof JavaConstant target) {
assert !(heap.hMetaAccess.isInstanceOf(target, WordBase.class)) : "word values are not references";

if (target.isNull()) {
return;
}

ObjectInfo targetInfo = heap.getConstantInfo(target);
verifyTargetDidNotChange(target, reason, targetInfo);
if (useHeapBase) {
Expand All @@ -177,6 +186,9 @@ void writeReference(RelocatableBuffer buffer, int index, JavaConstant target, Ob
} else {
addDirectRelocationWithoutAddend(buffer, index, referenceSize(), target);
}
} else {
assert Platform.includedIn(Platform.DARWIN_AMD64.class) : "[GR-43389] Workaround for ld64 bug that does not allow direct8 relocations in .text on amd64";
buffer.addRelocationWithoutAddend(index, ObjectFile.RelocationKind.DIRECT_8, ((SubstrateMethodPointerConstant) constant).pointer());
}
}

Expand Down

0 comments on commit e30698b

Please sign in to comment.