openjdk / panama-foreign Public
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8266814: Improve library loading with SymbolLookup abstraction #531
Changes from 7 commits
b168311
9abb5db
66cbb8e
feed514
0270d9b
59e794b
0a05157
1b5f087
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
@@ -0,0 +1,59 @@ | ||
# | ||
# Copyright (c) 2021, Oracle and/or its affiliates. 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. | ||
# | ||
|
||
include LibCommon.gmk | ||
|
||
ifeq ($(call isTargetOs, linux), true) | ||
|
||
$(eval $(call SetupJdkLibrary, BUILD_LIBCSTDLIB, \ | ||
NAME := syslookup, \ | ||
OPTIMIZATION := HIGH, \ | ||
DISABLED_WARNINGS_gcc := sign-compare pointer-arith, \ | ||
DISABLED_WARNINGS_clang := sign-compare pointer-arith format-nonliteral, \ | ||
CFLAGS := $(CFLAGS_JDKLIB), \ | ||
CXXFLAGS := $(CXXFLAGS_JDKLIB), \ | ||
LDFLAGS := -Wl$(COMMA)--no-as-needed -lc -lm $(LDFLAGS_JDKLIB) $(call SET_SHARED_LIBRARY_ORIGIN), \ | ||
LIBS := $(LIBCXX), \ | ||
)) | ||
|
||
else ifeq ($(call isTargetOs, windows), false) | ||
|
||
$(eval $(call SetupJdkLibrary, BUILD_LIBCSTDLIB, \ | ||
NAME := syslookup, \ | ||
OPTIMIZATION := HIGH, \ | ||
DISABLED_WARNINGS_gcc := sign-compare pointer-arith, \ | ||
DISABLED_WARNINGS_clang := sign-compare pointer-arith format-nonliteral, \ | ||
CFLAGS := $(CFLAGS_JDKLIB), \ | ||
CXXFLAGS := $(CXXFLAGS_JDKLIB), \ | ||
LDFLAGS := $(LDFLAGS_JDKLIB) $(call SET_SHARED_LIBRARY_ORIGIN), \ | ||
LIBS := $(LIBCXX), \ | ||
)) | ||
|
||
|
||
endif | ||
|
||
TARGETS += $(BUILD_LIBCSTDLIB) | ||
|
||
################################################################################ |
@@ -0,0 +1,97 @@ | ||
/* | ||
* Copyright (c) 2021, Oracle and/or its affiliates. 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 jdk.incubator.foreign; | ||
|
||
import jdk.internal.access.JavaLangAccess; | ||
import jdk.internal.access.SharedSecrets; | ||
import jdk.internal.reflect.CallerSensitive; | ||
import jdk.internal.reflect.Reflection; | ||
|
||
import java.util.Objects; | ||
import java.util.Optional; | ||
|
||
/** | ||
* A symbol lookup. Exposes a lookup operation for searching symbols, see {@link SymbolLookup#lookup(String)}. | ||
* <p> Unless otherwise specified, passing a {@code null} argument, or an array argument containing one or more {@code null} | ||
* elements to a method in this class causes a {@link NullPointerException NullPointerException} to be thrown. </p> | ||
*/ | ||
@FunctionalInterface | ||
public interface SymbolLookup { | ||
|
||
/** | ||
* Looks up a symbol with given name in this lookup. | ||
* | ||
* @param name the symbol name. | ||
* @return the memory address associated with the symbol (if any). | ||
*/ | ||
Optional<MemoryAddress> lookup(String name); | ||
|
||
/** | ||
* Obtains a symbol lookup suitable to find symbols in native libraries associated with the caller's classloader | ||
* (that is, libraries loaded using {@link System#loadLibrary} or {@link System#load}). | ||
* <p> | ||
* This method is <a href="package-summary.html#restricted"><em>restricted</em></a>. | ||
* Restricted method are unsafe, and, if used incorrectly, their use might crash | ||
* the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on | ||
* restricted methods, and use safe and supported functionalities, where possible. | ||
* | ||
* @return a symbol lookup suitable to find symbols in libraries loaded by the caller's classloader. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this include an |
||
*/ | ||
@CallerSensitive | ||
static SymbolLookup loaderLookup() { | ||
Class<?> caller = Reflection.getCallerClass(); | ||
Reflection.ensureNativeAccess(caller); | ||
ClassLoader loader = Objects.requireNonNull(caller.getClassLoader()); | ||
return loaderLookup0(loader); | ||
} | ||
|
||
/** | ||
* Obtains a symbol lookup suitable to find symbols in native libraries associated with the given classloader | ||
* (that is, libraries loaded using {@link System#loadLibrary} or {@link System#load}). | ||
* <p> | ||
* This method is <a href="package-summary.html#restricted"><em>restricted</em></a>. | ||
* Restricted method are unsafe, and, if used incorrectly, their use might crash | ||
* the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on | ||
* restricted methods, and use safe and supported functionalities, where possible. | ||
* | ||
* @param loader the classloader whose symbol lookup is to be retrieved. | ||
* @return a symbol lookup suitable to find symbols in libraries loaded by given classloader. | ||
*/ | ||
@CallerSensitive | ||
static SymbolLookup loaderLookup(ClassLoader loader) { | ||
Reflection.ensureNativeAccess(Reflection.getCallerClass()); | ||
Objects.requireNonNull(loader); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we filter null (bootstrap) loader? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because boot loader is under control of the JDK, not the user. I don't think it's desirable to introduce coupling between user code and set of libraries which happen to be loaded by boot loader. |
||
return loaderLookup0(loader); | ||
} | ||
|
||
private static SymbolLookup loaderLookup0(ClassLoader loader) { | ||
return name -> { | ||
Objects.requireNonNull(name); | ||
JavaLangAccess javaLangAccess = SharedSecrets.getJavaLangAccess(); | ||
MemoryAddress addr = MemoryAddress.ofLong(javaLangAccess.findNative(loader, name)); | ||
return addr == MemoryAddress.NULL? Optional.empty() : Optional.of(addr); | ||
}; | ||
} | ||
} |
@@ -0,0 +1,60 @@ | ||||||
/* | ||||||
* Copyright (c) 2021, Oracle and/or its affiliates. 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 jdk.internal.foreign; | ||||||
|
||||||
import jdk.incubator.foreign.SymbolLookup; | ||||||
import jdk.incubator.foreign.MemoryAddress; | ||||||
import jdk.internal.loader.NativeLibraries; | ||||||
import jdk.internal.loader.NativeLibrary; | ||||||
|
||||||
import java.nio.file.Path; | ||||||
import java.util.Objects; | ||||||
import java.util.Optional; | ||||||
|
||||||
public class SystemLookup implements SymbolLookup { | ||||||
|
||||||
private SystemLookup() { } | ||||||
|
||||||
final static SystemLookup INSTANCE = new SystemLookup(); | ||||||
|
||||||
final NativeLibrary syslookup = switch (CABI.current()) { | ||||||
case SysV, AArch64 -> NativeLibraries.rawNativeLibraries(SystemLookup.class, false).loadLibrary("syslookup"); | ||||||
case Win64 -> NativeLibraries.rawNativeLibraries(SystemLookup.class, false) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e.g. a short comment here that explains why Windows is different. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add a comment here |
||||||
.loadLibrary(Path.of(System.getenv("SystemRoot"), "System32", "msvcrt.dll").toString()); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this loadLibrary call does not except paths:
So this is causing an assertion error on Windows. I'll try to find a fix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a bug - we should use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This patch fixes the problem:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this work?! Path load has to use System::load as @mcimadamore mentioned... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I can tell there's no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Right - note that this code was NOT using System::loadLibrary, but NativeLibraries::loadLibrary - hence the confusion. |
||||||
}; | ||||||
|
||||||
@Override | ||||||
public Optional<MemoryAddress> lookup(String name) { | ||||||
Objects.requireNonNull(name); | ||||||
long addr = syslookup.find(name); | ||||||
return addr == 0 ? | ||||||
Optional.empty() : Optional.of(MemoryAddress.ofLong(addr)); | ||||||
} | ||||||
|
||||||
public static SystemLookup getInstance() { | ||||||
return INSTANCE; | ||||||
} | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this code, and maybe also the associated code in SystemLookup that loads msvcrt.dll could do with a comment that explains why the same strategy is not used on Windows; namely that symbol lookup on Windows does not search a library's dependencies, as opposed to dlsym, so it's not as easy to re-export the symbols in msvcrt by creating a shim library.