On 2020-02-27 00:31, David Holmes wrote:
Hi Bob,
On 27/02/2020 7:01 am, Bob Vandette wrote:
Here’s an updated webrev that implementes the suggestion that allows
JNIEXPORT in jni.h to be overridden
and the build limits visibility for static libraries.
! #if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) &&
(__GNUC_MINOR__ > 2))) || __has_attribute(visibility)
Don't we have minimum gcc version requirement (>4.2) that negates the
need for this explicit check now? Magnus?
You are forgetting that this file is not really for us, it's part of our
exported interface in jni.h. (And in fact, as I have stated before, I
think we are making a mistake in using the defines from jni.h for our
internal purposes.) This file is consumed by all Java developers who are
using JNI. And they might have any kind of compiler.
/Magnus
If this webrev is accepted, I’ll update the CSR solution to match
this implementation.
I'm not even sure a CSR request is even warranted now.
Thanks,
David
http://cr.openjdk.java.net/~bobv/8239563/webrev.01
Bob.
On Feb 26, 2020, at 10:35 AM, Magnus Ihse Bursie
<magnus.ihse.bur...@oracle.com
<mailto:magnus.ihse.bur...@oracle.com>> wrote:
On 2020-02-26 15:56, Bob Vandette wrote:
On Feb 26, 2020, at 9:17 AM, Magnus Ihse Bursie
<magnus.ihse.bur...@oracle.com
<mailto:magnus.ihse.bur...@oracle.com>> wrote:
On 2020-02-26 14:31, Bob Vandette wrote:
On Feb 26, 2020, at 7:31 AM, Magnus Ihse Bursie
<magnus.ihse.bur...@oracle.com
<mailto:magnus.ihse.bur...@oracle.com>> wrote:
On 2020-02-26 08:41, David Holmes wrote:
Hi Bob,
Adding build-dev.
Thanks for noticing that we were missing, David!
Sorry, I should have included you folks.
On 26/02/2020 6:37 am, Bob Vandette wrote:
Please review this RFE that alters the visibility of JNI
entrypoints to hidden when a shared library
is created using static JDK libraries.
RFE:
https://bugs.openjdk.java.net/browse/JDK-8239563
WEBREV:
http://cr.openjdk.java.net/~bobv/8239563/webrev.00/
CSR:
https://bugs.openjdk.java.net/browse/JDK-8239791
All JNI entrypoints that exist in JDK static libraries are
declared as exported or visible.
If a dynamic library is built from these static libraries, we
end up with many exported
functions that we don't want to provide access to,
This RFE will change the definition of JNIEXPORT for libraries
built when JNI_STATIC_BUILD
is defined. When defined, functions declared with JNIEXPORT
will not be exported when
linked into shared or dynamic libraries. This will still
allow linking of these functions into
dynamic libraries but will not export the JDK symbols outside
of the shared library.
A CSR has been filed
(https://bugs.openjdk.java.net/browse/JDK-8239791) to add the
JNI_STATIC_BUILD
define support in jni.h.
I have reservations about turning this into something we have
to expose and support, rather than being something totally
handled within the OpenJDK build system.
I fully agree. The JNI headers are an exported interface. Our
internal build mechanisms have nothing to do there.
I'm tempted to suggest the header files be adjusted to have:
#ifndef JNIEXPORT
<JNIEXPORT basic definitions as they are now >
#endif
and then we define the modified JNIEXPORT via the build system
when doing a static build.
Is that feasible?
It's definitely doable, and a far better solution.
Yes, I like this solution.
A patch something akin to this would be needed:
diff --git a/make/autoconf/flags-cflags.m4
b/make/autoconf/flags-cflags.m4
--- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -709,7 +709,10 @@
# JDK libraries.
STATIC_LIBS_CFLAGS="-DSTATIC_BUILD=1"
if test "x$TOOLCHAIN_TYPE" = xgcc || test "x$TOOLCHAIN_TYPE"
= xclang; then
- STATIC_LIBS_CFLAGS="$STATIC_LIBS_CFLAGS -ffunction-sections
-fdata-sections"
+ STATIC_LIBS_CFLAGS="$STATIC_LIBS_CFLAGS -ffunction-sections \
+ -fdata-sections
-DJNIEXPORT=__attribute__((visibility(\"hidden\")))"
+ else
+ STATIC_LIBS_CFLAGS="$STATIC_LIBS_CFLAGS -DJNIEXPORT="
fi
if test "x$TOOLCHAIN_TYPE" = xgcc; then
# Disable relax-relocation to enable compatibility with
older linkers
(With the reservation that I just wrote this on the fly and have
not tested it -- things like quoting might be off. Also, I'm not
sure if the match of
compilers is correct -- it might be the case that all compilers
except Microsoft defines __GNUC__, so maybe the addition of this
-D flag might need
a separate if statement to cover all our compilers correctly.)
Most of the STATIC_BUILD support is done in jni_util.h. We could
define JNIEXPORT in that header file after allowing it to be
overridden in jni.h.
I'm not sure I understand you correctly here. Do you mean that
you'd like to re-define JNIEXPORT inside jni_util.h instead of
using compiler command line flags? I don't think that'd work --
all libraries using JNIEXPORT that does not include jni_util.h
first would then export their symbols just the same. Even if you
fixed those, the system would be very fragile.
I was just trying to keep all static library building options in
one place. The static libraries that we produce need to include
jni_util.h
or the JNI_OnLoad_xxx functions will not be declared properly. Why
not expand that dependency to the JNIEXPORT?
Unless *all* libraries that include jni.h also include jni_util.h,
then the current definition of JNIEXPORT in jni.h will be used --
meaning that the so decorated functions will be exported -- which
was exactly what you wanted to prevent. So I fail to see how this
can be a solution.
Do we really have access to all of these compiler defines from
within our Makefiles?
#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) &&
(__GNUC_MINOR__ > 2))) || __has_attribute(visibility)
Well, yes and no. I'm not certain which compilers define __GNUC__
just to show compatibility with gcc, but otoh that does not really
matter. All that matters is that we know how we want JNIEXPORT to be
defined when creating a static build -- and that we know, since we
can check which toolchain we're using. (This is btw a far better
check than to look for __GNUC__).
/Magnus
Bob.
/Magnus
Bob.
/Magnus
Thanks,
David
BACKGROUND:
In JDK8 the JNI specification and JDK implementation was
enhanced to support static JNI libraries
but we didn’t consider the issue of exportibility of JNI
entrypoint symbols.
https://bugs.openjdk.java.net/browse/JDK-8005716
If developers use these static JDK libraries in order to
produce a custom shared library, all of the
JNIEXPORTS will be exposed by this library even if the
developer did not choose to export these.
This is a security issue and a potential problem if this
library is mixed with other libraries containing
these symbols.
Bob.
