Skip to content

Commit 782014f

Browse files
spaske00spaske00
committed
[GR-79030] Make GOT writes isolate-safe with writer lease
PullRequest: graal/25162
2 parents e9274fd + 2b429ec commit 782014f

6 files changed

Lines changed: 145 additions & 88 deletions

File tree

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/pltgot/GOTAccess.java‎

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,31 +24,149 @@
2424
*/
2525
package com.oracle.svm.core.pltgot;
2626

27+
import org.graalvm.nativeimage.c.type.WordPointer;
2728
import org.graalvm.word.Pointer;
2829
import org.graalvm.word.UnsignedWord;
2930
import org.graalvm.word.impl.Word;
3031

3132
import com.oracle.svm.core.SubstrateTarget;
33+
import com.oracle.svm.core.locks.PlatformLockingSupport;
34+
import com.oracle.svm.core.locks.PlatformLockingSupport.PlatformMutex;
35+
import com.oracle.svm.core.os.VirtualMemoryProvider;
36+
import com.oracle.svm.guest.staging.c.CGlobalData;
37+
import com.oracle.svm.guest.staging.c.CGlobalDataFactory;
38+
import com.oracle.svm.guest.staging.c.function.CEntryPointErrors;
3239
import com.oracle.svm.shared.Uninterruptible;
40+
import com.oracle.svm.shared.util.VMError;
3341

