Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mcimadamore committed May 14, 2024
1 parent bad1094 commit 6805e29
Show file tree
Hide file tree
Showing 12 changed files with 35 additions and 27 deletions.
3 changes: 1 addition & 2 deletions src/hotspot/share/prims/nativeLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ address NativeLookup::lookup_style(const methodHandle& method, char* pure_name,
}

// Otherwise call static method findNative in ClassLoader

Klass* klass = vmClasses::ClassLoader_klass();
Handle jni_class(THREAD, method->method_holder()->java_mirror());
Handle jni_name_arg = java_lang_String::create_from_str(jni_name, CHECK_NULL);
Expand Down Expand Up @@ -416,14 +415,14 @@ address NativeLookup::lookup_base(const methodHandle& method, TRAPS) {
entry = lookup_entry_prefixed(method, CHECK_NULL);
if (entry != nullptr) return entry;


if (THREAD->has_pending_exception()) {
oop exception = THREAD->pending_exception();
if (exception->is_a(vmClasses::IllegalCallerException_klass())) {
// we already have a pending exception from the restricted method check, just return
return nullptr;
}
}

// Native function not found, throw UnsatisfiedLinkError
stringStream ss;
ss.print("'");
Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/classes/java/lang/ClassLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -2448,7 +2448,7 @@ static NativeLibrary loadLibrary(Class<?> fromClass, String name) {
static long findNative(ClassLoader loader, Class<?> clazz, String entryName, String javaName) {
long addr = findNativeInternal(loader, entryName);
if (addr != 0 && loader != null) {
Reflection.ensureNativeAccess(clazz, clazz, javaName);
Reflection.ensureNativeAccess(clazz, clazz, javaName, true);
}
return addr;
}
Expand Down
23 changes: 16 additions & 7 deletions src/java.base/share/classes/java/lang/Module.java
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ private Module moduleForNativeAccess() {
}

// This is invoked from Reflection.ensureNativeAccess
void ensureNativeAccess(Class<?> owner, String methodName, Class<?> currentClass) {
void ensureNativeAccess(Class<?> owner, String methodName, Class<?> currentClass, boolean jni) {
// The target module whose enableNativeAccess flag is ensured
Module target = moduleForNativeAccess();
ModuleBootstrap.IllegalNativeAccess illegalNativeAccess = ModuleBootstrap.illegalNativeAccess();
Expand All @@ -328,12 +328,21 @@ void ensureNativeAccess(Class<?> owner, String methodName, Class<?> currentClass
}
String modflag = isNamed() ? getName() : "ALL-UNNAMED";
String caller = currentClass != null ? currentClass.getName() : "code";
System.err.printf("""
WARNING: A restricted method in %s has been called
WARNING: %s has been called by %s in %s
WARNING: Use --enable-native-access=%s to avoid a warning for callers in this module
WARNING: Restricted methods will be blocked in a future release unless native access is enabled
%n""", cls, mtd, caller, mod, modflag);
if (jni) {
System.err.printf("""
WARNING: A native method in %s has been bound
WARNING: %s has been called by %s in %s
WARNING: Use --enable-native-access=%s to avoid a warning for callers in this module
WARNING: Restricted methods will be blocked in a future release unless native access is enabled
%n""", cls, mtd, caller, mod, modflag);
} else {
System.err.printf("""
WARNING: A restricted method in %s has been called
WARNING: %s has been called by %s in %s
WARNING: Use --enable-native-access=%s to avoid a warning for callers in this module
WARNING: Restricted methods will be blocked in a future release unless native access is enabled
%n""", cls, mtd, caller, mod, modflag);
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/classes/java/lang/ModuleLayer.java
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ public Controller addOpens(Module source, String pn, Module target) {
public Controller enableNativeAccess(Module target) {
ensureInLayer(target);
Reflection.ensureNativeAccess(Reflection.getCallerClass(), Module.class,
"enableNativeAccess");
"enableNativeAccess", false);
target.implAddEnableNativeAccess();
return this;
}
Expand Down
4 changes: 2 additions & 2 deletions src/java.base/share/classes/java/lang/Runtime.java
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,7 @@ public void runFinalization() {
@Restricted
public void load(String filename) {
Class<?> caller = Reflection.getCallerClass();
Reflection.ensureNativeAccess(caller, Runtime.class, "load");
Reflection.ensureNativeAccess(caller, Runtime.class, "load", false);
load0(caller, filename);
}

Expand Down Expand Up @@ -910,7 +910,7 @@ void load0(Class<?> fromClass, String filename) {
@Restricted
public void loadLibrary(String libname) {
Class<?> caller = Reflection.getCallerClass();
Reflection.ensureNativeAccess(caller, Runtime.class, "loadLibrary");
Reflection.ensureNativeAccess(caller, Runtime.class, "loadLibrary", false);
loadLibrary0(caller, libname);
}

Expand Down
8 changes: 4 additions & 4 deletions src/java.base/share/classes/java/lang/System.java
Original file line number Diff line number Diff line change
Expand Up @@ -2030,7 +2030,7 @@ public static void runFinalization() {
@Restricted
public static void load(String filename) {
Class<?> caller = Reflection.getCallerClass();
Reflection.ensureNativeAccess(caller, System.class, "load");
Reflection.ensureNativeAccess(caller, System.class, "load", false);
Runtime.getRuntime().load0(caller, filename);
}

Expand Down Expand Up @@ -2073,7 +2073,7 @@ public static void load(String filename) {
@Restricted
public static void loadLibrary(String libname) {
Class<?> caller = Reflection.getCallerClass();
Reflection.ensureNativeAccess(caller, System.class, "loadLibrary");
Reflection.ensureNativeAccess(caller, System.class, "loadLibrary", false);
Runtime.getRuntime().loadLibrary0(caller, libname);
}

Expand Down Expand Up @@ -2550,8 +2550,8 @@ public boolean addEnableNativeAccess(ModuleLayer layer, String name) {
public void addEnableNativeAccessToAllUnnamed() {
Module.implAddEnableNativeAccessToAllUnnamed();
}
public void ensureNativeAccess(Module m, Class<?> owner, String methodName, Class<?> currentClass) {
m.ensureNativeAccess(owner, methodName, currentClass);
public void ensureNativeAccess(Module m, Class<?> owner, String methodName, Class<?> currentClass, boolean jni) {
m.ensureNativeAccess(owner, methodName, currentClass, jni);
}
public ServicesCatalog getServicesCatalog(ModuleLayer layer) {
return layer.getServicesCatalog();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ static SymbolLookup loaderLookup() {
@Restricted
static SymbolLookup libraryLookup(String name, Arena arena) {
Reflection.ensureNativeAccess(Reflection.getCallerClass(),
SymbolLookup.class, "libraryLookup");
SymbolLookup.class, "libraryLookup", false);
if (Utils.containsNullChars(name)) {
throw new IllegalArgumentException("Cannot open library: " + name);
}
Expand Down Expand Up @@ -326,7 +326,7 @@ static SymbolLookup libraryLookup(String name, Arena arena) {
@Restricted
static SymbolLookup libraryLookup(Path path, Arena arena) {
Reflection.ensureNativeAccess(Reflection.getCallerClass(),
SymbolLookup.class, "libraryLookup");
SymbolLookup.class, "libraryLookup", false);
if (path.getFileSystem() != FileSystems.getDefault()) {
throw new IllegalArgumentException("Path not in default file system: " + path);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ public interface JavaLangAccess {
* Ensure that the given module has native access. If not, warn or
* throw exception depending on the configuration.
*/
void ensureNativeAccess(Module m, Class<?> owner, String methodName, Class<?> currentClass);
void ensureNativeAccess(Module m, Class<?> owner, String methodName, Class<?> currentClass, boolean jni);

/**
* Returns the ServicesCatalog for the given Layer.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public final MemorySegment reinterpret(Arena arena, Consumer<MemorySegment> clea
}

public MemorySegment reinterpretInternal(Class<?> callerClass, long newSize, Scope scope, Consumer<MemorySegment> cleanup) {
Reflection.ensureNativeAccess(callerClass, MemorySegment.class, "reinterpret");
Reflection.ensureNativeAccess(callerClass, MemorySegment.class, "reinterpret", false);
Utils.checkNonNegativeArgument(newSize, "newSize");
if (!isNative()) throw new UnsupportedOperationException("Not a native segment");
Runnable action = cleanup != null ?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,15 @@ private record LinkRequest(FunctionDescriptor descriptor, LinkerOptions options)
@Override
@CallerSensitive
public final MethodHandle downcallHandle(MemorySegment symbol, FunctionDescriptor function, Option... options) {
Reflection.ensureNativeAccess(Reflection.getCallerClass(), Linker.class, "downcallHandle");
Reflection.ensureNativeAccess(Reflection.getCallerClass(), Linker.class, "downcallHandle", false);
SharedUtils.checkSymbol(symbol);
return downcallHandle0(function, options).bindTo(symbol);
}

@Override
@CallerSensitive
public final MethodHandle downcallHandle(FunctionDescriptor function, Option... options) {
Reflection.ensureNativeAccess(Reflection.getCallerClass(), Linker.class, "downcallHandle");
Reflection.ensureNativeAccess(Reflection.getCallerClass(), Linker.class, "downcallHandle", false);
return downcallHandle0(function, options);
}

Expand All @@ -115,7 +115,7 @@ private MethodHandle downcallHandle0(FunctionDescriptor function, Option... opti
@Override
@CallerSensitive
public final MemorySegment upcallStub(MethodHandle target, FunctionDescriptor function, Arena arena, Linker.Option... options) {
Reflection.ensureNativeAccess(Reflection.getCallerClass(), Linker.class, "upcallStub");
Reflection.ensureNativeAccess(Reflection.getCallerClass(), Linker.class, "upcallStub", false);
Objects.requireNonNull(arena);
Objects.requireNonNull(target);
Objects.requireNonNull(function);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ public int hashCode() {
@Override
@CallerSensitive
public AddressLayout withTargetLayout(MemoryLayout layout) {
Reflection.ensureNativeAccess(Reflection.getCallerClass(), AddressLayout.class, "withTargetLayout");
Reflection.ensureNativeAccess(Reflection.getCallerClass(), AddressLayout.class, "withTargetLayout", false);
Objects.requireNonNull(layout);
return new OfAddressImpl(order(), byteSize(), byteAlignment(), layout, name());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public static void ensureMemberAccess(Class<?> currentClass,
}

@ForceInline
public static void ensureNativeAccess(Class<?> currentClass, Class<?> owner, String methodName) {
public static void ensureNativeAccess(Class<?> currentClass, Class<?> owner, String methodName, boolean jni) {
// if there is no caller class, act as if the call came from unnamed module of system class loader
Module module = currentClass != null ?
currentClass.getModule() :
Expand All @@ -121,7 +121,7 @@ class Holder {
}
if (module != null) {
// not in init phase
Holder.JLA.ensureNativeAccess(module, owner, methodName, currentClass);
Holder.JLA.ensureNativeAccess(module, owner, methodName, currentClass, jni);
}
}

Expand Down

0 comments on commit 6805e29

Please sign in to comment.