3442
public class GOTAccess {
43+
private static final CGlobalData<Pointer> GOT_WRITABLE = CGlobalDataFactory.createWord(Word.<Word> signed(1));
44+
private static final CGlobalData<WordPointer> ACTIVE_WRITERS = CGlobalDataFactory.createWord();
45+
/**
46+
* Changing page protection calls the OS, so use a platform mutex instead of NativeSpinLockUtils.
47+
*/
48+
private static final CGlobalData<PlatformMutex> GOT_PROTECTION_LOCK = CGlobalDataFactory.createBytes(() -> PlatformLockingSupport.singleton().mutexSize());
3549

3650
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
3751
public static int getGOTEntryOffsetFromHeapRegister(int gotEntry) {
3852
return -(gotEntry + 1) * SubstrateTarget.getWordSize();
3953
}
4054

41-
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
55+
@Uninterruptible(reason = "Writing to GOT must be uninterruptible because safepoints re-protect the GOT mapping when no writer leases are active.")
4256
public static void writeToGOTEntry(int gotEntry, UnsignedWord address) {
57+
acquireWriteLease();
58+
try {
59+
writeToGOTEntryWithLease(gotEntry, address);
60+
} finally {
61+
releaseWriteLease();
62+
}
63+
}
64+
65+
@Uninterruptible(reason = "Writing to GOT must be uninterruptible because safepoints re-protect the GOT mapping when no writer leases are active.")
66+
public static void writeToGOTEntries(int[] gotEntries, int[] offsets, Pointer baseAddress) {
67+
assert gotEntries.length == offsets.length : "GOT entries and offsets must have the same length.";
68+
acquireWriteLease();
69+
try {
70+
for (int i = 0; i < gotEntries.length; i++) {
71+
writeToGOTEntryWithLease(gotEntries[i], baseAddress.add(offsets[i]));
72+
}
73+
} finally {
74+
releaseWriteLease();
75+
}
76+
}
77+
78+
@Uninterruptible(reason = "Uses the no-transition GOT mutex, the lease and store must not be interrupted by a safepoint.", callerMustBe = true)
79+
private static void acquireWriteLease() {
80+
lockMutex();
81+
try {
82+
setGOTWritable(true);
83+
UnsignedWord writers = ACTIVE_WRITERS.get().read(0);
84+
ACTIVE_WRITERS.get().write(0, writers.add(1));
85+
} finally {
86+
unlockMutex();
87+
}
88+
}
89+
90+
@Uninterruptible(reason = "Uses the no-transition GOT mutex, the lease and store must not be interrupted by a safepoint.", callerMustBe = true)
91+
private static void releaseWriteLease() {
92+
lockMutex();
93+
try {
94+
UnsignedWord writers = ACTIVE_WRITERS.get().read(0);
95+
VMError.guarantee(writers.aboveThan(0), "GOT writer count underflow.");
96+
ACTIVE_WRITERS.get().write(0, writers.subtract(1));
97+
} finally {
98+
unlockMutex();
99+
}
100+
}
101+
102+
@Uninterruptible(reason = "Releasing the GOT mutex without a thread-state transition requires the caller to be uninterruptible.", callerMustBe = true)
103+
private static void unlockMutex() {
104+
PlatformLockingSupport.singleton().unlockMutex(GOT_PROTECTION_LOCK.get());
105+
}
106+
107+
@Uninterruptible(reason = "Acquiring the GOT mutex without a thread-state transition requires the caller to be uninterruptible.", callerMustBe = true)
108+
private static void lockMutex() {
109+
PlatformLockingSupport.singleton().lockMutexNoTransition(GOT_PROTECTION_LOCK.get());
110+
}
111+
112+
@Uninterruptible(reason = "Locking the GOT mutex requires the whole critical section to be uninterruptible.")
113+
static int initializeInFirstIsolate() {
114+
int ret = PlatformLockingSupport.singleton().initializeMutex(GOT_PROTECTION_LOCK.get());
115+
if (ret != 0) {
116+
return CEntryPointErrors.THREADING_INITIALIZATION_FAILED;
117+
}
118+
lockMutex();
119+
try {
120+
setGOTWritable(false);
121+
} finally {
122+
unlockMutex();
123+
}
124+
return CEntryPointErrors.NO_ERROR;
125+
}
126+
127+
@Uninterruptible(reason = "Locking the GOT mutex requires the whole critical section to be uninterruptible.")
128+
public static void reprotectGOTAtSafepoint() {
129+
if (!GOTHeapSupport.hasGOTEntries()) {
130+
return;
131+
}
132+
lockMutex();
133+
try {
134+
if (ACTIVE_WRITERS.get().read(0).rawValue() == 0) {
135+
setGOTWritable(false);
136+
}
137+
} finally {
138+
unlockMutex();
139+
}
140+
}
141+
142+
@Uninterruptible(reason = "The caller holds a GOT writer lease.", callerMustBe = true)
143+
private static void writeToGOTEntryWithLease(int gotEntry, UnsignedWord address) {
43144
Pointer gotStartAddress = GOTHeapSupport.GOT_START_ADDRESS.get().read();
44145
Pointer gotEndAddress = gotStartAddress.add(GOTHeapSupport.getPageAlignedGOTSize());
45146
gotEndAddress.writeWord(getGOTEntryOffsetFromHeapRegister(gotEntry), address);
46147
}
47148

149+
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
150+
private static boolean isGOTWritable() {
151+
return GOT_WRITABLE.get().readWord(0).rawValue() != 0;
152+
}
153+
154+
@Uninterruptible(reason = "The caller holds the no-transition GOT mutex.", callerMustBe = true)
155+
private static void setGOTWritable(boolean writable) {
156+
if (isGOTWritable() != writable) {
157+
Pointer gotMappingStartAddress = GOTHeapSupport.GOT_START_ADDRESS.get().read();
158+
int protection = VirtualMemoryProvider.Access.READ | (writable ? VirtualMemoryProvider.Access.WRITE : 0);
159+
int ret = VirtualMemoryProvider.get().protect(gotMappingStartAddress, GOTHeapSupport.getPageAlignedGOTSize(), protection);
160+
VMError.guarantee(ret == 0, "Failed to change GOT protection.");
161+
GOT_WRITABLE.get().writeWord(0, Word.signed(writable ? 1 : 0));
162+
}
163+
}
164+
48165
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
49166
public static Word readFromGOTEntry(int gotEntry) {
50167
Pointer gotStartAddress = GOTHeapSupport.GOT_START_ADDRESS.get().read();
51168
Pointer gotEndAddress = gotStartAddress.add(GOTHeapSupport.getPageAlignedGOTSize());
52169
return gotEndAddress.readWord(getGOTEntryOffsetFromHeapRegister(gotEntry));
53170
}
171+
54172
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/pltgot/GOTHeapSupport.java‎

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,12 @@
3939

4040
import com.oracle.svm.core.SubstrateOptions;
4141
import com.oracle.svm.core.code.DynamicMethodAddressResolutionHeapSupport;
42-
import com.oracle.svm.core.os.VirtualMemoryProvider;
43-
import com.oracle.svm.core.os.VirtualMemoryProvider.Access;
44-
import com.oracle.svm.guest.staging.core.graal.KnownIntrinsics;
4542
import com.oracle.svm.core.util.PointerUtils;
4643
import com.oracle.svm.guest.staging.c.CGlobalData;
4744
import com.oracle.svm.guest.staging.c.CGlobalDataFactory;
4845
import com.oracle.svm.guest.staging.c.function.CEntryPointErrors;
4946
import com.oracle.svm.guest.staging.config.SubstrateGuestTarget;
5047
import com.oracle.svm.shared.Uninterruptible;
51-
import com.oracle.svm.shared.util.VMError;
5248

5349
import jdk.graal.compiler.api.replacements.Fold;
5450
import jdk.graal.compiler.nodes.PauseNode;
@@ -95,7 +91,7 @@ protected static UnsignedWord getGOTSectionSize() {
9591
* Runtime code must use this logical flag, not section allocation size, to decide whether GOT initialization is needed.
9692
*/
9793
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
98-
private static boolean hasGOTEntries() {
94+
static boolean hasGOTEntries() {
9995
Word numberOfEntries = NUMBER_OF_GOT_ENTRIES.get().readWord(0);
10096
return numberOfEntries.aboveThan(0);
10197
}
@@ -123,20 +119,6 @@ public static GOTHeapSupport get() {
123119
return (GOTHeapSupport) ImageSingletons.lookup(DynamicMethodAddressResolutionHeapSupport.class);
124120
}
125121

126-
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
127-
public void makeGOTWritable() {
128-
if (hasGOTEntries()) {
129-
changeGOTMappingProtection(true);
130-
}
131-
}
132-
133-
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
134-
public void makeGOTReadOnly() {
135-
if (hasGOTEntries()) {
136-
changeGOTMappingProtection(false);
137-
}
138-
}
139-
140122
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
141123
protected abstract int mapGOT(Pointer address);
142124

@@ -149,31 +131,13 @@ public int install(Pointer heapBase) {
149131
return mapGOT(getPreHeapMappingStartAddress(heapBase));
150132
}
151133

152-
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
153-
protected Pointer getPreHeapMappingStartAddress() {
154-
return getPreHeapMappingStartAddress(KnownIntrinsics.heapBase());
155-
}
156-
157134
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
158135
protected Pointer getPreHeapMappingStartAddress(PointerBase heapBase) {
159136
return ((Pointer) heapBase).subtract(getRequiredPreHeapMemoryInBytes());
160137
}
161138

162-
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
163-
private static void changeGOTMappingProtection(boolean writable) {
164-
Pointer gotMappingStartAddress = GOT_START_ADDRESS.get().read();
165-
VMError.guarantee(gotMappingStartAddress.isNonNull());
166-
int access = Access.READ;
167-
if (writable) {
168-
access |= Access.WRITE;
169-
}
170-
// GR-79030: coordinate protection of the process-wide GOT mapping across isolates.
171-
int ret = VirtualMemoryProvider.get().protect(gotMappingStartAddress, getPageAlignedGOTSize(), access);
172-
VMError.guarantee(ret == 0, "Failed to change GOT protection.");
173-
}
174-
175139
@Override
176-
@Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
140+
@Uninterruptible(reason = "Called during isolate initialization.")
177141
public int initialize() {
178142
if (!hasGOTEntries()) {
179143
return CEntryPointErrors.NO_ERROR;
@@ -195,8 +159,9 @@ public int initialize() {
195159
// Only the first isolate can reach here.
196160
int ret = initialize(GOT_START_ADDRESS.get());
197161
if (ret == CEntryPointErrors.NO_ERROR) {
198-
makeGOTReadOnly();
162+
ret = GOTAccess.initializeInFirstIsolate();
199163
}
164+
200165
GOT_STATUS.get().writeWordVolatile(0, Word.signed(ret));
201166
return ret;
202167
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/pltgot/MethodAddressResolutionDispatcher.java‎

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -25,41 +25,10 @@
2525
package com.oracle.svm.core.pltgot;
2626

2727
import com.oracle.svm.shared.Uninterruptible;
28-
import com.oracle.svm.core.thread.JavaSpinLockUtils;
29-
30-
import jdk.internal.misc.Unsafe;
3128

3229
public class MethodAddressResolutionDispatcher {
33-
private static final MethodAddressResolutionDispatcher dispatcher = new MethodAddressResolutionDispatcher();
34-
private static final long LOCK_OFFSET = Unsafe.getUnsafe().objectFieldOffset(MethodAddressResolutionDispatcher.class, "lock");
35-
36-
@SuppressWarnings("unused")//
37-
private volatile int lock;
38-
private int activeResolverInstances = 0;
39-
4030
@Uninterruptible(reason = "PLT/GOT method address resolution doesn't support interruptible code paths.")
4131
protected static long resolveMethodAddress(long gotEntry) {
42-
try {
43-
JavaSpinLockUtils.lockNoTransition(dispatcher, LOCK_OFFSET);
44-
if (dispatcher.activeResolverInstances == 0) {
45-
GOTHeapSupport.get().makeGOTWritable();
46-
}
47-
dispatcher.activeResolverInstances++;
48-
} finally {
49-
JavaSpinLockUtils.unlock(dispatcher, LOCK_OFFSET);
50-
}
51-
52-
long resolvedMethodAddress = PLTGOTConfiguration.singleton().getMethodAddressResolver().resolveMethodWithGOTEntry(gotEntry);
53-
54-
try {
55-
JavaSpinLockUtils.lockNoTransition(dispatcher, LOCK_OFFSET);
56-
if (dispatcher.activeResolverInstances == 1) {
57-
GOTHeapSupport.get().makeGOTReadOnly();
58-
}
59-
dispatcher.activeResolverInstances--;
60-
} finally {
61-
JavaSpinLockUtils.unlock(dispatcher, LOCK_OFFSET);
62-
}
63-
return resolvedMethodAddress;
32+
return PLTGOTConfiguration.singleton().getMethodAddressResolver().resolveMethodWithGOTEntry(gotEntry);
6433
}
6534
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/pltgot/MethodAddressResolver.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ public interface MethodAddressResolver {
3333
*
3434
* Note that it is the responsibility of this method to write the resolved address to the GOT
3535
* entry as otherwise it will be called for subsequent calls of the same method.
36+
* Implementations should use {@link GOTAccess#writeToGOTEntry} or {@link GOTAccess#writeToGOTEntries}
37+
* for batch writes. GOT mapping is made read-only at the next safepoint with no active writers
38+
* to avoid repeated page protection changes, {@link GOTAccess#reprotectGOTAtSafepoint()}.
3639
*
3740
* This method might execute in a context that does not declare writes to the memory locations
3841
* it uses or other side effects that it has (e.g., foreign calls), so it must be very careful

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/Safepoint.java‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
*/
2525
package com.oracle.svm.core.thread;
2626

27-
import static com.oracle.svm.guest.staging.option.RuntimeOptionKey.RuntimeOptionKeyFlag.RelevantForCompilationIsolates;
2827
import static com.oracle.svm.core.thread.VMThreads.SAFEPOINT_MUTEX;
28+
import static com.oracle.svm.guest.staging.option.RuntimeOptionKey.RuntimeOptionKeyFlag.RelevantForCompilationIsolates;
2929
import static com.oracle.svm.shared.Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE;
3030

3131
import org.graalvm.nativeimage.CurrentIsolate;
@@ -36,25 +36,27 @@
3636
import org.graalvm.word.UnsignedWord;
3737
import org.graalvm.word.impl.Word;
3838

39+
import com.oracle.svm.core.code.DynamicMethodAddressResolutionHeapSupport;
3940
import com.oracle.svm.core.heap.Heap;
40-
import com.oracle.svm.guest.staging.core.heap.RestrictHeapAccess;
4141
import com.oracle.svm.core.jfr.JfrTicks;
4242
import com.oracle.svm.core.jfr.events.SafepointBeginEvent;
4343
import com.oracle.svm.core.jfr.events.SafepointEndEvent;
44+
import com.oracle.svm.core.pltgot.GOTAccess;
45+
import com.oracle.svm.core.thread.VMThreads.SafepointBehavior;
46+
import com.oracle.svm.core.thread.VMThreads.StatusSupport;
4447
import com.oracle.svm.guest.staging.core.graal.KnownIntrinsics;
48+
import com.oracle.svm.guest.staging.core.heap.RestrictHeapAccess;
4549
import com.oracle.svm.guest.staging.log.Log;
4650
import com.oracle.svm.guest.staging.option.RuntimeOptionKey;
47-
import com.oracle.svm.core.thread.VMThreads.SafepointBehavior;
48-
import com.oracle.svm.core.thread.VMThreads.StatusSupport;
49-
import com.oracle.svm.shared.util.DuplicatedInNativeCode;
50-
import com.oracle.svm.shared.util.TimeUtils;
5151
import com.oracle.svm.shared.Uninterruptible;
5252
import com.oracle.svm.shared.singletons.AutomaticallyRegisteredImageSingleton;
5353
import com.oracle.svm.shared.singletons.traits.BuiltinTraits.AllAccess;
5454
import com.oracle.svm.shared.singletons.traits.BuiltinTraits.NoLayeredCallbacks;
5555
import com.oracle.svm.shared.singletons.traits.BuiltinTraits.PartiallyLayerAware;
5656
import com.oracle.svm.shared.singletons.traits.SingletonLayeredInstallationKind.Duplicable;
5757
import com.oracle.svm.shared.singletons.traits.SingletonTraits;
58+
import com.oracle.svm.shared.util.DuplicatedInNativeCode;
59+
import com.oracle.svm.shared.util.TimeUtils;
5860
import com.oracle.svm.shared.util.VMError;
5961

6062
import jdk.graal.compiler.api.replacements.Fold;
@@ -176,6 +178,7 @@ void endSafepoint(boolean acquiredThreadsLock) {
176178
assert VMOperationControl.mayExecuteVmOperations();
177179
long startTicks = JfrTicks.elapsedTicks();
178180

181+
beforeSafepointEnd();
179182
safepointState = NOT_AT_SAFEPOINT;
180183
releaseThreadsFromSafepoint();
181184

@@ -203,6 +206,12 @@ void endSafepoint(boolean acquiredThreadsLock) {
203206
VMThreads.singleton().cleanupExitedOsThreads();
204207
}
205208

209+
private static void beforeSafepointEnd() {
210+
if (DynamicMethodAddressResolutionHeapSupport.isEnabled()) {
211+
GOTAccess.reprotectGOTAtSafepoint();
212+
}
213+
}
214+
206215
/**
207216
* Acquires the {@link ThreadsLock} with non-exclusive write access, if the current thread
208217
* doesn't already hold it with that access.

0 commit comments

Comments
 (0)